[lexical-playground] Fix bug: Insert an image inside another image's caption (#6109) #33
Conversation
![local-dev-korbit-ai-mentor[bot]](https://avatars.githubusercontent.com/in/318591?s=60&v=4){kind=small}
![[local-dev] Korbit - AI Mentor](https://avatars.githubusercontent.com/in/318591?s=40&u=4153787aed90096cc5d05c9c38a439007e524086&v=4){kind=avatar}
|
Clone of the PR facebook/lexical#6230 |
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
WalkthroughThe recent updates introduce enhanced functionality to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ToolbarPlugin
participant ImageComponent
participant ImageNode
User->>ToolbarPlugin: Interacts with toolbar
ToolbarPlugin->>ImageComponent: Checks editing context
ImageComponent->>ImageNode: Loads caption with nodes
ToolbarPlugin->>User: Updates available options
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
| this.__caption = | ||
| caption || | ||
| createEditor({ | ||
| nodes: [], | ||
| }); |
There was a problem hiding this comment.
The change to initialize the caption editor with an empty 'nodes' array might unintentionally restrict the types of content that can be added to image captions. Consider if there are specific node types that should be allowed in captions and include them in the 'nodes' array. For example, if you want to allow text formatting in captions, you might need to include text nodes and formatting nodes.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| export function $isEditorIsNestedEditor(editor: LexicalEditor): boolean { | ||
| return editor._parentEditor !== null; | ||
| } |
There was a problem hiding this comment.
The function name '$isEditorIsNestedEditor' is redundant and grammatically incorrect. Consider renaming it to '$isNestedEditor' for better clarity and consistency with Lexical naming conventions. The function implementation itself is correct.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| useEffect(() => { | ||
| return editor.registerCommand( | ||
| SELECTION_CHANGE_COMMAND, | ||
| (_payload, newEditor) => { | ||
| $updateToolbar(); | ||
| setActiveEditor(newEditor); | ||
| $updateToolbar(); | ||
| return false; | ||
| }, | ||
| COMMAND_PRIORITY_CRITICAL, | ||
| ); | ||
| }, [editor, $updateToolbar]); |
There was a problem hiding this comment.
The toolbar state is not being updated correctly when switching between the main editor and nested editors (like image captions). Consider updating the toolbar state whenever the active editor changes, not just on selection changes. This can be done by adding a useEffect hook that listens for changes to the activeEditor state.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
|
/review |
PR Reviewer Guide 🔍(Review updated until commit 1769e5e)
|
github-actions
bot
added
Review effort [1-5]: 4
and removed
Review effort [1-5]: 3
labels
|
Persistent review updated to latest commit 1769e5e |
|
@coderabbitai full review |
Actions performedFull review triggered. |
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/lexical-playground/src/nodes/ImageComponent.tsx (4 hunks)
- packages/lexical-playground/src/nodes/ImageNode.tsx (1 hunks)
- packages/lexical-playground/src/plugins/ToolbarPlugin/index.tsx (9 hunks)
- packages/lexical-utils/src/index.ts (1 hunks)
Additional comments not posted (10)
packages/lexical-playground/src/nodes/ImageNode.tsx (1)
156-160: Skip similar comment.The change to initialize the caption editor with an empty 'nodes' array might unintentionally restrict the types of content that can be added to image captions. This has been addressed in previous comments.
packages/lexical-playground/src/nodes/ImageComponent.tsx (1)
18-19: LGTM! Verify the usage of new node types.The inclusion of
HashtagNode,LinkNode,EmojiNode, andKeywordNodeenhances the flexibility of the caption editor.Ensure that these new node types are correctly integrated and utilized within the application.
Run the following script to verify the usage of new node types:
Also applies to: 45-49, 66-68
Verification successful
New node types are integrated and utilized in the codebase.
The new node types
HashtagNode,LinkNode,EmojiNode, andKeywordNodeare present and used across multiple files, including plugins and tests. This indicates they are integrated into the application.
- HashtagNode: Found in
packages/lexical-text/src/registerLexicalTextEntity.tsandpackages/lexical-react/src/LexicalHashtagPlugin.ts.- LinkNode: Extensively used in various files such as
packages/lexical-playground/src/plugins/ContextMenuPlugin/index.tsxandpackages/lexical-react/src/LexicalLinkPlugin.ts.- EmojiNode: Used in
examples/vanilla-js-plugin/src/emoji-plugin/EmojiPlugin.tsandpackages/lexical-playground/src/plugins/EmojisPlugin/index.ts.- KeywordNode: Found in
packages/lexical-playground/src/plugins/KeywordsPlugin/index.ts.These nodes are being registered with editors and used in transformations, confirming their integration.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new node types in the codebase. # Test: Search for the usage of new node types. Expect: Occurrences of the new node types. rg --type ts -A 5 $'HashtagNode|LinkNode|EmojiNode|KeywordNode'Length of output: 131491
packages/lexical-utils/src/index.ts (1)
610-612: Skip similar comment.The function name
$isEditorIsNestedEditoris redundant and grammatically incorrect. This has been addressed in previous comments.packages/lexical-playground/src/plugins/ToolbarPlugin/index.tsx (7)
549-549: LGTM: Addition ofisImageCaptionstate.The addition of the
isImageCaptionstate is appropriate for tracking the context of image captions.
554-563: LGTM: Update to$updateToolbarfunction.The changes to the
$updateToolbarfunction correctly handle the context of nested editors and update theisImageCaptionstate.
686-690: LGTM: Addition ofuseEffecthook for$updateToolbar.The
useEffecthook ensures the toolbar updates according to the active editor's state, enhancing context awareness.
861-862: LGTM: Addition of computed variables for toolbar visibility.The computed variables
canViewerSeeInsertDropdownandcanViewerSeeInsertCodeButtonare correctly used to conditionally render toolbar options.
973-985: LGTM: Conditional rendering of insert code button.The conditional rendering of the insert code button based on
canViewerSeeInsertCodeButtonis appropriate for the intended functionality.
1064-1232: LGTM: Conditional rendering of insert dropdown.The conditional rendering of the insert dropdown based on
canViewerSeeInsertDropdownenhances usability by reducing clutter.
Line range hint
895-930: LGTM: Replacement ofeditorprop withactiveEditor.The use of
activeEditorensures consistent handling of the editor's state, which is crucial for toolbar actions.
|
Zero comments from coderabbit |
Context:
Now users are able to insert an image/GIF/table etc, into another image's caption.
Solution:
This PR limits what the users can insert into the image caption and its nested editor. Excluding some controls from the toolbar if the selection is inside the nested editor that belongs to the image caption.
Closes #6109
Test plan
Before
before-image-caption.mov
After
image-caption.mov
Description by Korbit AI
What change is being made?
Fix the bug that allows inserting an image inside another image's caption by updating the
ImageComponent,ImageNode, andToolbarPlugincomponents to handle nested editors correctly.Why are these changes being made?
This change addresses a bug where images could be inserted into the caption of another image, which is not the intended behavior. By adding checks for nested editors and updating the toolbar functionality, we ensure that the editor behaves as expected and maintains a clean user experience.
Summary by CodeRabbit
New Features
ImageComponentto support a wider variety of content types in captions, including hashtags and links.ToolbarPluginfor better contextual awareness, adapting options based on the editing context.Bug Fixes