Ai feature update #48
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive AI feature update that adds Angular equivalents of React BlockNote hooks, utilities for handling editor state, table cell manipulation features, and suggestion menu components.
- Complete Angular service equivalents for React hooks with reactive signal-based state management
- Enhanced table handling with color picker, header toggle, and cell splitting functionality
- Improved suggestions menu infrastructure with grid layouts and keyboard navigation
Reviewed Changes
Copilot reviewed 115 out of 115 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
services/ngx-blocknote.service.ts |
Updated service with proper typing and root-level injection |
services/hooks-equivalents/*.ts |
Angular service equivalents of React hooks using signals for reactive state |
util/*.ts |
Utility functions for URL sanitization, ref merging, and element overflow detection |
schema/*.ts |
Angular-specific schema implementations for blocks, inline content, and styles |
components/table-handles/**/*.ts |
Enhanced table manipulation with color picker, header controls, and cell splitting |
components/suggestions-menu/**/*.ts |
Grid-based suggestion menu with keyboard navigation and loading states |
editor/*.ts |
Context services and improved editor component with signal-based reactivity |
i18n/*.ts |
Dictionary service for internationalization support |
interfaces/*.ts |
Updated color options interface with additional properties |
Comments suppressed due to low confidence (1)
libs/ngx-blocknote/src/lib/schema/angular-inline-content-spec.ts:1
- Incorrect import - should import from '@tiptap/core' instead of '@tiptap/react' since this is an Angular implementation.
import {
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,15 @@ | |||
| export function elementOverflow(element: HTMLElement, container: HTMLElement) { | |||
There was a problem hiding this comment.
Missing JSDoc documentation for this utility function. Should include parameter descriptions and return value documentation explaining the possible return values: 'both', 'top', 'bottom', 'none'.
| const listener = () => { | ||
| const newScheme = this.getColorSchemeFromQueries(darkQuery, lightQuery); | ||
| this.preferredColorScheme.set(newScheme); | ||
| }; | ||
|
|
||
| // This handles both dark and light changes | ||
| darkQuery?.addEventListener('change', listener); | ||
| lightQuery?.addEventListener('change', listener); | ||
|
|
||
| // Clean up listeners | ||
| this.destroyRef.onDestroy(() => { | ||
| darkQuery?.removeEventListener('change', listener); | ||
| lightQuery?.removeEventListener('change', listener); |
There was a problem hiding this comment.
This fallback code will cause the same listener function to be added twice to both queries, which could lead to duplicate state updates. The listener should handle both cases internally or separate listeners should be used.
| const listener = () => { | |
| const newScheme = this.getColorSchemeFromQueries(darkQuery, lightQuery); | |
| this.preferredColorScheme.set(newScheme); | |
| }; | |
| // This handles both dark and light changes | |
| darkQuery?.addEventListener('change', listener); | |
| lightQuery?.addEventListener('change', listener); | |
| // Clean up listeners | |
| this.destroyRef.onDestroy(() => { | |
| darkQuery?.removeEventListener('change', listener); | |
| lightQuery?.removeEventListener('change', listener); | |
| const darkListener = () => { | |
| if (darkQuery?.matches) { | |
| this.preferredColorScheme.set('dark'); | |
| } | |
| }; | |
| const lightListener = () => { | |
| if (lightQuery?.matches) { | |
| this.preferredColorScheme.set('light'); | |
| } | |
| }; | |
| darkQuery?.addEventListener('change', darkListener); | |
| lightQuery?.addEventListener('change', lightListener); | |
| // Clean up listeners | |
| this.destroyRef.onDestroy(() => { | |
| darkQuery?.removeEventListener('change', darkListener); | |
| lightQuery?.removeEventListener('change', lightListener); |
| }); | ||
|
|
||
| return { | ||
| showLoader: showLoader.asReadonly(), |
There was a problem hiding this comment.
The return type should be () => boolean to match the interface, but showLoader.asReadonly() returns a signal function. Should return showLoader.asReadonly() directly without the wrapper object.
| showLoader: showLoader.asReadonly(), | |
| showLoader: () => showLoader(), |
| onMouseDown(event: MouseEvent): void { | ||
| this.onMouseDown(); |
There was a problem hiding this comment.
Infinite recursion - the method calls itself. Should call this.onMouseDown() input property or rename the method to avoid the conflict.
| export class BnaExtendButtonComponent< | ||
| I extends InlineContentSchema = DefaultInlineContentSchema, | ||
| S extends StyleSchema = DefaultStyleSchema, | ||
| > implements OnInit, OnDestroy, BnaExtendButtonProps<I, S> { |
There was a problem hiding this comment.
Component implements OnInit but doesn't import it from '@angular/core' and doesn't have an ngOnInit method defined.
| > implements OnInit, OnDestroy, BnaExtendButtonProps<I, S> { | |
| > implements OnDestroy, BnaExtendButtonProps<I, S> { |
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| onChange: any = () => {}; | ||
| onChange: (value: Block<BSchema, ISchema, SSchema>[]) => void = () => {}; |
There was a problem hiding this comment.
[nitpick] These ControlValueAccessor methods should use proper typing. Consider using any type for onChange to match ControlValueAccessor interface or properly type the value parameter.
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| onTouch: any = () => {}; | ||
| onTouch: () => void = () => {}; |
There was a problem hiding this comment.
[nitpick] These ControlValueAccessor methods should use proper typing. Consider using any type for onChange to match ControlValueAccessor interface or properly type the value parameter.
Improve compatibility with ES module resolution across libraries.
Refactor to prevent redundant ngxBlockNoteService call.
Ensure correct binding for custom editor input property.
2 participants
No description provided.