[WEB-2528] fix: side menu rendering even if created already #5687
Conversation
WalkthroughThe changes involve modifications across several components within the editor. Key updates include altering the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
packages/editor/src/core/extensions/side-menu.tsx (3)
81-83: Approve: Conditional initialization of AI handleThis change effectively prevents multiple initializations of the AI handle, addressing the issue described in the PR. Good job!
Consider caching the result of
editorSideMenu.querySelector("#ai-handle")to avoid repeated DOM queries:const aiHandle = editorSideMenu.querySelector("#ai-handle"); if (handlesConfig.ai && !aiHandle) { aiHandleView(view, editorSideMenu); }This minor optimization could improve performance, especially if this code is executed frequently.
85-87: Approve: Conditional initialization of drag handleThis change successfully prevents multiple initializations of the drag handle, addressing the issue described in the PR. Well done!
Similar to the AI handle, consider caching the result of the querySelector to optimize performance:
const dragHandle = editorSideMenu.querySelector("#drag-handle"); if (handlesConfig.dragDrop && !dragHandle) { dragHandleView(view, editorSideMenu); }This minor optimization could improve performance, especially if this code is executed frequently.
Line range hint
41-157: Consider optimizing handle initializationThe current implementation checks for the existence of handles on every mousemove event. While this works, it may be more efficient to perform these checks once during the plugin's initialization.
Consider refactoring the SideMenu plugin to initialize the handles only once:
const SideMenu = (options: SideMenuPluginProps) => { const { handlesConfig } = options; const editorSideMenu: HTMLDivElement | null = document.createElement("div"); editorSideMenu.id = "editor-side-menu"; // ... (other code) let aiHandleInitialized = false; let dragHandleInitialized = false; return new Plugin({ key: new PluginKey("sideMenu"), view: (view) => { hideSideMenu(); view?.dom.parentElement?.appendChild(editorSideMenu); // Initialize handles once if (handlesConfig.ai && !aiHandleInitialized) { aiHandleView(view, editorSideMenu); aiHandleInitialized = true; } if (handlesConfig.dragDrop && !dragHandleInitialized) { dragHandleView(view, editorSideMenu); dragHandleInitialized = true; } // ... (rest of the code) }, props: { handleDOMEvents: { mousemove: (view, event) => { // ... (existing mousemove logic) // Remove handle checks from here showSideMenu(); if (handlesConfig.dragDrop) { dragHandleDOMEvents?.mousemove(); } if (handlesConfig.ai) { aiHandleDOMEvents?.mousemove?.(); } }, // ... (other event handlers) }, }, }); };This approach would initialize the handles only once when the plugin is created, potentially improving performance by avoiding repeated DOM queries on every mousemove event.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
packages/editor/src/core/plugins/ai-handle.ts (1)
Multiple Usages of
nodeDOMAtCoordsStill Present in CodebaseThe
nodeDOMAtCoordsfunction is still being used in several files, which will cause runtime errors if not addressed.Affected Files:
packages/editor/src/core/plugins/ai-handle.tspackages/editor/src/core/plugins/drag-handle.tspackages/editor/src/core/extensions/side-menu.tsxAnalysis chain
Line range hint
71-75: UpdatehandleClickfunction to handle removal ofnodeDOMAtCoordsThe
nodeDOMAtCoordsfunction has been removed, but it's still being used in thehandleClickfunction. This will cause a runtime error. The functionality ofnodeDOMAtCoordsneeds to be replaced or reimplemented.Please update the
handleClickfunction to handle the removal ofnodeDOMAtCoords. This may involve reimplementing the functionality directly in this function or using an alternative method to determine the node at the given coordinates.To verify the usage of
nodeDOMAtCoordsin other parts of the codebase, run the following script:This will help identify if the function is still being used elsewhere and needs to be updated or if it has been completely removed from the project.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other usages of nodeDOMAtCoords in the codebase # Test: Search for usages of nodeDOMAtCoords rg --type typescript 'nodeDOMAtCoords'Length of output: 73
Script:
#!/bin/bash # Description: Check for other usages of nodeDOMAtCoords in the codebase # Test: Search for usages of nodeDOMAtCoords rg --type ts 'nodeDOMAtCoords'Length of output: 715
packages/editor/src/core/extensions/custom-image/components/image-block.tsx (1)
Line range hint
148-152: LGTM! Consider documenting the purpose of the new class.The addition of the "image-component" class to the image element is a good change. It provides a more specific selector for styling or JavaScript operations targeting these image components.
To improve maintainability, consider adding a brief comment explaining the purpose of the "image-component" class, especially if it's used for specific styling or functionality elsewhere in the application.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/editor/src/core/extensions/custom-image/components/image-block.tsx (1 hunks)
- packages/editor/src/core/extensions/side-menu.tsx (2 hunks)
- packages/editor/src/core/plugins/ai-handle.ts (1 hunks)
- packages/editor/src/core/plugins/drag-handle.ts (1 hunks)
- packages/editor/src/styles/drag-drop.css (0 hunks)
Files not reviewed due to no reviewable changes (1)
- packages/editor/src/styles/drag-drop.css
Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/extensions/side-menu.tsx
Additional comments not posted (2)
packages/editor/src/core/plugins/drag-handle.ts (2)
Line range hint
1-380: Overall assessment of changesThe modifications to the
nodeDOMAtCoordsfunction in this file represent an improvement in the handling of image components and element selection for the drag-and-drop functionality. The code is now more streamlined and potentially more performant due to the removal of nested image detection.However, it's crucial to ensure that these changes don't introduce any unintended side effects, particularly in:
- Handling of nested images
- Behavior of the drag handle for the first paragraph in the editor
- Overall drag-and-drop functionality for various element types
To maintain the integrity of the editor's functionality, I recommend:
- Thoroughly testing the drag-and-drop behavior with various content types, especially images and nested structures.
- Updating any relevant documentation to reflect the new behavior of element selection.
- Reviewing and updating any affected unit or integration tests to cover the new scenarios introduced by these changes.
44-45: Improved selector handling for image componentsThe addition of
.image-componentand.image-upload-componentto thegeneralSelectorsarray is a positive change. It suggests that the editor now has dedicated components for handling images, which is good for maintainability and extensibility.However, there are a few points to consider:
The removal of nested image detection logic (previously handled by
hasNestedImg) might affect how nested images are handled in terms of selection and drag-and-drop operations. Please ensure that this change doesn't introduce any regressions in image handling.The function now returns the first child
<p>tag of the.ProseMirrorelement. This might change the behavior of the drag handle for the first paragraph in the editor. Verify that this change aligns with the expected editor behavior.The new check for non-empty
<p>tags in table cells (tdandth) is a good improvement for usability, as it prevents selection of empty cells.To ensure these changes don't introduce any regressions, please run the following verification script:
This script will help identify any potential issues related to the removal of
hasNestedImgand verify the usage of the new image component selectors across the codebase.
| @@ -2,45 +2,12 @@ import { NodeSelection } from "@tiptap/pm/state"; | |||
| import { EditorView } from "@tiptap/pm/view"; | |||
| // extensions | |||
| import { SideMenuHandleOptions, SideMenuPluginProps } from "@/extensions"; | |||
| // plugins | |||
| import { nodeDOMAtCoords } from "@/plugins/drag-handle"; | |||
There was a problem hiding this comment.
Remove unused import nodeDOMAtCoords
The nodeDOMAtCoords function is imported but not used in this file after the recent changes. To maintain clean code and avoid potential confusion, it's recommended to remove unused imports.
Apply this diff to remove the unused import:
-import { nodeDOMAtCoords } from "@/plugins/drag-handle";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.
| import { nodeDOMAtCoords } from "@/plugins/drag-handle"; |
Palanikannan1437
changed the title
Description
Summary by CodeRabbit
CustomImageBlockcomponent with updated styling for improved visual consistency.SideMenucomponent to prevent duplication of AI handle and drag handle views, ensuring a cleaner user interface.drag-handlefunctionality for better performance.