[WIKI-696] fix: peak view close on click block menu options #7861
Conversation
VipinDevelops
requested review from
Copilot and
aaryan610
and removed request for
Copilot
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughThe block menu now adds the SIDE_MENU dropbar extension on drag-handle activation and removes it when the menu closes or on Escape/outside-click/scroll. Menu item clicks are wrapped to invoke the action, then close the menu and remove the extension. Types are updated to allow SIDE_MENU in active dropbar extensions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant DragHandle as Drag Handle
participant BlockMenu as Block Menu
participant Editor as Editor.commands
participant Dropbar as ActiveDropbarExtensions
User->>DragHandle: Click
DragHandle->>BlockMenu: Open menu
BlockMenu->>Editor: addActiveDropbarExtension(SIDE_MENU)
Editor->>Dropbar: SIDE_MENU active
alt Outside click / Esc / Scroll
User->>BlockMenu: Dismiss interaction
BlockMenu->>Editor: removeActiveDropbarExtension(SIDE_MENU)
BlockMenu-->>User: Close menu
end
rect rgba(230,245,255,0.5)
note right of BlockMenu: Menu item click flow
User->>BlockMenu: Click menu item
BlockMenu->>BlockMenu: Invoke item onClick
BlockMenu->>Editor: removeActiveDropbarExtension(SIDE_MENU)
BlockMenu-->>User: Close menu and stop propagation
end
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
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
🧹 Nitpick comments (4)
packages/editor/src/core/components/menus/block-menu.tsx (4)
73-85: Stabilize hook deps to prevent unnecessary re-binding.Using editor.commands in deps can be unstable and cause listener churn. Prefer editor (which is stable) in the deps for handleClickDragHandle.
Apply:
- [editor.commands, refs] + [editor, refs]
87-99: Ensure SIDE_MENU is always cleared on close/unmount.You clear the extension on several events, but if the menu closes via other paths or the component unmounts while open, SIDE_MENU may remain active. Add explicit cleanup in the effect and a small watcher on isOpen.
Add cleanup on unmount:
return () => { document.removeEventListener("click", handleClickDragHandle); document.removeEventListener("contextmenu", handleClickDragHandle); document.removeEventListener("keydown", handleKeyDown); document.removeEventListener("scroll", handleScroll, true); + // Ensure SIDE_MENU is not left active on unmount + editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.SIDE_MENU); };Also add a watcher so any close path clears it:
+ // Ensure SIDE_MENU is cleared whenever the menu closes via any path + useEffect(() => { + if (!isOpen) { + editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.SIDE_MENU); + } + }, [isOpen, editor]);Also applies to: 104-111
110-110: Use editor (not editor.commands) in effect deps.Prevents unnecessary add/remove of global listeners if commands proxy is non-stable.
- }, [editor.commands, handleClickDragHandle]); + }, [editor, handleClickDragHandle]);
223-229: Call preventDefault/stopPropagation before executing item action.Prevents any default/bubbling side-effects from firing before the action runs.
- onClick={(e) => { - item.onClick(e); - e.preventDefault(); - e.stopPropagation(); - setIsOpen(false); - editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.SIDE_MENU); - }} + onClick={(e) => { + e.preventDefault(); + e.stopPropagation(); + item.onClick(e); + setIsOpen(false); + editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.SIDE_MENU); + }}
📜 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(3 hunks)packages/editor/src/core/extensions/utility.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)
🔇 Additional comments (1)
packages/editor/src/core/extensions/utility.ts (1)
14-20: Approve SIDE_MENU addition
TAdditionalActiveDropbarExtensions is defined asnever, so addingCORE_EXTENSIONS.SIDE_MENUintroduces no duplicate union.
Description
This PR adds logic to use the active modal utility, preventing the issue peek view from closing unexpectedly.
Type of Change
Screenshots and Media (if applicable)
Screen.Recording.2025-09-26.at.6.40.59.PM.mov
Test Scenarios
References
Summary by CodeRabbit
New Features
Bug Fixes