[WEB-5068] fix: CustomMenu closeOnSelect behavior#7897
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughReplaced onClick handlers across multiple React components to remove explicit event.preventDefault() and stopPropagation() calls. Handlers now invoke actions directly (often with captureClick first). No exported API changes, except localized handler signature simplifications where noted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant M as CustomMenu.MenuItem
participant T as Tracker (captureClick)
participant A as Action
rect rgb(240,248,255)
note over M: Before
U->>M: click
M->>M: preventDefault() / stopPropagation()
M->>T: captureClick(...)
M->>A: item.action()
end
rect rgb(245,255,240)
note over M: After
U->>M: click
M->>T: captureClick(...) (when present)
M->>A: item.action()
note over U,M: Default event flow proceeds (no explicit suppression)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 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)
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.
Pull Request Overview
Fixed CustomMenu.MenuItem onClick handlers by removing preventDefault() and stopPropagation() calls that were preventing the closeOnSelect prop from functioning correctly.
- Removed event.preventDefault() and event.stopPropagation() calls from CustomMenu.MenuItem onClick handlers
- Simplified onClick handlers to use arrow functions without event parameters where the event object is not needed
- Applied the fix consistently across multiple components that use CustomMenu.MenuItem
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| view-list-item.tsx | Removed event handling calls from two CustomMenu.MenuItem onClick handlers |
| quick-action.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| default-view-quick-action.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| invitations-list-item.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| views/quick-actions.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| pages/dropdowns/actions.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| modules/quick-actions.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| workspace-draft/quick-action.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| project-issue.tsx | Removed event handling calls from multiple CustomMenu.MenuItem onClick handlers |
| module-issue.tsx | Removed event handling calls from multiple CustomMenu.MenuItem onClick handlers |
| issue-detail.tsx | Removed event handling calls from multiple CustomMenu.MenuItem onClick handlers |
| cycle-issue.tsx | Removed event handling calls from multiple CustomMenu.MenuItem onClick handlers |
| archived-issue.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| all-issue.tsx | Removed event handling calls from multiple CustomMenu.MenuItem onClick handlers |
| link-item.tsx | Removed event handling calls from two CustomMenu.MenuItem onClick handlers |
| quick-action-button.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| list-item.tsx | Removed event handling calls from multiple CustomMenu.MenuItem onClick handlers |
| relations/quick-action-button.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| attachment-list-item.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
| cycles/quick-actions.tsx | Removed event handling calls from CustomMenu.MenuItem onClick handler |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![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 (1)
apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/archived-issue.tsx (1)
97-100: Tracking order inconsistent with other files.The action is invoked before
captureClick(), whereas other files in this PR (e.g.,issue-detail.tsxlines 280-281,all-issue.tsxlines 179-180) callcaptureClick()first, then the action. While this doesn't break functionality, consistent ordering helps with tracking accuracy.Apply this diff for consistency:
onClick={() => { - item.action(); captureClick({ elementName: WORK_ITEM_TRACKER_ELEMENTS.QUICK_ACTIONS.ARCHIVED }); + item.action(); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/web/core/components/cycles/quick-actions.tsx(1 hunks)apps/web/core/components/issues/attachment/attachment-list-item.tsx(1 hunks)apps/web/core/components/issues/issue-detail-widgets/relations/quick-action-button.tsx(1 hunks)apps/web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx(4 hunks)apps/web/core/components/issues/issue-detail-widgets/sub-issues/quick-action-button.tsx(1 hunks)apps/web/core/components/issues/issue-detail/links/link-item.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/all-issue.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/archived-issue.tsx(1 hunks)apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/cycle-issue.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/issue-detail.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/module-issue.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/project-issue.tsx(2 hunks)apps/web/core/components/issues/workspace-draft/quick-action.tsx(1 hunks)apps/web/core/components/modules/quick-actions.tsx(1 hunks)apps/web/core/components/pages/dropdowns/actions.tsx(1 hunks)apps/web/core/components/views/quick-actions.tsx(1 hunks)apps/web/core/components/workspace/settings/invitations-list-item.tsx(1 hunks)apps/web/core/components/workspace/views/default-view-quick-action.tsx(1 hunks)apps/web/core/components/workspace/views/quick-action.tsx(1 hunks)apps/web/core/components/workspace/views/view-list-item.tsx(2 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: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (26)
apps/web/core/components/issues/workspace-draft/quick-action.tsx (1)
34-36: LGTM! Clean removal of event control.The onClick handler correctly removes
preventDefaultandstopPropagation, allowing thecloseOnSelectprop to function as intended while still executing the item action.apps/web/core/components/modules/quick-actions.tsx (1)
200-205: LGTM! Event flow correctly restored.The handler now allows the menu's
closeOnSelectbehavior to work while preserving click tracking and action invocation.apps/web/core/components/views/quick-actions.tsx (1)
98-101: LGTM! Consistent with PR objective.The simplified onClick handler enables the
closeOnSelectfunctionality while maintaining click analytics.apps/web/core/components/cycles/quick-actions.tsx (1)
226-231: LGTM! Pattern correctly applied.The refactored handler allows menu closure on selection while preserving event tracking.
apps/web/core/components/issues/attachment/attachment-list-item.tsx (1)
86-88: LGTM! Simplified correctly.The handler now allows the menu to close on selection after invoking the delete modal.
apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/module-issue.tsx (2)
194-197: LGTM! Nested menu handler fixed.The submenu item onClick correctly allows menu closure while tracking clicks and executing actions.
230-233: LGTM! Regular menu handler fixed.The top-level menu item onClick now enables proper
closeOnSelectbehavior.apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/cycle-issue.tsx (2)
195-198: LGTM! Submenu handler corrected.The nested menu item now allows the menu to close on selection as expected.
231-234: LGTM! Top-level handler corrected.The regular menu item onClick enables proper closure behavior.
apps/web/core/components/issues/issue-detail-widgets/sub-issues/quick-action-button.tsx (1)
89-91: LGTM! Handler correctly simplified.The onClick now allows the
closeOnSelectprop to function properly while invoking the item's action.apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/issue-detail.tsx (1)
279-282: LGTM! Event handling correctly simplified.The onClick handlers now directly invoke actions without suppressing event propagation, which allows the
closeOnSelectprop (line 242) to function correctly. The tracking call is preserved and executed before the action in both nested and regular menu items.Also applies to: 315-318
apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/all-issue.tsx (1)
178-181: LGTM! Consistent refactor for closeOnSelect.The onClick handlers are correctly simplified to allow event propagation, enabling the
closeOnSelectprop (line 141) to work as intended. The tracking logic remains intact.Also applies to: 214-217
apps/web/core/components/workspace/views/view-list-item.tsx (1)
54-59: Verify Link navigation behavior.The onClick handlers no longer prevent default behavior. Since these menu items are rendered within a
Linkcomponent (line 41), clicks will now propagate and potentially trigger navigation to the link'shrefwhile opening the modal. This differs from the previous behavior wherepreventDefault()would have blocked navigation.Verify that this is the intended behavior. If menu actions should not trigger navigation, consider one of these approaches:
onClick={(e) => { e.preventDefault(); captureClick({ elementName: GLOBAL_VIEW_TRACKER_ELEMENTS.LIST_ITEM }); setUpdateViewModal(true); }}Or restructure the component so the CustomMenu is outside the Link wrapper.
Also applies to: 67-72
apps/web/core/components/workspace/settings/invitations-list-item.tsx (1)
188-190: LGTM! Handler simplified correctly.The onClick handler directly invokes the action, allowing the
closeOnSelectprop (line 182) to function properly. The tracking logic is embedded in the action definitions (e.g., line 99-102), so the refactor maintains the intended behavior.apps/web/core/components/workspace/views/default-view-quick-action.tsx (1)
64-66: LGTM! Simplified handler enables closeOnSelect.The onClick handler correctly invokes the action without event suppression, allowing the
closeOnSelectprop (line 56) to work as intended.apps/web/core/components/issues/issue-detail-widgets/relations/quick-action-button.tsx (1)
53-55: LGTM! Handler correctly refactored.The onClick handler now directly invokes
handleOnClick()without event suppression, enabling thecloseOnSelectprop (line 45) to function correctly.apps/web/core/components/pages/dropdowns/actions.tsx (1)
220-222: LGTM! Handler with safe optional chaining.The onClick handler correctly uses optional chaining to invoke the action, allowing the
closeOnSelectprop (line 214) to function properly. The tracking logic is embedded in the action definitions (e.g., lines 98-101).apps/web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx (4)
188-191: LGTM!The onClick handler correctly invokes the edit action directly without event interference, allowing the CustomMenu's closeOnSelect behavior to function properly.
201-203: LGTM!The onClick handler correctly invokes the copy link action directly, allowing the CustomMenu's closeOnSelect behavior to function properly.
213-216: LGTM!The onClick handler correctly invokes the remove action directly while preserving the project_id guard, allowing the CustomMenu's closeOnSelect behavior to function properly.
229-232: LGTM!The onClick handler correctly invokes the delete action directly without event interference, allowing the CustomMenu's closeOnSelect behavior to function properly.
apps/web/core/components/issues/issue-detail/links/link-item.tsx (2)
98-100: LGTM!The onClick handler correctly invokes the edit action directly without event interference, allowing the CustomMenu's closeOnSelect behavior to function properly.
107-109: LGTM!The onClick handler correctly invokes the delete action directly, allowing the CustomMenu's closeOnSelect behavior to function properly.
apps/web/core/components/issues/issue-layouts/quick-action-dropdowns/project-issue.tsx (2)
195-198: LGTM!The onClick handler correctly invokes the analytics tracking followed by the nested item action, without event interference, allowing the CustomMenu's closeOnSelect behavior to function properly.
231-234: LGTM!The onClick handler correctly invokes the analytics tracking followed by the item action, without event interference, allowing the CustomMenu's closeOnSelect behavior to function properly.
apps/web/core/components/workspace/views/quick-action.tsx (1)
74-79: LGTM!The onClick handler correctly invokes the analytics tracking followed by the item action, without event interference, allowing the CustomMenu's closeOnSelect behavior to function properly.
Description
Fixed
CustomMenu.MenuItemonClick handlers by removinge.preventDefault()ande.stopPropagation()calls that were preventing thecloseOnSelectprop from functioning correctly.Type of Change
Summary by CodeRabbit
Refactor
Bug Fixes