-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[WIKI-728] fix : update emoji insertion method #7962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: preview
Are you sure you want to change the base?
Conversation
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughAdds a forceOpen flag to emoji storage and dropdown, introduces openEmojiPicker command, wires slash command to invoke it, and updates suggestion/emoji insertion logic to use fragment-based replace and explicit selection handling. The dropdown can render without a query when forced open and keyboard handling respects this state. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant E as Editor
participant SC as Slash Commands
participant EC as editor-commands.openEmojiPicker
participant ES as Emoji Storage (forceOpen)
participant S as Suggestion Plugin
participant D as EmojisListDropdown
U->>SC: Select "Emoji" slash command
SC->>EC: openEmojiPicker(editor, range)
EC->>E: deleteRange(range)?
EC->>ES: set forceOpen = true
EC->>E: insert ":" at selection
E->>S: trigger emoji suggestion
S->>D: render with props { query, forceOpen: true }
D-->>U: Dropdown visible even if query is empty
U->>D: Navigate/select emoji (keyboard/mouse)
D->>S: onSelect(emoji)
S->>E: run emoji insert command
sequenceDiagram
autonumber
participant S as Suggestion Plugin
participant EE as Emoji Extension
participant PM as ProseMirror (Fragment, TextSelection)
S->>EE: Insert emoji at range
EE->>PM: replace range with Fragment(emojiNode + " ")
EE->>PM: set TextSelection.near(after emoji + space)
EE->>PM: restore stored marks
PM-->>S: selection and content updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the emoji insertion method to preserve parent block structure when inserting emojis through slash commands. Instead of replacing the entire range content (which could break heading structures), it now uses a more targeted approach that replaces only the inline content while maintaining the parent block.
- Updates emoji insertion logic to use
replaceWith
instead ofinsertContentAt
for better range handling - Introduces a
forceOpen
flag to control emoji picker visibility - Refactors slash command emoji trigger to use dedicated
openEmojiPicker
helper
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/editor/src/core/helpers/editor-commands.ts | Adds new openEmojiPicker function and imports for extended emoji storage |
packages/editor/src/core/extensions/slash-commands/command-items-list.tsx | Updates emoji slash command to use new openEmojiPicker helper |
packages/editor/src/core/extensions/emoji/suggestion.ts | Adds support for forceOpen flag in emoji suggestion handling |
packages/editor/src/core/extensions/emoji/emoji.ts | Refactors emoji insertion logic and extends storage interface with forceOpen flag |
packages/editor/src/core/extensions/emoji/components/emojis-list.tsx | Updates emoji list component to handle forceOpen prop for visibility control |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
54-61
: Guard active editor in handleClose
handleClose() may be invoked without aneditor
; accessingeditor?.storage.emoji
then.emoji
throws. Useeditor ?? editorRef
for storage access:- const handleClose = (editor?: Editor) => { - component?.destroy(); - component = null; - (editor || editorRef)?.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.EMOJI); - const emojiStorage = editor?.storage.emoji as ExtendedEmojiStorage; - emojiStorage.forceOpen = false; - cleanup(); - }; + const handleClose = (editor?: Editor) => { + const activeEditor = editor ?? editorRef; + component?.destroy(); + component = null; + activeEditor?.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.EMOJI); + const storage = (activeEditor?.storage?.emoji as ExtendedEmojiStorage | undefined); + if (storage) storage.forceOpen = false; + cleanup(); + };
🧹 Nitpick comments (3)
packages/editor/src/core/helpers/editor-commands.ts (1)
189-194
: Make delete+insert atomic and guard storage accessSet forceOpen defensively and perform delete+insert in one chain to avoid split transactions.
-export const openEmojiPicker = (editor: Editor, range?: Range) => { - if (range) editor.chain().focus().deleteRange(range).run(); - const emojiStorage = editor.storage.emoji as ExtendedEmojiStorage; - emojiStorage.forceOpen = true; - editor.chain().focus().insertContent(":").run(); -}; +export const openEmojiPicker = (editor: Editor, range?: Range) => { + // Set flag first so the suggestion reads it on start + const storage = (editor.storage?.emoji as ExtendedEmojiStorage | undefined) ?? undefined; + if (storage) storage.forceOpen = true; + + const chain = editor.chain().focus(); + if (range) chain.deleteRange(range); + chain.insertContent(":").run(); +};packages/editor/src/core/extensions/emoji/components/emojis-list.tsx (1)
45-53
: Solid guards for empty query; consider minor a11y tweaksKeyboard handling and render guards look good. Optionally add listbox/option roles for accessibility.
Example:
- Add role="listbox" to the container div.
- Add role="option" and aria-selected to each button.
Also applies to: 111-111
packages/editor/src/core/extensions/emoji/emoji.ts (1)
159-160
: Storage default flag addedforceOpen default in storage is fine. Optionally confirm that isSupported returns the desired value for emojis without a version (currently false).
Would you like to treat versionless emojis as supported by default? If yes, change to:
isSupported: (e) => (e.version ? supportMap[e.version] : true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/editor/src/core/extensions/emoji/components/emojis-list.tsx
(4 hunks)packages/editor/src/core/extensions/emoji/emoji.ts
(3 hunks)packages/editor/src/core/extensions/emoji/suggestion.ts
(3 hunks)packages/editor/src/core/extensions/slash-commands/command-items-list.tsx
(2 hunks)packages/editor/src/core/helpers/editor-commands.ts
(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/editor/src/core/extensions/slash-commands/command-items-list.tsx (1)
packages/editor/src/core/helpers/editor-commands.ts (1)
openEmojiPicker
(189-194)
packages/editor/src/core/helpers/editor-commands.ts (1)
packages/editor/src/core/extensions/emoji/emoji.ts (1)
ExtendedEmojiStorage
(23-25)
packages/editor/src/core/extensions/emoji/suggestion.ts (2)
packages/editor/src/core/extensions/emoji/emoji.ts (1)
ExtendedEmojiStorage
(23-25)packages/editor/src/core/extensions/emoji/components/emojis-list.tsx (2)
EmojisListDropdownProps
(20-23)EmojisListDropdown
(25-178)
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (3)
packages/editor/src/core/helpers/editor-commands.ts (1)
8-8
: Import looks correctType import aligns with the new storage interface.
packages/editor/src/core/extensions/slash-commands/command-items-list.tsx (1)
201-203
: LGTM: delegate to openEmojiPickerCentralizes emoji trigger logic and preserves block structure via range-aware delete.
packages/editor/src/core/extensions/emoji/emoji.ts (1)
112-128
: Emoji insertion via fragment + explicit selection is correctReplaces matched range with [emoji, space], moves caret after space, and preserves marks. This aligns with inline-only replacements.
Description
This PR update the logic of inserting emoji by replacing only the inline content within the range while preserving the parent block structure (the heading)
Type of Change
Screenshots and Media (if applicable)
Screen.Recording.2025-10-13.at.7.49.45.PM.mov
Test Scenarios
References
Summary by CodeRabbit
New Features
Improvements