Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
you have to add each emoji img as its own file?? |
I'm using their plugins not implementing new one. I manage to display somehow toolbar, now need to fix almost everything :D |
|
also I need to remove odd code that we are not using there from playground example, maybe those emoji are only optional. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (2)
104-117: Consider throttlingonChangeHTML export (can be expensive on large docs).
$generateHtmlFromNodes(editor)on every update can become a hot path; consider debouncing/throttling (or emitting only on blur/submit) if consumers don’t need per-keystroke HTML.
119-168: Replaceanytypes withLexicalEditorfor editor parameters.The
parseHtmlToLexicalNodes(editor: any, ...)function at line 120 and thecreateInitialEditorState(...)initializer at line 172 should use theLexicalEditortype. This matches the documented Lexical API for editor state initializers and$generateNodesFromDOMcalls. Uncomment the import and apply the type to both functions:import { ParagraphNode, - // LexicalEditor, + type LexicalEditor, } from 'lexical'; -function parseHtmlToLexicalNodes(editor: any, html: string) { +function parseHtmlToLexicalNodes(editor: LexicalEditor, html: string) { - return (editor: any) => { + return (editor: LexicalEditor) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (10)
src/components/rich-text-lexical/plugins/ToolbarPlugin/fontSize.tsx (1)
parseAllowedFontSize(50-59)src/components/rich-text-lexical/ui/ColorPicker.tsx (1)
parseAllowedColor(23-25)src/components/rich-text-lexical/context/SettingsContext.tsx (1)
useSettings(57-59)src/components/rich-text-lexical/context/FlashMessageContext.tsx (1)
FlashMessageContext(35-61)src/components/rich-text-lexical/context/SharedHistoryContext.tsx (1)
SharedHistoryContext(21-33)src/components/rich-text-lexical/plugins/TablePlugin.tsx (1)
TableContext(55-82)src/components/rich-text-lexical/context/ToolbarContext.tsx (1)
ToolbarContext(94-130)src/components/rich-text-lexical/mainStyles.tsx (1)
EditorShell(670-1235)src/components/rich-text-lexical/Editor.tsx (1)
Editor(81-297)src/components/rich-text-lexical/plugins/TypingPerfPlugin/index.ts (1)
TypingPerfPlugin(33-126)
🪛 Biome (2.1.2)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx
[error] 281-281: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/rich-text-lexical/plugins/AutoEmbedPlugin/index.tsx (2)
223-231: Debounce helper may update state after unmount; consider adding cleanup and simplifying dependencies.The custom
debounceplususeMemopattern works but has two drawbacks:
- A timeout may still fire after the dialog has unmounted, calling
setEmbedResulton an unmounted component.embedResultin theuseMemodependency array recreates the debounced function on every validation result change, which is unnecessary and slightly increases churn.You can make
debouncemore robust and avoid extra re-creations by (a) typing the timeout handle more broadly and (b) adding acancelmethod that you call from auseEffectcleanup:-const debounce = (callback: (text: string) => void, delay: number) => { - let timeoutId: number; - return (text: string) => { - window.clearTimeout(timeoutId); - timeoutId = window.setTimeout(() => { - callback(text); - }, delay); - }; -}; +const debounce = (callback: (text: string) => void, delay: number) => { + let timeoutId: ReturnType<typeof setTimeout> | null = null; + + const debounced = (text: string) => { + if (timeoutId != null) { + clearTimeout(timeoutId); + } + timeoutId = setTimeout(() => { + callback(text); + }, delay); + }; + + debounced.cancel = () => { + if (timeoutId != null) { + clearTimeout(timeoutId); + timeoutId = null; + } + }; + + return debounced; +};Then in
AutoEmbedDialog:- const validateText = useMemo( - () => - debounce((inputText: string) => { + const validateText = useMemo( + () => + debounce((inputText: string) => { const urlMatch = URL_MATCHER.exec(inputText); if ( embedConfig != null && inputText != null && urlMatch != null ) { Promise.resolve(embedConfig.parseUrl(inputText)).then( parseResult => { setEmbedResult(parseResult); }, ); - } else if (embedResult != null) { - setEmbedResult(null); - } + } else { + setEmbedResult(null); + } }, 200), - [embedConfig, embedResult], + [embedConfig], ); + + useEffect( + () => () => { + // @ts-expect-error: optional cancel helper on debounced function + validateText.cancel?.(); + }, + [validateText], + );This keeps the behaviour but avoids stale timers and simplifies the dependency logic.
Also applies to: 246-265, 283-287
382-407: Minor styling nit: duplicated width definition and unused label styles.
AutoEmbededWrappersetswidth: 600px, and the JSX also passesstyle={{ width: '600px' }}. You can drop one of these to avoid duplication. Also,.Input__labelis defined but not used in this component; if there’s no caller markup using it, consider removing it to keep the styles focused.This is purely cosmetic; no behaviour impact.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx(1 hunks)
🔇 Additional comments (3)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx (3)
137-156: Content won't update ifhtmlprop changes after initialization.The
isInitializedflag prevents re-initialization whenhtmlchanges. If this viewer could receive updated content during its lifecycle, the content would become stale.If this is intentional (viewer is always unmounted/remounted for new content), you can ignore this. Otherwise, consider resetting
isInitializedwhenhtmlchanges, or using a ref to track the previoushtmlvalue.
198-216: LGTM!The styled components appropriately remove editor chrome for read-only mode and ensure links remain interactive.
185-190: TheRichTextPlugindoes not require aplaceholderprop. The placeholder prop is optional and defaults to null in @lexical/react v0.34.0. Omitting it will not cause console warnings or TypeScript errors. For a read-only viewer with content always present, this usage is correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx (1)
92-92: Regex captures only a single character instead of full content.The pattern
([^<])matches exactly one character. To capture all text content inside the span, use([^<]*)or([^<]+).Apply this diff:
- .replace(/<p[^>]*><span[^>]*>([^<])<\/span><\/p>/g, '<p>$1</p>') + .replace(/<p[^>]*><span[^>]*>([^<]*)<\/span><\/p>/g, '<p>$1</p>')
🧹 Nitpick comments (2)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx (2)
26-41: Consider extracting magic constants for default style values.The hardcoded values
'15px','rgb(255, 255, 255)', and'rgb(0, 0, 0)'could be extracted as named constants for better maintainability and to ensure consistency with your design system.Apply this diff to extract the constants:
+const DEFAULT_FONT_SIZE = '15px'; +const DEFAULT_BACKGROUND_COLOR = 'rgb(255, 255, 255)'; +const DEFAULT_TEXT_COLOR = 'rgb(0, 0, 0)'; + function getExtraStyles(element: HTMLElement): string { let extraStyles = ''; const fontSize = parseAllowedFontSize(element.style.fontSize); const backgroundColor = parseAllowedColor(element.style.backgroundColor); const color = parseAllowedColor(element.style.color); - if (fontSize !== '' && fontSize !== '15px') { + if (fontSize !== '' && fontSize !== DEFAULT_FONT_SIZE) { extraStyles += `font-size: ${fontSize};`; } - if (backgroundColor !== '' && backgroundColor !== 'rgb(255, 255, 255)') { + if (backgroundColor !== '' && backgroundColor !== DEFAULT_BACKGROUND_COLOR) { extraStyles += `background-color: ${backgroundColor};`; } - if (color !== '' && color !== 'rgb(0, 0, 0)') { + if (color !== '' && color !== DEFAULT_TEXT_COLOR) { extraStyles += `color: ${color};`; } return extraStyles; }
170-179: MemoizeinitialConfigandbuildImportMap()to prevent unnecessary re-initialization.The
initialConfigobject andbuildImportMap()call are recreated on every render, which may cause performance issues or unnecessary editor reconciliation.Apply this diff:
+import { useEffect, useState, useMemo } from 'react'; + +const importMap = buildImportMap(); + export default function RichTextLexicalViewer({ content, }: { content?: string; }) { if (!content) { return null; } - const initialConfig = { + const initialConfig = useMemo(() => ({ namespace: 'RichTextViewer', nodes: [...PlaygroundNodes], editable: false, // Read-only mode onError: (error: Error) => { console.error('Lexical viewer error:', error); }, theme: PlaygroundEditorTheme, - html: { import: buildImportMap() }, - }; + html: { import: importMap }, + }), []); return (Alternatively, if
buildImportMap()depends on dynamic state, memoize it separately:+const importMap = useMemo(() => buildImportMap(), []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx (3)
src/components/rich-text-lexical/plugins/ToolbarPlugin/fontSize.tsx (1)
parseAllowedFontSize(50-59)src/components/rich-text-lexical/ui/ColorPicker.tsx (1)
parseAllowedColor(23-25)src/components/rich-text-lexical/mainStyles.tsx (1)
EditorShell(670-1235)
🔇 Additional comments (2)
src/components/rich-text-lexical/RichTextLexicalViewer.tsx (2)
43-87: LGTM: DOM import map extension is correctly implemented.The function properly wraps TextNode's import map to inject additional inline styles during HTML import. The conditional logic correctly handles edge cases where the conversion output already has a node or special properties.
198-216: LGTM: Viewer styling is appropriate.The styled components properly remove editor chrome (borders, shadows) and ensure links remain clickable in read-only mode with
pointer-events: auto.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/rich-text-lexical/nodes/FigmaNode.tsx (3)
54-59: Consider adding sandbox attribute for defense-in-depth security.While Figma is a trusted domain, adding a
sandboxattribute provides an additional security layer by restricting iframe capabilities. This follows security best practices.Apply this diff to add the sandbox attribute:
<iframe width='560' height='315' src={iframeSrc} allowFullScreen={true} + sandbox="allow-scripts allow-same-origin allow-presentation" />
55-56: Consider making iframe dimensions configurable.The hardcoded dimensions (560x315) may not be responsive or suitable for all use cases. Consider making these configurable via props or using responsive sizing.
95-98: Consider validating documentID format.Adding validation to ensure the documentID matches expected Figma document ID patterns could prevent issues with malformed embeds.
Example validation pattern:
constructor(id: string, format?: ElementFormatType, key?: NodeKey) { super(format, key); if (!id || typeof id !== 'string') { throw new Error('FigmaNode requires a valid documentID'); } this.__id = id; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/rich-text-lexical/nodes/FigmaNode.tsx(1 hunks)
🔇 Additional comments (7)
src/components/rich-text-lexical/nodes/FigmaNode.tsx (7)
1-8: Verify license attribution is intentional.The Meta Platforms copyright header suggests this code is derived from Lexical's playground examples. Since you mentioned removing "unused/odd code from the playground example" in the PR comments, ensure this attribution is intentional and complies with the MIT license terms.
9-26: LGTM!Imports are clean, properly typed, and all necessary for the implementation.
28-36: LGTM!Type definition is well-structured with appropriate use of Readonly for props.
64-69: LGTM!Serialization type is properly defined and follows Lexical conventions.
164-184: LGTM! Robust URL parsing with proper error handling.The function correctly handles various Figma URL formats and gracefully returns null for invalid input. The try-catch ensures no runtime errors from malformed URLs.
186-199: LGTM!DOM conversion logic is correct with proper fallback handling and null returns for non-Figma content.
201-209: LGTM!Factory and type guard functions follow Lexical conventions and are correctly implemented.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/rich-text-lexical/plugins/ComponentPickerPlugin/index.tsx (1)
289-306: Duplicate functionality: "Date" and "Today" are identical.Both options create a new
Date()set to midnight and dispatch the same command with the same payload. Consider removing one to reduce menu clutter.- new ComponentPickerOption('Date', { - icon: <i className='icon calendar' />, - keywords: ['date', 'calendar', 'time'], - onSelect: () => { - const dateTime = new Date(); - dateTime.setHours(0, 0, 0, 0); // Set time to midnight - editor.dispatchCommand(INSERT_DATETIME_COMMAND, { dateTime }); - }, - }), new ComponentPickerOption('Today', { icon: <i className='icon calendar' />, - keywords: ['date', 'calendar', 'time', 'today'], + keywords: ['date', 'calendar', 'time', 'today', 'now'], onSelect: () => { const dateTime = new Date(); dateTime.setHours(0, 0, 0, 0); // Set time to midnight editor.dispatchCommand(INSERT_DATETIME_COMMAND, { dateTime }); }, }),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/rich-text-lexical/plugins/ComponentPickerPlugin/index.tsx(1 hunks)
🔇 Additional comments (2)
src/components/rich-text-lexical/plugins/ComponentPickerPlugin/index.tsx (2)
391-486: Component structure looks good overall.The plugin correctly integrates with Lexical's typeahead system, properly memoizes options and callbacks, and uses portal rendering for the popover. The modal integration and option filtering logic are well-structured.
137-141: No changes needed. TheINSERT_TABLE_COMMANDcorrectly expects string values forrowsandcolumns, and the implementation properly passes them as strings captured from the regex pattern.
src/components/rich-text-lexical/plugins/ComponentPickerPlugin/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (3)
261-267: PassmaxLengthtomaxLimit, notminLimit.Line 263 passes
maxLengthasminLimit, which inverts the constraint. The counter will trigger an error when the content is below the maximum length instead of above it.🔎 Proposed fix
{maxLength && ( <RichTextCounter - minLimit={maxLength} + maxLimit={maxLength} value={currentValue} setHasLimitError={setHasLimitError} /> )}
274-274: Fix error rendering: avoid shadowingError, safely extract error message, and use semantic element.Current issues:
- Line 283:
const Errorshadows the globalErrorconstructor (also flagged by static analysis)- Line 274: Casting
error as stringwill render"[object Object]"forFieldErrorobjects- Using
styled(GLink)is semantically incorrect for error messages🔎 Proposed fix
- {error && <Error>{error as string}</Error>} + {error ? ( + <ErrorText as='div' role='alert'> + {typeof error === 'string' + ? error + : (error as FieldError)?.message} + </ErrorText> + ) : null} </ToolbarContext> </TableContext> </SharedHistoryContext> </LexicalComposer> </FlashMessageContext> ); } -const Error = styled(GLink)` +const ErrorText = styled(GLink)` color: ${semanticColors.punch[500]}; margin-top: 4px; `;Also applies to: 283-286
54-100: Fix CSS style concatenation to prevent malformed style strings.Line 86 concatenates
textNode.getStyle() + extraStyleswithout ensuring a semicolon separator. If the existing style doesn't end with;, the result will be invalid CSS (e.g.,color: rgb(0,0,0)font-size: 12px;), breaking text formatting.🔎 Proposed fix
if ($isTextNode(textNode)) { - textNode.setStyle( - textNode.getStyle() + extraStyles, - ); + const prev = textNode.getStyle(); + const needsSemi = + prev && !prev.trim().endsWith(';'); + textNode.setStyle( + prev + ? `${prev}${needsSemi ? ';' : ''}${extraStyles}` + : extraStyles, + ); }
🧹 Nitpick comments (1)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (1)
199-206: Remove redundantroot.clear()call.Line 201 clears the root, but line 205 clears it again before appending nodes, making the first call unnecessary.
🔎 Proposed cleanup
editor.update(() => { const root = $getRoot(); - root.clear(); const fixedNodes = parseHtmlToLexicalNodes(editor, html); root.clear(); root.append(...fixedNodes); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx(1 hunks)src/components/rich-text-lexical/RichTextLexicalViewer.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/rich-text-lexical/RichTextLexicalViewer.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (7)
src/components/rich-text-lexical/plugins/ToolbarPlugin/fontSize.tsx (1)
parseAllowedFontSize(50-59)src/components/rich-text-lexical/ui/ColorPicker.tsx (1)
parseAllowedColor(23-25)src/components/rich-text-lexical/context/SettingsContext.tsx (1)
useSettings(57-59)src/components/rich-text-lexical/context/FlashMessageContext.tsx (1)
FlashMessageContext(35-61)src/components/rich-text-lexical/context/SharedHistoryContext.tsx (1)
SharedHistoryContext(21-33)src/components/rich-text-lexical/plugins/TablePlugin.tsx (1)
TableContext(55-82)src/wagmiConfigs.ts (1)
projectId(10-10)
🪛 Biome (2.1.2)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx
[error] 283-283: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (3)
54-100: Fix CSS concatenation to prevent malformed style strings.Lines 85-86 concatenate styles using
textNode.getStyle() + extraStyles, which produces invalid CSS when the existing style doesn't end with a semicolon (e.g.,"color: rgb(0,0,0)font-size: 12px;").🔎 Proposed fix
if ($isTextNode(textNode)) { + const prev = textNode.getStyle(); + const needsSemi = + prev && !prev.trim().endsWith(';'); textNode.setStyle( - textNode.getStyle() + extraStyles, + prev + ? `${prev}${needsSemi ? ';' : ''}${extraStyles}` + : extraStyles, ); }
261-267: Critical: PassmaxLengthtomaxLimit, notminLimit.Line 263 passes
maxLengthtominLimit, which inverts the constraint logic. TheRichTextCountercomponent checks if count exceedsmaxLimitor falls belowminLimit. With this configuration, the error triggers when count is below the maximum instead of above it.🔎 Proposed fix
{maxLength && ( <RichTextCounter - minLimit={maxLength} + maxLimit={maxLength} value={currentValue} setHasLimitError={setHasLimitError} /> )}
274-274: Fix error rendering: avoid shadowingError, handle FieldError properly, and use semantic markup.Multiple issues with error handling:
- Line 283: The styled component name
Errorshadows the globalErrorconstructor (also flagged by Biome lint).- Line 274: Casting
error as stringproduces"[object Object]"when error is aFieldError.- Line 283: Using
styled(GLink)implies link semantics for an error message, which is incorrect.🔎 Proposed fix
- {error && <Error>{error as string}</Error>} + {error ? ( + <ErrorText as='div' role='alert'> + {typeof error === 'string' + ? error + : (error as FieldError)?.message} + </ErrorText> + ) : null} </ToolbarContext> </TableContext> </SharedHistoryContext> </LexicalComposer> </FlashMessageContext> ); } -const Error = styled(GLink)` +const ErrorText = styled(GLink)` color: ${semanticColors.punch[500]}; margin-top: 4px; `;Also applies to: 283-286
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (10)
src/components/rich-text-lexical/plugins/ToolbarPlugin/fontSize.tsx (1)
parseAllowedFontSize(50-59)src/components/rich-text-lexical/ui/ColorPicker.tsx (1)
parseAllowedColor(23-25)src/components/rich-text-lexical/context/SettingsContext.tsx (1)
useSettings(57-59)src/components/rich-text-lexical/context/FlashMessageContext.tsx (1)
FlashMessageContext(35-61)src/components/rich-text-lexical/context/SharedHistoryContext.tsx (1)
SharedHistoryContext(21-33)src/components/rich-text-lexical/plugins/TablePlugin.tsx (1)
TableContext(55-82)src/components/rich-text-lexical/context/ToolbarContext.tsx (1)
ToolbarContext(94-130)src/components/rich-text-lexical/mainStyles.tsx (1)
EditorShell(670-1235)src/components/rich-text-lexical/Editor.tsx (1)
Editor(81-297)src/components/rich-text-lexical/plugins/TypingPerfPlugin/index.ts (1)
TypingPerfPlugin(33-126)
🪛 Biome (2.1.2)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx
[error] 283-283: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (6)
src/components/rich-text-lexical/RichTextLexicalEditor.tsx (6)
35-52: LGTM: Clean style extraction with proper validation.The function correctly extracts and validates inline styles, filters out defaults, and constructs well-formed CSS strings.
104-117: LGTM: Clean onChange handler implementation.The component correctly wraps the OnChangePlugin, safely reads editor state, and conditionally renders based on prop presence.
120-168: LGTM: Robust HTML parsing with proper orphan node handling.The function includes comprehensive HTML cleanup, correctly handles multi-character spans (line 124 now uses
([^<]*)), and elegantly wraps orphan inline nodes into paragraphs using a buffer-based approach.
171-189: LGTM: Well-structured initialization with error handling.The function includes proper guards against re-initialization, comprehensive error handling, and a sensible fallback to plain text when HTML parsing fails.
242-251: LGTM: Proper initial config with conditional editor state.The configuration correctly sets
editorStateto null for collab mode and uses the initializer for non-collab, with proper HTML import mapping and error handling.
269-271: LGTM: Conditional initialization prevents double-loading.The conditional rendering of
EditorInitializeronly for collab mode properly addresses the double-initialization issue, ensuring clean editor setup without unnecessary re-parsing.
Summary by CodeRabbit
New Features
Collaboration
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.