[WEB-4325] chore: additional changes for mobile editor support #7212
Conversation
WalkthroughThis update introduces comprehensive support for mobile-specific features in the editor, including new props, types, and conditional UI logic for mobile rendering. It enhances editor APIs with new commands, selection, and attribute retrieval methods, and adds a link menu item. CSS changes introduce mobile font and spacing variants, and placeholder rules are adjusted for consistency across platforms. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorComponent
participant PageRenderer
participant EditorContainer
participant CustomImageNode
participant ImageBlock
participant Toolbar
User->>EditorComponent: Open editor (mobile or desktop)
EditorComponent->>PageRenderer: Pass isMobile, onEditorClick, etc.
PageRenderer->>EditorContainer: Pass isMobile
PageRenderer->>CustomImageNode: Pass isMobile for image rendering
CustomImageNode->>ImageBlock: Pass isMobile for image interaction
ImageBlock->>Toolbar: Pass showExternalLink = !isMobile
User->>EditorComponent: Interact (drag-drop, click, etc.)
EditorComponent->>EditorContainer: Enable/disable features based on isMobile and isDragDropEnabled
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
1akhanBaheti
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
packages/editor/src/core/components/editors/editor-content.tsx (1)
4-10: 🛠️ Refactor suggestionIncorrect
onClicktype constrains legitimate handlersReact’s click handler receives a
MouseEvent, but the type here omits that parameter, preventing callers from accessing the event object.- onClick?: () => void; + onClick?: React.MouseEventHandler<HTMLDivElement>;
🧹 Nitpick comments (10)
packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (1)
15-20: Provide default forshowExternalLinkto avoidundefineddownstream
ImageFullScreenActionexpects a boolean; forwardingundefinedcould produce unintended UI states. Supply a default:-type Props = { +type Props = { ... - showExternalLink: boolean; + showExternalLink?: boolean; };And inside the component:
-const { containerClassName, image, showExternalLink } = props; +const { containerClassName, image, showExternalLink = false } = props;Also applies to: 30-34
packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
85-89: Guard against undefined when passing flag toSideMenuExtension.After adding the default suggested above this becomes safe, but at the moment
dragDropEnabledmay receiveundefined.- dragDropEnabled: isDragDropEnabled, + dragDropEnabled: !!isDragDropEnabled,A coercion makes the intent explicit and prevents accidental “truthy / falsy” surprises.
packages/editor/src/core/components/editors/editor-container.tsx (2)
19-24: MarkisMobileoptional or give it a default.
isMobileis required by theEditorContainerPropsinterface, but almost every existing desktop call-site will now need to be updated.
If the majority of usages are desktop-only, consider:- isMobile: boolean; + isMobile?: boolean; // defaults to falseand within the component
- const { …, isMobile } = props; + const { …, isMobile = false } = props;This avoids a breaking change across the codebase.
98-101: Early-return instead of fragment for cleaner DOM.You already gate
LinkViewContaineron!isMobile. Doing the same for the wrapper could avoid an extra React fragment:- {children} - {!isMobile && <LinkViewContainer … />} + {children} + {!isMobile && <LinkViewContainer … />}No functional change, purely a readability/DOM-tidiness tweak.
packages/editor/src/core/extensions/custom-image/custom-image.tsx (2)
45-50: Minor:isMobileleaks into every extension instance even when unused.Passing the flag is fine, but consider early-returning the mobile-specific props from
addNodeView()only whenisMobileistrueto avoid unnecessary prop bloat in desktop renders.
178-179: Type safety – tighten prop typing for node view factory.
CustomImageNodePropsalready includesisMobile, so the cast is redundant:- return ReactNodeViewRenderer((props: CustomImageNodeProps) => <CustomImageNode {...props} isMobile={isMobile} />); + return ReactNodeViewRenderer((props) => <CustomImageNode {...props} isMobile={isMobile} />);Keeps the generic closer to the actual value and avoids an unnecessary import.
packages/editor/src/core/types/config.ts (1)
19-20:isMobilebelongs in a dedicated mobile-config structure.Long-term, adding mobile-specific flags to generic handler types risks clutter.
Consider grouping all mobile-only knobs into a singlemobile?: { … }object for clearer intent.packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1)
53-60: Missingeditorin effect dependency listThe async
getImageSourceeffect dereferenceseditor, buteditorisn’t listed in the dependency array.
If the editor instance is ever replaced (e.g. during collaborative re-initialisation) the effect will not re-run, leavingresolvedSrcstale.- }, [imgNodeSrc]); + }, [editor, imgNodeSrc]);packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (2)
213-228:imgelement missingalttextFor accessibility the fullscreen image should carry an
altattribute (even if empty) to avoid screen-reader confusion.- <img + <img + alt=""
268-279:showExternalLinkflag name vs. behaviourThe conditional block renders a fullscreen toggle, not an external-link opener; the naming is mildly confusing. Consider renaming the prop to
showFullScreenToggle(or similar) to match intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx(3 hunks)packages/editor/src/core/components/editors/document/page-renderer.tsx(2 hunks)packages/editor/src/core/components/editors/document/read-only-editor.tsx(3 hunks)packages/editor/src/core/components/editors/editor-container.tsx(2 hunks)packages/editor/src/core/components/editors/editor-content.tsx(1 hunks)packages/editor/src/core/components/editors/editor-wrapper.tsx(4 hunks)packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx(2 hunks)packages/editor/src/core/components/menus/menu-items.ts(5 hunks)packages/editor/src/core/extensions/custom-image/components/image-block.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/components/image-node.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx(6 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/custom-image.tsx(3 hunks)packages/editor/src/core/helpers/editor-commands.ts(1 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(2 hunks)packages/editor/src/core/hooks/use-editor.ts(9 hunks)packages/editor/src/core/types/collaboration.ts(1 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/core/types/editor.ts(8 hunks)packages/editor/src/styles/editor.css(0 hunks)packages/editor/src/styles/variables.css(3 hunks)packages/editor/tsup.config.ts(1 hunks)space/styles/editor.css(1 hunks)space/styles/globals.css(1 hunks)web/styles/editor.css(1 hunks)web/styles/globals.css(1 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/styles/editor.css
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/editor/src/core/components/menus/menu-items.ts (1)
packages/editor/src/core/helpers/editor-commands.ts (2)
setLinkEditor(142-157)unsetLinkEditor(138-140)
packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
packages/editor/src/core/components/editors/editor-content.tsx (1)
EditorContentWrapper(12-26)
packages/editor/src/core/extensions/custom-image/custom-image.tsx (1)
packages/editor/src/core/extensions/custom-image/components/image-node.tsx (2)
CustomImageNodeProps(21-21)CustomImageNode(23-96)
packages/editor/src/core/types/editor.ts (3)
packages/editor/src/core/types/config.ts (2)
TDisplayConfig(28-33)TFileHandler(7-20)packages/editor/src/core/types/extensions.ts (1)
TExtensions(1-1)packages/editor/src/core/types/mention.ts (1)
TMentionHandler(26-28)
🔇 Additional comments (25)
packages/editor/tsup.config.ts (1)
7-7: Approve addingclean: truefor fresh build outputs.Enabling
cleanensures stale artifacts are removed before each build, which helps avoid inconsistent bundles in the mobile editor package.space/styles/globals.css (1)
2-2: Confirmeditor.cssimport precedence and path
The URL import foreditor.cssis placed before Tailwind directives to ensure editor-specific styles load globally. Verify that the path resolves correctly in the production CSS build.web/styles/globals.css (1)
2-2: Confirmeditor.cssimport for web global styles
This URL import brings in the editor-specific stylesheet; ensure the build pipeline includes and resolves this import for the web environment.space/styles/editor.css (1)
1-8: Placeholder styling for empty paragraphs
The.ProseMirror p.is-empty::beforerule correctly usescontent: attr(data-placeholder)and--color-placeholder. This aligns with the new placeholder behavior—looks good.web/styles/editor.css (1)
1-8: Placeholder styling for.ProseMirror p.is-empty
This new rule mirrors the implementation inspace/styles/editor.cssand properly displays placeholders using thedata-placeholderattribute and--color-placeholder.packages/editor/src/styles/variables.css (2)
91-111: Introduce.mobile-fontvariant for mobile editing
The new.mobile-fontclass defines mobile-optimized font sizes and line heights. Confirm that the editor components apply this class whenisMobileis true.
170-190: Add.line-spacing-mobile-regularspacing scheme
Mobile-specific line spacing is introduced here. Validate that this class is applied in mobile contexts and that its padding values match the design guidelines.packages/editor/src/core/types/collaboration.ts (1)
41-46:✅ Verification successful
isDragDropEnabled/isMobileshould be optional or have sane defaultsMaking both flags mandatory forces every caller to update immediately and may break downstream build-time contracts. Unless the intent is to require explicit opt-in on every usage, declare them as optional with a default value in the consuming component/hook.
- isDragDropEnabled: boolean; - isMobile: boolean; + /** + * Enables drag-drop in the editor. Defaults to `true` on desktop. + */ + isDragDropEnabled?: boolean; + /** + * Marks the editor as running in a mobile context. Defaults to `false`. + */ + isMobile?: boolean;Ensure all hooks/components that consume these props provide fallback values so behaviour stays unchanged for existing callers.
🏁 Script executed:
#!/bin/bash set -e echo "🔍 Searching for usages of isDragDropEnabled:" rg --color=never -n "isDragDropEnabled" -A3 echo -e "\n🔍 Searching for usages of isMobile:" rg --color=never -n "isMobile" -A3Length of output: 83449
🏁 Script executed:
#!/bin/bash # Display the beginning of the hook file to inspect its signature sed -n '1,60p' packages/editor/src/core/hooks/use-collaborative-editor.tsLength of output: 1998
Here’s the head of
docs/packages/editor/src/core/components/editors/document/collaborative-editor.tsxshowing prop defaults and hook invocation:export const CollaborativeEditor: React.FC<TCollaborativeEditorProps> = ({ id, isDragDropEnabled = true, isMobile = false, mentionHandler, placeholder, onTransaction, // …other props }) => { const editor = useCollaborativeEditor({ id, isDragDropEnabled, isMobile, mentionHandler, placeholder, onTransaction, // …forward remaining props }); // … };And the hook signature:
export const useCollaborativeEditor = (props: TCollaborativeEditorProps) => { const { // …, isDragDropEnabled, isMobile, // … } = props; // … Collaboration.configure({ dragDropEnabled: isDragDropEnabled /*bool*/ }); };Since every consuming component (right now only
CollaborativeEditor) already provides defaults (true/false), making these flags optional in the type is safe: callers that omit them will get the intended defaults.Proposed change:
in packages/editor/src/core/types/collaboration.ts - isDragDropEnabled: boolean; - isMobile: boolean; + /** + * Enables drag-drop in the editor. Defaults to `true` when not provided. + */ + isDragDropEnabled?: boolean; + /** + * Marks the editor as running in a mobile context. Defaults to `false` when not provided. + */ + isMobile?: boolean;No further updates needed—defaults live in
CollaborativeEditor.packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx (1)
23-26: Prop type drift – confirmIReadOnlyEditorProps&EditorContaineracceptisMobile
isMobileis now threaded through but the source type comes from@/types. If that interface wasn’t updated, TS will error. Likewise,EditorContainermust declareisMobile?: boolean. Double-check both points to avoid compilation failures.Also applies to: 45-50
packages/editor/src/core/components/editors/document/read-only-editor.tsx (1)
33-34: Mobile flag plumbing looks good – mind the optional chainPassing
isMobile={isMobile}throughPageRendereris consistent with other components – nice.Just verify
PageRenderermakes the prop optional (isMobile?: boolean) and sets a default to maintain backwards compatibility.Also applies to: 49-50, 82-89
packages/editor/src/core/components/editors/editor-content.tsx (1)
16-22: Good focus handling & new CSS hookThe extra
"editor-content"class andonFocuschain keep UX intact. LGTM.packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
14-34:❌ Incorrect review comment
Provide a safe default for
isDragDropEnabled.
isDragDropEnabledis taken straight from props without a default.
If the caller forgets to pass it,undefinedwill reachSideMenuExtension, which most likely expects a boolean and could break its internal checks.- isDragDropEnabled, + isDragDropEnabled = true,This keeps the previous behaviour intact while still allowing the flag to be explicitly switched off.
Also applies to: 27-28
🏁 Script executed:
#!/bin/bash set -e # Locate the prop interface to see if isDragDropEnabled is optional or required rg "interface TCollaborativeEditorProps" -A5 -n packages/editor/src # Locate SideMenuExtension and inspect its constructor/options for drag-and-drop defaults rg "class SideMenuExtension" -A15 -n packages/editor/src # Find all references to isDragDropEnabled in the codebase rg "isDragDropEnabled" -n packages/editor/srcLength of output: 78
🏁 Script executed:
#!/bin/bash set -e # Search for the props type to see if isDragDropEnabled is optional or has a default rg "TCollaborativeEditorProps" -n . # Find any occurrences of isDragDropEnabled and its usage rg "isDragDropEnabled" -n . # Locate SideMenuExtension declaration to inspect its constructor or default options rg "class SideMenuExtension" -n .Length of output: 1069
No default needed for
isDragDropEnabledin the hook
isDragDropEnabledis declared as a required boolean inTCollaborativeEditorPropsand is already defaulted totruein theCollaborativeEditorcomponent, so it will never beundefinedwhen passed intouseCollaborativeEditor.•
packages/editor/src/core/types/collaboration.ts:41–isDragDropEnabled: boolean;
•packages/editor/src/core/components/editors/document/collaborative-editor.tsx:35– destructured asisDragDropEnabled = trueLikely an incorrect or invalid review comment.
packages/editor/src/core/types/config.ts (1)
24-27: Confirm CSS tokens for new enum values exist.
"mobile-font"and"mobile-regular"are now valid class tokens.
Make sure corresponding CSS variables/classes were added, otherwise consumers will silently fall back to default styles.packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
20-31: Mobile gating & click-through wiring look solid
isMobiledefaults tofalse, is forwarded toEditorContainer, and correctly hides menus on mobile.
onEditorClickis bubbled through toEditorContentWrapper. No issues spotted.packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
121-132: Good call deferring automatic file-picker on mobileThe extra
!isMobileguard prevents the instant picker pop-up that mobile Safari/Chrome would trigger, yet still flagshasOpenedFileInputOnceso the component won’t spam subsequent mounts. Behaviour looks correct.packages/editor/src/core/components/editors/editor-wrapper.tsx (1)
25-34: Good addition of configurable props with sensible defaults.The new props (
editable,isMobile,onInitialContentLoad) enhance the component's flexibility and enable mobile-specific behavior. The default values ensure backward compatibility.Also applies to: 45-55, 78-78
packages/editor/src/core/extensions/custom-image/components/image-block.tsx (3)
203-212: Appropriate mobile-specific behavior for image interactions.The addition of
preventDefaultand conditional blur on mobile devices helps prevent unwanted default behaviors and improves the user experience when interacting with images on touch devices.
303-303: Appropriate conditional rendering for mobile UI.Hiding the external link option on mobile devices is a good UX decision given the limited screen space.
263-269:✅ Verification successful
Mobile-specific image source resolution addresses cross-origin limitations.
The implementation correctly handles the mobile platform limitation where authentication cookies cannot be passed in cross-origin requests. The async resolution with proper error handling is well implemented.
Verify that the
getImageSourcecommand is available in all editor configurations where this component is used:
🏁 Script executed:
#!/bin/bash # Description: Verify the getImageSource command implementation and usage # Search for getImageSource command definition ast-grep --pattern $'commands.getImageSource' # Search for image extension configurations that might need this command rg -A 5 "CustomImageExtension|ImageExtension" --type tsLength of output: 16377
🏁 Script executed:
#!/bin/bash # Search for any references to getImageSource across the codebase rg --color=never -n "getImageSource" -C 3 packages/editor/src # Inspect CustomImageExtension to see if it registers a getImageSource command rg --color=never -n "addCommands" -C 5 packages/editor/src/core/extensions/custom-image/custom-image.tsxLength of output: 6200
getImageSource is correctly registered and your mobile logic is solid
The
getImageSourcecommand is implemented in both the coreImageExtension(and its read-only variant) as well as inCustomImageExtension. Your async resolution and error handling for mobile-only image loading are appropriate and cover the cross-origin cookie limitation.No further changes needed—this is good to merge.
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (2)
46-58: Well-implemented extensions configuration with proper memoization.The use of
useMemoto memoize the extensions array is appropriate, and the conditional inclusion ofWorkItemEmbedExtensionbased onembedHandler.issueis correctly implemented with the right dependencies.
24-44: Proper prop handling and forwarding to child components.The new props are correctly destructured and forwarded to the appropriate child components, maintaining a clean component interface and enabling mobile-specific behaviors throughout the editor stack.
Also applies to: 65-66, 72-73, 105-106
packages/editor/src/core/hooks/use-editor.ts (2)
163-172: Good additions to the editor API.The new undo/redo methods and enhanced focus/scroll functionality provide useful editor control capabilities. The implementations properly handle edge cases with null checks.
353-357: Well-implemented attribute retrieval method.The
getSelectedNodeAttributesmethod correctly extends the mark range for links before retrieving attributes, ensuring complete link selection.packages/editor/src/core/types/editor.ts (2)
46-46: Proper typing for the new link command.The link command is correctly added to
TEditorCommandswith appropriate props including requiredurland optionaltext.Also applies to: 70-73
105-133: Comprehensive type definitions for enhanced editor capabilities.The extended
EditorRefApiinterface and updated editor prop interfaces properly type all the new functionality including:
- Selection and focus control methods
- Mobile support flags
- Drag-drop configuration
- Initial content load callbacks
The types align well with the implementations across the codebase.
Also applies to: 140-179
packages/editor/src/core/extensions/custom-image/components/image-node.tsx
Show resolved
Hide resolved
| createSelectionAtCursorPosition: () => { | ||
| if (!editor) return; | ||
| const { empty } = editor.state.selection; | ||
|
|
||
| // if (empty) return null; | ||
| if (empty) { | ||
| // Get the text content and position info | ||
| const { $from } = editor.state.selection; | ||
| const textContent = $from.parent.textContent; | ||
| const posInNode = $from.parentOffset; | ||
|
|
||
| // Find word boundaries | ||
| let start = posInNode; | ||
| let end = posInNode; | ||
|
|
||
| // Move start position backwards until we hit a word boundary | ||
| while (start > 0 && /\w/.test(textContent[start - 1])) { | ||
| start--; | ||
| } | ||
|
|
||
| // Move end position forwards until we hit a word boundary | ||
| while (end < textContent.length && /\w/.test(textContent[end])) { | ||
| end++; | ||
| } | ||
|
|
||
| // If we found a word, select it using editor commands | ||
| if (start !== end) { | ||
| const from = $from.start() + start; | ||
| const to = $from.start() + end; | ||
| editor.commands.setTextSelection({ from, to }); | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve word boundary detection for international text support.
The current regex pattern /\w/ only matches ASCII word characters (a-zA-Z0-9_) and won't properly handle international characters, emojis, or special characters.
Consider using a more inclusive pattern or Unicode-aware word boundary detection:
-while (start > 0 && /\w/.test(textContent[start - 1])) {
+while (start > 0 && /[\p{L}\p{N}_]/u.test(textContent[start - 1])) {
start--;
}
-while (end < textContent.length && /\w/.test(textContent[end])) {
+while (end < textContent.length && /[\p{L}\p{N}_]/u.test(textContent[end])) {
end++;
}This uses Unicode property escapes to match any letter (\p{L}) or number (\p{N}) character across all languages.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| createSelectionAtCursorPosition: () => { | |
| if (!editor) return; | |
| const { empty } = editor.state.selection; | |
| // if (empty) return null; | |
| if (empty) { | |
| // Get the text content and position info | |
| const { $from } = editor.state.selection; | |
| const textContent = $from.parent.textContent; | |
| const posInNode = $from.parentOffset; | |
| // Find word boundaries | |
| let start = posInNode; | |
| let end = posInNode; | |
| // Move start position backwards until we hit a word boundary | |
| while (start > 0 && /\w/.test(textContent[start - 1])) { | |
| start--; | |
| } | |
| // Move end position forwards until we hit a word boundary | |
| while (end < textContent.length && /\w/.test(textContent[end])) { | |
| end++; | |
| } | |
| // If we found a word, select it using editor commands | |
| if (start !== end) { | |
| const from = $from.start() + start; | |
| const to = $from.start() + end; | |
| editor.commands.setTextSelection({ from, to }); | |
| } | |
| } | |
| }, | |
| createSelectionAtCursorPosition: () => { | |
| if (!editor) return; | |
| const { empty } = editor.state.selection; | |
| // if (empty) return null; | |
| if (empty) { | |
| // Get the text content and position info | |
| const { $from } = editor.state.selection; | |
| const textContent = $from.parent.textContent; | |
| const posInNode = $from.parentOffset; | |
| // Find word boundaries | |
| let start = posInNode; | |
| let end = posInNode; | |
| // Move start position backwards until we hit a word boundary | |
| - while (start > 0 && /\w/.test(textContent[start - 1])) { | |
| + while (start > 0 && /[\p{L}\p{N}_]/u.test(textContent[start - 1])) { | |
| start--; | |
| } | |
| // Move end position forwards until we hit a word boundary | |
| - while (end < textContent.length && /\w/.test(textContent[end])) { | |
| + while (end < textContent.length && /[\p{L}\p{N}_]/u.test(textContent[end])) { | |
| end++; | |
| } | |
| // If we found a word, select it using editor commands | |
| if (start !== end) { | |
| const from = $from.start() + start; | |
| const to = $from.start() + end; | |
| editor.commands.setTextSelection({ from, to }); | |
| } | |
| } | |
| }, |
🤖 Prompt for AI Agents
In packages/editor/src/core/hooks/use-editor.ts between lines 173 and 205, the
current word boundary detection uses the regex /\w/, which only matches ASCII
characters and fails for international characters and emojis. Replace the regex
with a Unicode-aware pattern using Unicode property escapes, such as
/\p{L}|\p{N}/u, to correctly identify letters and numbers across all languages.
Update both the backward and forward boundary checks to use this new regex for
improved international text support.
| const onEditorContentLoad = useCallback(() => { | ||
| const editorContainer = document.querySelector(".tiptap"); | ||
| if ( | ||
| editorContainer && | ||
| '<p class="editor-paragraph-block"></p>' !== editorContainer.innerHTML && | ||
| !isInitialContentLoaded && | ||
| onInitialContentLoad | ||
| ) { | ||
| setIsInitialContentLoaded(true); | ||
| onInitialContentLoad?.(editorContainer.clientHeight); | ||
| } | ||
| }, [onInitialContentLoad, isInitialContentLoaded]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving the initial content load detection logic.
The current implementation has potential issues:
- The hardcoded
.tiptapclass selector is fragile and could break if the editor's internal structure changes - The innerHTML comparison with a specific paragraph format is brittle
Consider using a more robust approach:
-const editorContainer = document.querySelector(".tiptap");
-if (
- editorContainer &&
- '<p class="editor-paragraph-block"></p>' !== editorContainer.innerHTML &&
- !isInitialContentLoaded &&
- onInitialContentLoad
-) {
+const editorElement = editor?.view?.dom;
+if (
+ editorElement &&
+ editor?.state.doc.textContent.trim() !== "" &&
+ !isInitialContentLoaded &&
+ onInitialContentLoad
+) {
setIsInitialContentLoaded(true);
- onInitialContentLoad?.(editorContainer.clientHeight);
+ onInitialContentLoad?.(editorElement.clientHeight);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/editor/src/core/hooks/use-editor.ts around lines 83 to 94, the
current logic for detecting initial content load relies on a hardcoded '.tiptap'
selector and a brittle innerHTML string comparison. To fix this, replace the
selector with a more stable reference to the editor container, such as a ref or
a prop if available, and instead of comparing innerHTML, use a more reliable
method like checking if the editor's content length or text content is
non-empty. This will make the detection more robust against structural changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/editor/src/core/extensions/custom-image/read-only-custom-image.tsx (1)
75-77: Shadowedpropsidentifier – rename for clarity
The inner arrow function parameter shadows the outerprops. While not technically wrong, it’s an easy readability foot-gun.-return ReactNodeViewRenderer((props: CustomImageNodeProps) => <CustomImageNode {...props} isMobile={isMobile} />); +return ReactNodeViewRenderer( + (nodeViewProps: CustomImageNodeProps) => ( + <CustomImageNode {...nodeViewProps} isMobile={isMobile} /> + ), +);packages/editor/src/core/extensions/image/read-only-image.tsx (1)
33-35: Avoid shadowing outerprops
Replicate the renaming shown in the Custom-Image counterpart to keep consistency and avoid confusion.-return ReactNodeViewRenderer((props: CustomImageNodeProps) => <CustomImageNode {...props} isMobile={isMobile} />); +return ReactNodeViewRenderer( + (nodeViewProps: CustomImageNodeProps) => ( + <CustomImageNode {...nodeViewProps} isMobile={isMobile} /> + ), +);packages/editor/src/core/extensions/image/extension.tsx (1)
59-61: Consistent param naming
Same param-shadowing nitpick; rename the innerprops.-return ReactNodeViewRenderer((props: CustomImageNodeProps) => <CustomImageNode {...props} isMobile={isMobile} />); +return ReactNodeViewRenderer( + (nodeViewProps: CustomImageNodeProps) => ( + <CustomImageNode {...nodeViewProps} isMobile={isMobile} /> + ), +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/editor/src/core/components/menus/menu-items.ts(6 hunks)packages/editor/src/core/extensions/custom-image/read-only-custom-image.tsx(2 hunks)packages/editor/src/core/extensions/image/extension.tsx(3 hunks)packages/editor/src/core/extensions/image/read-only-image.tsx(2 hunks)packages/editor/src/core/helpers/editor-commands.ts(1 hunks)packages/editor/src/core/types/config.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/editor/src/core/helpers/editor-commands.ts
- packages/editor/src/core/types/config.ts
- packages/editor/src/core/components/menus/menu-items.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/editor/src/core/extensions/custom-image/read-only-custom-image.tsx (2)
packages/editor/src/core/types/config.ts (1)
TReadOnlyFileHandler(1-6)packages/editor/src/core/extensions/custom-image/components/image-node.tsx (2)
CustomImageNodeProps(21-21)CustomImageNode(23-96)
packages/editor/src/core/extensions/image/read-only-image.tsx (2)
packages/editor/src/core/types/config.ts (1)
TReadOnlyFileHandler(1-6)packages/editor/src/core/extensions/custom-image/components/image-node.tsx (2)
CustomImageNodeProps(21-21)CustomImageNode(23-96)
packages/editor/src/core/extensions/image/extension.tsx (1)
packages/editor/src/core/extensions/custom-image/components/image-node.tsx (2)
CustomImageNodeProps(21-21)CustomImageNode(23-96)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/editor/src/core/extensions/custom-image/read-only-custom-image.tsx (1)
12-13:isMobileaddition looks correct – confirm all call-sites
isMobileis plumbed through cleanly here, but double-check every place whereCustomReadOnlyImageExtensionis instantiated so that new call-sites don’t silently default tofalsewhen mobile behaviour is actually required.packages/editor/src/core/extensions/image/read-only-image.tsx (1)
9-10: Prop drilling check
Same remark as above: ensure every creator ofReadOnlyImageExtensionnow feeds the correctisMobileflag – missing it will quietly fall back to desktop rendering.
| isMobile = false, | ||
| } = fileHandler; | ||
|
|
There was a problem hiding this comment.
Potential type-safety gap – does TFileHandler include isMobile?
isMobile is destructured from fileHandler, but the older TFileHandler definition historically didn’t expose that field. If the type hasn’t been updated, this will raise a compile-time error or force an any escape.
Make sure TFileHandler mirrors the new shape, e.g.:
export type TFileHandler = {
getAssetSrc: (path: string) => Promise<string>;
validation: { maxFileSize: number };
// NEW
isMobile?: boolean;
};🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/image/extension.tsx around lines 18 to
20, the isMobile property is destructured from fileHandler but may not be
defined in the TFileHandler type, causing type-safety issues. Update the
TFileHandler type definition to include an optional isMobile boolean property to
match the new object shape and prevent compile-time errors.
Description
This PR includes updates to support the mobile editor.
Type of Change
Summary by CodeRabbit
New Features
Style
Bug Fixes
Chores