[WIKI-707] [WIKI-708] fix: editor floating ui modal#7909
Conversation
|
Linked to Plane Work Item(s)
This comment was auto-generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes floating UI modal issues in the editor by addressing z-index conflicts and preventing unwanted modal closures. The changes ensure proper layering of UI elements and prevent event propagation that was causing modals to close unexpectedly.
- Updated z-index handling for the block menu component
- Added click event handler to prevent mention modal from closing unexpectedly
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/editor/src/core/extensions/mentions/utils.ts | Added click event handler with stopPropagation to prevent modal closing |
| packages/editor/src/core/components/menus/block-menu.tsx | Updated z-index from CSS class to inline style and removed conflicting z-20 class |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| element.addEventListener("click", (e) => { | ||
| e.stopPropagation(); | ||
| }); |
There was a problem hiding this comment.
The event listener is added but never removed, which could lead to memory leaks. Consider storing the listener reference and removing it in the cleanup phase, or use a more specific event delegation pattern.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds visual stacking and DOM attachment for dropdowns and prevents event bubbling from the mentions dropdown; no public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MentionsComponent as Mentions UI
participant Utils as mentions/utils
participant Dropdown as DropdownElement
participant Document as document.body
User->>MentionsComponent: open mention menu (hover/caret)
MentionsComponent->>Utils: renderMentionsDropdown.onStart()
Utils->>Dropdown: create & style dropdown
Utils->>Document: append Dropdown to body
Utils->>Dropdown: update position
Note right of Dropdown: Event handlers added\nstopPropagation on click/mouseDown
User->>Dropdown: click item
Dropdown-->>Utils: stopPropagation prevents ancestor events
Dropdown->>MentionsComponent: selectItem (item chosen)
MentionsComponent->>User: insert mention
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (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). (2)
🔇 Additional comments (3)
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 |
![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
♻️ Duplicate comments (1)
packages/editor/src/core/extensions/mentions/utils.ts (1)
36-38: Memory leak: Event listener never removed.The click event listener prevents modal closure correctly but is never cleaned up. When the component is destroyed in
onExit(lines 59-63), the element is removed from the DOM but the listener reference persists in memory.Store the listener reference and remove it during cleanup:
export const renderMentionsDropdown = (args: Pick<TMentionHandler, "searchCallback">): SuggestionOptions["render"] => () => { const { searchCallback } = args; let component: ReactRenderer<CommandListInstance, MentionsListDropdownProps> | null = null; + let clickHandler: ((e: Event) => void) | null = null; return { onStart: (props) => { if (!searchCallback) return; if (!props.clientRect) return; component = new ReactRenderer<CommandListInstance, MentionsListDropdownProps>(MentionsListDropdown, { props: { ...props, searchCallback, }, editor: props.editor, }); props.editor.commands.addActiveDropbarExtension(CORE_EXTENSIONS.MENTION); const element = component.element as HTMLElement; element.style.position = "absolute"; element.style.zIndex = "100"; // Add event handlers to prevent modal closing - element.addEventListener("click", (e) => { + clickHandler = (e) => { e.stopPropagation(); - }); + }; + element.addEventListener("click", clickHandler); document.body.appendChild(element); updateFloatingUIFloaterPosition(props.editor, element); }, onUpdate: (props) => { component?.updateProps(props); if (!props.clientRect) return; if (component?.element) { updateFloatingUIFloaterPosition(props.editor, component?.element as HTMLElement); } }, onKeyDown: ({ event }) => { if (event.key === "Escape") { component?.destroy(); component = null; return true; } return component?.ref?.onKeyDown({ event }) ?? false; }, onExit: ({ editor }) => { editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.MENTION); + if (component?.element && clickHandler) { + (component.element as HTMLElement).removeEventListener("click", clickHandler); + } component?.element.remove(); component?.destroy(); + clickHandler = null; }, }; };
🧹 Nitpick comments (1)
packages/editor/src/core/components/menus/block-menu.tsx (1)
209-212: LGTM! zIndex moved inline to 100 for block-menu, matching most floating UIs (floating menu, emoji list, link tooltip, table drag handles). Slash commands and mentions lists still use z-10—verify stacking when menus overlap or consolidate them to the same z-index scale.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/editor/src/core/components/menus/block-menu.tsx(1 hunks)packages/editor/src/core/extensions/mentions/utils.ts(1 hunks)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
Description
Type of Change
Screenshots and Media (if applicable)
Screen.Recording.2025-10-06.at.3.17.17.PM.mov
Test Scenarios
References
Summary by CodeRabbit
Bug Fixes
Style