[WIKI-568] refactor: add touch device support to editor #7439
Conversation
WalkthroughThis change introduces comprehensive support for touch device detection and configuration throughout the collaborative editor. It adds new props and types for touch device awareness, editor focus handling, and drag-and-drop enablement. Editor commands and APIs are expanded, mobile-specific CSS styles are introduced, and menu items are enhanced with link editing capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorComponent
participant Extensions
participant UtilityExtension
User->>EditorComponent: Open editor (with device context)
EditorComponent->>Extensions: Initialize (isTouchDevice, editorProps, dragDropEnabled, etc.)
Extensions->>UtilityExtension: Pass isTouchDevice
UtilityExtension-->>Extensions: Store isTouchDevice in storage
EditorComponent->>EditorComponent: Render UI (menus, containers) based on isTouchDevice
User->>EditorComponent: Interact (focus, drag, link, image)
EditorComponent->>EditorComponent: Handle events differently for touch devices
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
* chore: misc editor updated and utility functions * fix: code review * passed isTouchDevice prop to editor-wrapper * added more props to editor-wrapper. * chore: update types --------- Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com>
aaryan610
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
aaryan610
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
packages/editor/src/ce/extensions/rich-text/read-only-extensions.tsx(0 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(4 hunks)packages/editor/src/core/components/editors/document/page-renderer.tsx(2 hunks)packages/editor/src/core/components/editors/editor-container.tsx(3 hunks)packages/editor/src/core/components/editors/editor-wrapper.tsx(3 hunks)packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx(2 hunks)packages/editor/src/core/components/editors/rich-text/editor.tsx(1 hunks)packages/editor/src/core/components/menus/menu-items.ts(6 hunks)packages/editor/src/core/extensions/custom-image/components/block.tsx(5 hunks)packages/editor/src/core/extensions/custom-image/components/node-view.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/root.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/uploader.tsx(2 hunks)packages/editor/src/core/extensions/extensions.ts(3 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(2 hunks)packages/editor/src/core/extensions/utility.ts(2 hunks)packages/editor/src/core/helpers/editor-commands.ts(1 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(3 hunks)packages/editor/src/core/hooks/use-editor.ts(7 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/core/types/editor.ts(8 hunks)packages/editor/src/core/types/hook.ts(2 hunks)packages/editor/src/styles/variables.css(2 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/ce/extensions/rich-text/read-only-extensions.tsx
🧰 Additional context used
🧠 Learnings (2)
packages/editor/src/core/extensions/custom-image/components/node-view.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/custom-image/components/block.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
🔇 Additional comments (60)
packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx (2)
24-24: LGTM: Proper prop destructuring.The
isTouchDeviceprop is correctly destructured from the incoming props.
51-51: No additional prop needed for useReadOnlyEditor hookThe
useReadOnlyEditorhook’s props type (TReadOnlyEditorHookProps) does not includeisTouchDevice—touch‐specific handling is delegated toEditorContainer. No changes are required to the hook signature or its invocation.• TReadOnlyEditorHookProps (packages/editor/src/core/types/hook.ts:58–60) extends only
TCoreHookProps, selectedTEditorHookProps, and selectedIReadOnlyEditorProps—none includeisTouchDevice.
• The wrapper (packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx) correctly forwardsisTouchDevice={!!isTouchDevice}toEditorContaineronly.packages/editor/src/core/types/config.ts (2)
27-27: LGTM: Mobile font size type addition.The addition of
"mobile-font"to theTEditorFontSizetype is consistent with the existing naming pattern and aligns with the mobile styling support.
29-29: LGTM: Mobile line spacing type addition.The addition of
"mobile-regular"to theTEditorLineSpacingtype follows the existing naming convention and supports mobile-specific line spacing configuration.packages/editor/src/core/extensions/custom-image/components/node-view.tsx (2)
27-27: LGTM: Improved state initialization logic.Initializing
isUploadedbased on the presence ofimgNodeSrcis more accurate than always starting withfalse. This ensures the component correctly reflects the actual state when an image node already has a source.
46-52: LGTM: Enhanced effect logic and dependencies.The updated effect correctly considers both
resolvedSrcandimgNodeSrcwhen determining upload status, and the dependency array properly includesimgNodeSrcto ensure the effect runs when the node's src attribute changes. This improves the component's reactivity and state consistency.packages/editor/src/core/extensions/extensions.ts (3)
40-48: LGTM: Well-structured type definition update.The
TArgumentstype is properly updated to include the optionalisTouchDeviceproperty, maintaining consistency with the existing type structure and the broader touch device support implementation.
59-59: LGTM: Proper destructuring with sensible default.The
isTouchDeviceparameter is correctly destructured with a default value offalse, which is a sensible fallback for non-touch devices.
112-112: LGTM: Correct prop forwarding to UtilityExtension.The
isTouchDeviceflag is properly passed to theUtilityExtension, enabling touch device-specific behavior at the extension level.packages/editor/src/core/extensions/custom-image/components/uploader.tsx (2)
43-43: LGTM! Clean touch device detection.The implementation correctly retrieves the
isTouchDeviceflag from the utility extension storage and ensures a boolean value with the double negation operator.
129-136: Excellent touch device UX improvement!The conditional file input triggering is a thoughtful enhancement:
- Prevents unexpected file picker behavior on touch devices
- Maintains existing functionality for non-touch devices
- Properly includes
isTouchDevicein the dependency arrayThis aligns well with touch device interaction patterns where users typically expect explicit actions to trigger file selection.
packages/editor/src/core/helpers/editor-commands.ts (1)
130-146: Well-implemented link editor enhancement!The addition of the optional
textparameter adds valuable functionality while maintaining backward compatibility. The implementation correctly:
- Preserves current selection coordinates before modification
- Deletes existing selection and inserts new text when provided
- Sets the selection to cover the inserted text before applying the link
- Falls back to existing behavior when no text is provided
The selection management logic is sound and ensures the link is applied to the correct text range.
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/root.tsx (3)
16-16: Good prop typing and destructuring.The
isTouchDeviceprop is properly typed as a boolean and correctly destructured from props, maintaining consistency with the touch device support pattern.Also applies to: 21-21
35-37: LGTM! Proper prop forwarding.The props are correctly passed to the
ImageFullScreenModalcomponent, ensuring the touch device flag and download source are available for conditional behavior.
42-42: Excellent touch device UX consideration!Conditionally disabling tooltips on touch devices is a thoughtful improvement, as tooltips typically don't provide value on touch interfaces and can interfere with touch interactions.
packages/editor/src/styles/variables.css (2)
92-112: Well-designed mobile font sizing!The mobile font size variables are thoughtfully configured:
- Maintains clear heading hierarchy
- Uses slightly smaller regular text (0.95rem) for better mobile readability
- Preserves consistent line heights for good typography
- Follows the established CSS variable naming pattern
172-191: Appropriate mobile line spacing!The mobile line spacing configuration provides good visual density for mobile screens by using reduced padding values similar to the small spacing variant. This helps maximize content visibility on smaller screens while maintaining readability.
packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (3)
15-18: Clean prop handling and typing.The
isTouchDeviceprop is properly typed and destructured. The prop reordering doesn't affect functionality and maintains good organization.Also applies to: 22-22
36-36: Smart conditional rendering for touch devices!Hiding the download action on touch devices is a good UX decision, as download functionality is typically less accessible or relevant on touch devices.
42-46: Proper prop forwarding to child components.The
ImageFullScreenActionRootreceives all necessary props including theisTouchDeviceflag, ensuring consistent touch device behavior throughout the image toolbar.packages/editor/src/core/extensions/read-only-extensions.ts (2)
36-39: LGTM! Proper type extension for touch device support.The Props type correctly includes
isTouchDevicefromIReadOnlyEditorProps, maintaining consistency with the existing prop selection pattern.
42-42: LGTM! Proper prop handling with sensible defaults.The
isTouchDeviceprop is correctly destructured with a default value offalseand properly passed to theUtilityExtension. This ensures backward compatibility and follows the established pattern.Also applies to: 85-85
packages/editor/src/core/extensions/utility.ts (3)
38-38: LGTM! Appropriate interface extension.The
isTouchDeviceboolean property is correctly added to theUtilityExtensionStorageinterface, enabling storage and access to touch device state throughout the extension system.
44-44: LGTM! Consistent type definition.The
isTouchDeviceboolean property is appropriately added to the Props type, maintaining consistency with other required properties in the interface.
48-48: LGTM! Proper prop handling and storage.The
isTouchDeviceprop is correctly destructured and stored in the extension's storage, making it accessible throughout the editor system. The implementation follows the established pattern for other stored properties.Also applies to: 81-81
packages/editor/src/core/hooks/use-collaborative-editor.ts (2)
30-33: LGTM! Well-designed prop additions.The new props are properly structured:
dragDropEnabledhas a sensible default oftruefor backward compatibilityisTouchDeviceandonEditorFocusare appropriately optional- The destructuring follows established patterns
92-92: LGTM! Proper prop forwarding and usage.The new props are correctly utilized:
dragDropEnabledproperly configures the SideMenuExtensionisTouchDeviceandonEditorFocusare appropriately forwarded to theuseEditorhookThis maintains the prop chain and enables the intended functionality.
Also applies to: 113-113, 117-117
packages/editor/src/core/components/editors/document/page-renderer.tsx (3)
14-14: LGTM! Appropriate prop interface extensions.The new props are correctly typed:
documentLoaderClassNameis appropriately optional for flexible stylingisTouchDeviceis consistently required as a booleanAlso applies to: 19-19
24-35: LGTM! Clean and comprehensive prop destructuring.All props, including the new
documentLoaderClassNameandisTouchDevice, are properly destructured following the established code pattern.
44-44: LGTM! Well-designed conditional rendering for touch devices.The implementation correctly:
- Passes
documentLoaderClassNameto the loader component- Forwards
isTouchDevicetoEditorContainer- Conditionally renders bubble and block menus only for non-touch, editable editors
- Maintains AI features availability across all device types
This approach appropriately adapts the UI for touch device limitations.
Also applies to: 51-51, 54-60
packages/editor/src/core/components/editors/editor-container.tsx (3)
1-1: LGTM! Appropriate type-only import optimization.Converting to
import type { Editor }is correct since Editor is only used in type positions, improving bundle efficiency.
13-20: LGTM! Clean interface update with new touch device support.The interface changes are well-executed:
- Renaming to
Propsimproves consistencyisTouchDeviceboolean prop is appropriately added- All existing props are preserved
23-23: LGTM! Proper conditional rendering for touch device optimization.The implementation correctly:
- Destructures the
isTouchDeviceprop following established patterns- Conditionally renders
LinkViewContaineronly on non-touch devicesThis approach appropriately disables potentially problematic UI elements on touch devices while maintaining full functionality on desktop.
Also applies to: 99-99
packages/editor/src/core/components/editors/editor-wrapper.tsx (3)
27-27: LGTM: Clean prop additions for enhanced editor functionality.The new props
editorProps,isTouchDevice, andonEditorFocusare properly integrated into the component interface and correctly forwarded to the underlying hooks and components.Also applies to: 31-31, 37-37
50-50: LGTM: Proper prop forwarding to useEditor hook.The new props are correctly passed to the
useEditorhook, maintaining consistency with the component's prop interface.Also applies to: 57-57, 61-61
84-84: LGTM: Safe boolean conversion for isTouchDevice.The
!!casting ensures thatisTouchDeviceis always a boolean when passed toEditorContainer, handling cases where the prop might be undefined.packages/editor/src/core/extensions/custom-image/components/block.tsx (5)
5-8: LGTM: Proper imports for extension storage access.The new imports enable accessing the
isTouchDeviceflag from the editor's utility extension storage, following the established pattern for extension data access.
64-65: LGTM: Clean touch device detection from extension storage.The
isTouchDeviceflag is properly retrieved from the utility extension storage with safe boolean conversion using!!.
197-205: LGTM: Appropriate touch device handling in mouse events.The touch-specific behavior prevents default actions and blurs the editor before selecting the image node, which is appropriate for touch interfaces to avoid unwanted interactions.
303-307: LGTM: Consistent prop forwarding to ImageToolbarRoot.The
isTouchDeviceflag is properly forwarded to theImageToolbarRootcomponent, enabling touch-aware UI behavior in the toolbar.
267-272: Review touch-device image refresh logic
Please confirm that the custom file handler’s getImageSource (getAssetSrc) implementation actually returns a newly refreshed URL (e.g. with cache-busting parameters) when called after restoreImage. Right now, on touch devices you callconst refreshedSrc = await extension.options.getImageSource?.(imgNodeSrc); imageRef.current.src = refreshedSrc;but on non-touch you reuse the initial resolvedImageSrc. If getAssetSrc always returns the same URL, users may still see stale images after restoration.
• File: packages/editor/src/core/extensions/custom-image/components/block.tsx
• Lines: ~267–272 (inside the try/catch restoring logic)Verify that your TFileHandler’s getAssetSrc behavior matches this intent or adjust it to return a fresh URL when needed.
packages/editor/src/core/types/hook.ts (2)
6-16: LGTM: Well-structured type definition using Pick pattern.The updated
TCoreHookPropsproperly picks the new properties fromIEditorProps, maintaining type safety while allowing the hook props to evolve with the editor interface. The use ofPickis a good pattern for maintaining consistency between related types.
53-56: LGTM: Consistent expansion of collaborative editor props.The addition of
dragDropEnabledto the picked properties fromICollaborativeDocumentEditorPropsfollows the established pattern and maintains type consistency.packages/editor/src/core/components/menus/menu-items.ts (4)
25-25: LGTM: Proper imports for link functionality.The new imports for
LinkIcon,setLinkEditor, andunsetLinkEditorprovide the necessary dependencies for the new link menu item functionality.Also applies to: 34-34, 49-49
195-202: LGTM: Consistent typing for HorizontalRuleItem.The explicit return type annotation
EditorMenuItem<"divider">withas constassertion ensures proper type inference and consistency with other menu items.
204-215: LGTM: Well-implemented LinkItem following established patterns.The
LinkItemimplementation follows the same structure as other menu items, with proper command logic that handles both setting links (when URL is provided) and unsetting them (when no URL). The conditional logic in the command function is appropriate.
273-273: LGTM: Proper integration of LinkItem into menu items array.The
LinkItemis correctly added to thegetEditorMenuItemsarray, making it available in the editor's menu system.packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (5)
15-16: LGTM: Clean addition of isTouchDevice prop.The
isTouchDeviceboolean prop is properly added to thePropstype interface, maintaining type safety for the component.
23-23: LGTM: Proper prop destructuring.The
isTouchDeviceprop is correctly destructured from the props object and made available throughout the component.
237-243: LGTM: Appropriate touch device event handling for zoom controls.The conditional
preventDefault()andstopPropagation()calls for touch devices help prevent unwanted browser behaviors during zoom interactions, which is a good practice for touch interfaces.Also applies to: 253-259
267-286: LGTM: Sensible UI adaptation for touch devices.Hiding the download and external link buttons on touch devices is appropriate since these actions may not work reliably or provide a good user experience on touch interfaces. File downloads and opening new tabs can be problematic on mobile devices.
299-302: LGTM: Improved portal fallback logic.The updated fallback logic that attempts to render into
document.bodywhen the preferred portal element is not found is a reasonable improvement, with appropriate error messaging.packages/editor/src/core/components/editors/document/collaborative-editor.tsx (2)
23-42: LGTM! Well-structured prop additions.The new props are appropriately typed and have sensible defaults. The renaming of
extensionstoexternalExtensionsimproves clarity by distinguishing between externally provided and internally managed extensions.
104-108: Good defensive type casting.The use of
!!isTouchDeviceensures the prop is always a boolean value, handling potential undefined/null cases appropriately.packages/editor/src/core/hooks/use-editor.ts (3)
37-42: Props integration looks good.The new
isTouchDeviceandonEditorFocusprops are properly integrated into the hook, following the existing patterns for prop handling and forwarding.Also applies to: 69-69, 83-83
136-168: Word selection implementation is correct.The
createSelectionAtCursorPositionmethod properly implements word selection by finding word boundaries using regex pattern matching. The algorithm correctly expands from the cursor position in both directions until non-word characters are found.
286-290: API improvement with object parameters.The signature change from positional to object parameters improves API clarity and extensibility. The default smooth scrolling behavior enhances user experience.
packages/editor/src/core/types/editor.ts (3)
47-47: Link command type definition is well-structured.The addition of the "link" command with required
urland optionaltextproperties follows the established pattern and provides appropriate type safety.Also applies to: 66-69
110-135: EditorRefApi extensions are properly typed.The new method signatures in
EditorRefApicorrectly match their implementations inuse-editor.tsand provide good type safety with appropriate return types and parameter definitions.
173-173: Interface update aligns with component implementation.Removing "extensions" from the
Omitclause correctly reflects thatCollaborativeDocumentEditornow accepts anextensionsprop (renamed internally toexternalExtensions).
packages/editor/src/core/components/editors/document/collaborative-editor.tsx
Show resolved
Hide resolved
packages/editor/src/core/components/editors/rich-text/editor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/editor/src/core/hooks/use-editor.ts (1)
136-168: Remove commented code and consider internationalization.
- Remove the commented out early return on line 140 if it's no longer needed.
- The regex
/\w/only matches ASCII word characters. Consider using a more inclusive pattern or Unicode property escapes for better internationalization support.- // if (empty) return null; if (empty) {For better unicode support, consider:
- 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++; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
packages/editor/src/ce/extensions/rich-text/read-only-extensions.tsx(0 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(4 hunks)packages/editor/src/core/components/editors/document/page-renderer.tsx(2 hunks)packages/editor/src/core/components/editors/editor-container.tsx(3 hunks)packages/editor/src/core/components/editors/editor-wrapper.tsx(3 hunks)packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx(2 hunks)packages/editor/src/core/components/editors/rich-text/editor.tsx(1 hunks)packages/editor/src/core/components/menus/menu-items.ts(6 hunks)packages/editor/src/core/extensions/custom-image/components/block.tsx(5 hunks)packages/editor/src/core/extensions/custom-image/components/node-view.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/root.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/uploader.tsx(2 hunks)packages/editor/src/core/extensions/extensions.ts(3 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(2 hunks)packages/editor/src/core/extensions/utility.ts(2 hunks)packages/editor/src/core/helpers/editor-commands.ts(1 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(3 hunks)packages/editor/src/core/hooks/use-editor.ts(7 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/core/types/editor.ts(8 hunks)packages/editor/src/core/types/hook.ts(2 hunks)packages/editor/src/styles/variables.css(2 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/ce/extensions/rich-text/read-only-extensions.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SatishGandham
PR: makeplane/plane#5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In `packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts`, the `getImageSource` command returns a string directly, not a function.
packages/editor/src/core/extensions/custom-image/components/node-view.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/custom-image/components/block.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
🔇 Additional comments (68)
packages/editor/src/core/types/config.ts (2)
27-27: LGTM! Mobile font size type addition looks good.The addition of
"mobile-font"toTEditorFontSizefollows the existing naming convention and supports mobile-specific styling configurations.
29-29: LGTM! Mobile line spacing type addition looks good.The addition of
"mobile-regular"toTEditorLineSpacingfollows the existing naming convention and complements the mobile font size support.packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx (2)
24-24: LGTM! Touch device prop properly destructured.The
isTouchDeviceprop is correctly added to the component props destructuring.
51-51: NoisTouchDeviceprop required on useReadOnlyEditor
Verified thatuseReadOnlyEditoronly destructures properties fromTReadOnlyEditorHookProps(content, extensions, handlers, etc.) and does not accept anisTouchDevicefield. The boolean flag is only needed byEditorContainer, so no changes to the hook are necessary.packages/editor/src/core/extensions/custom-image/components/node-view.tsx (2)
27-27: LGTM! Improved initial upload state logic.Initializing
isUploadedbased on!!imgNodeSrccorrectly reflects the actual state when the component mounts with an existing image source, rather than always starting withfalse.
46-52: LGTM! Enhanced upload state synchronization.The updated useEffect correctly handles upload state based on both
resolvedSrcandimgNodeSrc, ensuring the state accurately reflects the image availability from multiple sources. The dependency array update ensures proper reactivity.packages/editor/src/core/extensions/custom-image/components/uploader.tsx (2)
43-43: LGTM! Proper touch device detection.The
isTouchDeviceflag is correctly retrieved from the utility extension storage using the established pattern.
129-136: LGTM! Excellent touch device UX improvement.The conditional logic properly prevents automatic file input triggering on touch devices, which improves user experience by avoiding unwanted file picker popups. The dependency array correctly includes
isTouchDeviceto ensure the effect re-runs if device detection changes.packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (4)
15-18: LGTM: Props interface updated correctly.The addition of
isTouchDeviceboolean prop and reordering of properties is well-implemented and maintains type safety.
22-22: LGTM: Proper prop destructuring.The
isTouchDeviceprop is correctly destructured alongside existing props.
36-36: LGTM: Appropriate conditional rendering for touch devices.Disabling the download action on touch devices is a sensible UX decision, as file downloads are typically more complex on mobile/touch interfaces.
42-46: LGTM: Consistent prop propagation.The
isTouchDeviceprop is properly passed to theImageFullScreenActionRootcomponent, maintaining consistency in the component tree.packages/editor/src/core/helpers/editor-commands.ts (1)
130-146: LGTM: Enhanced link insertion with text support.The function correctly handles both scenarios:
- Without text: applies link to current selection (existing behavior)
- With text: replaces selection with provided text and applies link
The selection management logic is properly implemented, ensuring the link is applied to the correct text range.
packages/editor/src/core/components/editors/editor-container.tsx (4)
1-1: LGTM: Appropriate type-only import.Converting to type-only import is correct since
Editoris only used for type annotations in this file.
13-20: LGTM: Props interface updated correctly.The interface renaming to
Propsand addition ofisTouchDeviceboolean property follows common React patterns and maintains type safety.
22-23: LGTM: Proper prop destructuring.The
isTouchDeviceprop is correctly included in the destructuring assignment.
99-99: LGTM: Sensible conditional rendering for touch devices.Disabling
LinkViewContaineron touch devices is appropriate, as link preview overlays can interfere with touch interactions and navigation patterns on mobile devices.packages/editor/src/core/extensions/extensions.ts (3)
41-48: LGTM: TArguments type properly extended.The addition of
isTouchDeviceto the type definition maintains type safety and follows the existing pattern of destructuring IEditorProps properties.
59-59: LGTM: Sensible default value.Using
falseas the default forisTouchDeviceis appropriate, assuming non-touch devices when the property is not explicitly provided.
108-113: LGTM: Proper prop propagation to UtilityExtension.The
isTouchDeviceflag is correctly passed to theUtilityExtension, enabling touch device awareness throughout the extension system.packages/editor/src/core/components/editors/editor-wrapper.tsx (3)
27-27: LGTM: New props properly integrated.The addition of
editorProps,isTouchDevice, andonEditorFocusprops enhances the editor's configurability and touch device awareness.Also applies to: 31-31, 37-37
50-50: LGTM: Props correctly passed to useEditor hook.The new props are properly forwarded to the
useEditorhook, enabling the editor instance to be aware of touch device context and handle focus events.Also applies to: 57-57, 61-61
84-84: LGTM: Explicit boolean casting for isTouchDevice.The
!!isTouchDevicecasting ensuresEditorContainerreceives a proper boolean value, preventing potential issues with undefined or falsy values.packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/root.tsx (2)
8-18: LGTM! Proper touch device prop integration.The addition of
isTouchDeviceto the Props type and component destructuring follows the consistent pattern across the codebase for touch device support.
42-42: Good UX decision to disable tooltips on touch devices.Conditionally disabling tooltips on touch devices is appropriate since tooltips are primarily designed for hover interactions, which don't exist on touch interfaces.
packages/editor/src/core/extensions/utility.ts (2)
33-39: LGTM! Proper extension storage interface update.Adding
isTouchDeviceto theUtilityExtensionStorageinterface enables touch device awareness across all extensions that access this storage.
75-83: Correct implementation of touch device storage initialization.The
isTouchDevicevalue is properly stored in the extension storage during initialization, making it accessible to other parts of the editor system.packages/editor/src/core/components/editors/document/page-renderer.tsx (2)
10-21: LGTM! Proper prop type extensions.The addition of
documentLoaderClassNameandisTouchDeviceprops properly extends the component interface for enhanced customization and touch device support.
54-60: Excellent UX decision for touch device menu handling.Conditionally hiding the bubble menu and block menu on touch devices while preserving the AI features menu is a smart UX choice. These menus are typically hover-based and not suitable for touch interfaces.
packages/editor/src/core/extensions/custom-image/components/block.tsx (5)
5-8: LGTM! Proper imports for touch device detection.Adding the necessary imports to access the touch device state from the utility extension storage is correct.
64-65: Correct implementation of touch device detection.Retrieving the
isTouchDeviceflag from the utility extension storage usinggetExtensionStorageis the proper way to access this shared state.
194-206: Good touch device handling in mouse down event.The conditional logic to prevent default behavior and blur the editor on touch devices is appropriate for proper image selection handling on touch interfaces.
303-307: LGTM! Proper prop threading to toolbar component.The
ImageToolbarRootcomponent correctly receives theisTouchDeviceprop to enable touch-aware behavior in the toolbar.
267-272: Review Touch vs. Non-Touch Image Restoration LogicI wasn’t able to find any documentation or comments explaining why we refresh the image URL on touch devices but use
resolvedImageSrcdirectly on non-touch. Please confirm this divergence is intentional and reflects a specific touch-device caching or rendering workaround.• File: packages/editor/src/core/extensions/custom-image/components/block.tsx
Lines: ~267–272if (isTouchDevice) { const refreshedSrc = await extension.options.getImageSource?.(imgNodeSrc); imageRef.current.src = refreshedSrc; } else { imageRef.current.src = resolvedImageSrc; }Points to verify:
- Is there a known touch-device issue (e.g. Mobile Safari caching) that necessitates fetching a fresh URL?
- If so, please add a brief inline comment or documentation to clarify the rationale.
- Otherwise, consider unifying the logic or handling both cases consistently.
packages/editor/src/core/hooks/use-collaborative-editor.ts (3)
30-33: LGTM! Good configurability improvements.Adding
dragDropEnabled,isTouchDevice, andonEditorFocusprops enhances the hook's flexibility and enables touch device awareness and custom focus handling.
90-93: Excellent refactor to make drag-drop configurable.Replacing the hardcoded drag-drop behavior with the configurable
dragDropEnabledprop improves the component's flexibility while maintaining backward compatibility with the default value.
113-117: Proper prop forwarding to useEditor hook.The
isTouchDeviceandonEditorFocusprops are correctly forwarded to theuseEditorhook, ensuring consistent touch device awareness and focus handling throughout the editor system.packages/editor/src/core/extensions/read-only-extensions.ts (3)
36-39: LGTM! Proper type definition for isTouchDevice prop.The Props type correctly includes the new
isTouchDeviceproperty fromIReadOnlyEditorProps, maintaining consistency with the interface contract.
42-42: Good implementation with sensible default.The destructuring includes
isTouchDevice = falseas a default value, ensuring backward compatibility for existing usage without the prop.
81-86: Correct integration with UtilityExtension.The
isTouchDeviceprop is properly passed to theUtilityExtensionconfiguration, enabling touch-specific behavior in the read-only editor extensions.packages/editor/src/core/types/hook.ts (2)
6-16: Well-structured type expansion for new editor features.The
TCoreHookPropstype correctly incorporates the newisTouchDevice,editorProps, andonEditorFocusproperties fromIEditorProps. The use of thePickutility type maintains consistency with the existing pattern and ensures type safety.
53-56: Consistent type enhancement for collaborative features.The addition of
"dragDropEnabled"toTCollaborativeEditorHookPropsfollows the established pattern and supports the enhanced collaborative editor functionality.packages/editor/src/core/components/menus/menu-items.ts (4)
25-25: Good addition of link-related imports.The imports for
LinkIcon,setLinkEditor, andunsetLinkEditorproperly support the new link functionality being added to the editor menu system.Also applies to: 34-34, 49-49
195-202: Improved explicit typing for HorizontalRuleItem.Adding the explicit return type annotation
EditorMenuItem<"divider">withas constimproves type inference and consistency with other menu items.
204-215: Well-implemented LinkItem with proper command logic.The
LinkItemimplementation correctly:
- Uses proper typing with
EditorMenuItem<"link">- Handles both setting links (with URL and optional text) and unsetting links
- Integrates with the updated editor command helpers
- Follows the established pattern for menu items
273-273: Proper integration into menu items array.The
LinkItemis correctly added to thegetEditorMenuItemsarray, making the link functionality available in the editor's menu system.packages/editor/src/styles/variables.css (2)
92-112: Well-designed mobile font sizing.The
.mobile-fontclass provides appropriate font sizes for mobile devices:
- Regular text at 0.95rem strikes a good balance between readability and screen space
- Heading sizes maintain hierarchy while being mobile-friendly
- Line heights ensure good readability on touch devices
172-191: Appropriate mobile spacing for touch interfaces.The
.line-spacing-mobile-regularclass provides reduced padding values that work well for mobile/touch devices:
- Reduced heading padding (16px top vs 28px) conserves vertical space
- Minimal paragraph and list spacing optimizes content density
- Values align well with mobile UX best practices
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (4)
15-17: Proper integration of isTouchDevice prop.The addition of
isTouchDevice: booleanto the Props type correctly supports touch device detection for conditional behavior in the fullscreen modal.
237-243: Smart touch-specific event handling.The conditional
preventDefaultandstopPropagationcalls on touch devices help prevent unwanted touch behaviors (like scrolling or zooming) when interacting with zoom controls. This improves the user experience on mobile devices.Also applies to: 253-259
267-276: Appropriate UX adaptation for touch devices.Hiding the download and "open in new tab" buttons on touch devices makes sense, as these actions are typically handled differently or not available in mobile browsers. This reduces UI clutter and focuses on the core functionality.
Also applies to: 277-286
299-302: Improved portal fallback handling.The addition of a
document.bodyfallback when the#editor-portalelement is not found is a good defensive programming practice that ensures the modal can still render in edge cases.packages/editor/src/core/components/editors/document/collaborative-editor.tsx (4)
1-2: LGTM!The imports are correctly added to support the new functionality.
23-48: LGTM!Props are correctly destructured with appropriate defaults. Good practice renaming
extensionstoexternalExtensionsfor clarity.
69-89: LGTM!New props are correctly passed to the
useCollaborativeEditorhook.
104-108: LGTM!Props are correctly passed to
PageRenderer. Good practice using!!to ensureisTouchDeviceis always a boolean.packages/editor/src/core/hooks/use-editor.ts (5)
37-41: Props addition looks good!The new
isTouchDeviceandonEditorFocusprops are properly destructured and follow the existing pattern.
69-83: Props are correctly propagated to editor configuration.The
isTouchDeviceprop is properly passed toCoreEditorExtensionsandonEditorFocusis correctly wired to the editor's focus event.
183-190: New editor API methods are well-implemented.The
focus,getCoordsFromPos, andgetAttributesWithExtendedMarkmethods are properly implemented with appropriate null checks and sensible defaults.
285-290: API improvements for undo and scroll methods.The addition of
redomethod and the refactoredscrollToNodeViaDOMCoordinateswith object parameters and default smooth scrolling behavior improve the API ergonomics.
318-318: Undo method implementation is correct.The
undomethod properly delegates to the editor's undo command, maintaining consistency with theredomethod.packages/editor/src/core/types/editor.ts (7)
1-4: Import reorganization improves clarity.Using explicit
typeimports is a best practice that improves code clarity and enables better tree-shaking.
47-47: Link command type definitions are well-structured.The addition of the "link" command with required
urland optionaltextprops follows the existing pattern and provides appropriate flexibility.Also applies to: 66-69
110-118: EditorRefApi interface updates are comprehensive and well-typed.The new methods and updated signatures properly reflect the implementation changes in
use-editor.ts. Good use of TypeScript utility types and proper return type annotations.Also applies to: 128-134
145-156: IEditorProps interface additions maintain backward compatibility.The new optional props (
editorProps,isTouchDevice,onEditorFocus) are properly typed and maintain backward compatibility.
173-173: Removing "extensions" from omitted props increases flexibility.Allowing collaborative editors to have custom extensions is a positive change that enables more customization options.
175-176: Collaborative editor configuration options are well-defined.The
documentLoaderClassNameanddragDropEnabledprops provide useful configuration options with clear naming and appropriate types.
205-205: Read-only editor props correctly include isTouchDevice.Including
isTouchDevicein the read-only editor props maintains consistency across editor types.
packages/editor/src/core/components/editors/document/collaborative-editor.tsx
Show resolved
Hide resolved
packages/editor/src/core/components/editors/rich-text/editor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/editor/src/core/components/editors/rich-text/editor.tsx(1 hunks)packages/editor/src/core/extensions/utility.ts(2 hunks)packages/editor/src/core/helpers/editor-ref.ts(4 hunks)packages/editor/src/core/hooks/use-editor.ts(3 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/core/types/editor.ts(7 hunks)packages/editor/src/core/types/hook.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/editor/src/core/components/editors/rich-text/editor.tsx
- packages/editor/src/core/hooks/use-editor.ts
- packages/editor/src/core/extensions/utility.ts
- packages/editor/src/core/types/config.ts
- packages/editor/src/core/types/hook.ts
- packages/editor/src/core/types/editor.ts
🔇 Additional comments (5)
packages/editor/src/core/helpers/editor-ref.ts (5)
27-27: LGTM!Simple and safe implementation using optional chaining to handle null editor instances.
106-106: LGTM!Clean wrapper implementation with proper optional chaining and argument forwarding.
109-113: LGTM!Well-implemented method with proper null checking and clear logic flow. The early return pattern for null editor is appropriate.
208-208: LGTM!Both undo and redo methods are clean wrapper implementations with proper optional chaining for safety.
Also applies to: 241-241
209-213: LGTM!Good API improvement changing from multiple parameters to a single object parameter with default values. This makes the method more readable and maintainable while preserving the original functionality.
* refactor: add isTouchDevice prop * chore: handle event propagation in touch devices * refactor: isTouchDevice implementation * chore: misc editor updates and utility functions (#7455) * chore: misc editor updated and utility functions * fix: code review * passed isTouchDevice prop to editor-wrapper * added more props to editor-wrapper. * chore: update types --------- Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com> * fix: remove unnecessary deps --------- Co-authored-by: Lakhan Baheti <94619783+1akhanBaheti@users.noreply.github.com>
Description
This PR adds support for touch devices to the editor package.
Type of Change
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Summary by CodeRabbit
New Features
Improvements
Style