Skip to content

fix(board): isolate OverflowMenu events from dnd-kit drag listeners#174

Merged
ryota-murakami merged 1 commit intomainfrom
fix/overflowmenu-dnd-event-bubbling
Apr 24, 2026
Merged

fix(board): isolate OverflowMenu events from dnd-kit drag listeners#174
ryota-murakami merged 1 commit intomainfrom
fix/overflowmenu-dnd-event-bubbling

Conversation

@ryota-murakami
Copy link
Copy Markdown
Contributor

@ryota-murakami ryota-murakami commented Apr 23, 2026

Summary

  • 🐛 Production bug: clicking Move to Maintenance / Move to Another Board / Remove from Board in a card's overflow menu silently no-ops or opens the unrelated Note modal instead of triggering the action.
  • 🔬 Root cause: RepoCard spreads useSortable attributes and listeners on its wrapper div, so the entire card is a drag handle. Radix renders DropdownMenuContent inside a Portal, but React still bubbles synthetic events from portal children up through the React tree. A pointerdown on a menu item therefore reached MouseSensor on the wrapper. With activationConstraint.distance: 8, even small cursor jitter between pointerdown and pointerup started a drag, and dnd-kit then suppressed the trailing click — so the menu item's onClick never fired.
  • Fix:
    • OverflowMenu now mounts a display: contents wrapper that stops propagation for pointerdown / mousedown / click / keydown. Radix's own handlers still run first because they live deeper in the React tree, but the draggable ancestor never sees menu events.
    • RepoCard.handleKeyDown early-returns when e.target !== e.currentTarget, so portal-forwarded keystrokes (e.g. Enter on a focused menu item) can no longer accidentally open the Note modal.

Why this design

  • Isolation belongs to the menu, not its host. Any future component that reuses OverflowMenu inside a drag context gets the fix for free.
  • display: contents keeps the existing flex layout untouched — no visual diff, no extra spacing.
  • Radix-friendly: stopping propagation above Radix preserves all its internal pointer/keyboard handling. The trigger still opens the menu, items still fire onSelect, the Cancel/Confirm dialog still works.
  • Defense in depth on RepoCard: even if a future ancestor forgets to isolate, the card's own shortcuts won't misfire on bubbled events.

Test plan

  • pnpm vitest run src/tests/unit/components/Board/194/194 pass (5 new tests pinning the new contract):
    • OverflowMenuEvent Propagation Isolation (dnd-kit guard) (4 tests):
      • Renders the display: contents isolation wrapper
      • Click on Move to Maintenance does not bubble click/pointerdown/mousedown to a draggable ancestor
      • Click on the trigger does not bubble to the ancestor
      • Keyboard events on the trigger do not bubble to the ancestor
    • RepoCardKeyboard Navigation (1 test): Enter bubbled from a descendant is ignored (no onNote fired)
  • pnpm typecheck — clean
  • pnpm lint on changed files — clean
  • Manual verification on production after deploy: open a Stable board card menu → Move to Maintenance → card disappears + toast shows "Moved to maintenance"

Files changed

File Change
src/components/Board/OverflowMenu.tsx Add display: contents isolation wrapper
src/components/Board/RepoCard.tsx Defensive e.target !== e.currentTarget guard in handleKeyDown
src/tests/unit/components/Board/OverflowMenu.test.tsx +4 regression tests
src/tests/unit/components/Board/RepoCard.test.tsx +1 regression test

Summary by CodeRabbit

  • Bug Fixes

    • Resolved menu dropdown interactions being interrupted when used on draggable items.
    • Improved keyboard event handling to prevent shortcuts from triggering unexpectedly in nested interactive elements.
  • Tests

    • Added regression tests for menu operation isolation in draggable contexts.
    • Added tests verifying proper keyboard event behavior in card interactions.

Move to Maintenance / Move to Another Board / Remove from Board silently
no-op'd in production. Symptom: clicking an item in the card's overflow
menu would open the Note modal or do nothing at all.

Root cause: RepoCard spreads useSortable `attributes` and `listeners` on
its wrapper div, turning the entire card into a drag handle. Radix
renders DropdownMenuContent inside a Portal, but React still bubbles
synthetic events from portal children up through the React tree. So a
pointerdown on a menu item reached MouseSensor on the wrapper. With
`activationConstraint.distance: 8`, even small cursor jitter between
pointerdown and pointerup triggered a drag, after which dnd-kit
suppressed the trailing click — and the menu item's onClick never fired.

Fix:
- OverflowMenu now mounts a `display: contents` wrapper that stops
  propagation for pointerdown / mousedown / click / keydown so the
  draggable ancestor never sees menu events. Radix's own handlers run
  first because they live deeper in the React tree.
- RepoCard's handleKeyDown now early-returns when `e.target !==
  e.currentTarget`, so portal-forwarded keystrokes (e.g. Enter on a
  focused menu item) can no longer accidentally open the Note modal.

Coverage:
- OverflowMenu: 4 new "Event Propagation Isolation" tests verify the
  guard short-circuits ancestor handlers for clicks (trigger + items)
  and keystrokes.
- RepoCard: 1 new test pins the bubbled-Enter early return.
- All 194 Board unit tests, typecheck, and lint pass.
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitbox Ready Ready Preview, Comment Apr 23, 2026 11:57pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c8a48d8f-7004-4092-b40c-2e00d7032b9f

📥 Commits

Reviewing files that changed from the base of the PR and between 552ef73 and e9ad717.

📒 Files selected for processing (4)
  • src/components/Board/OverflowMenu.tsx
  • src/components/Board/RepoCard.tsx
  • src/tests/unit/components/Board/OverflowMenu.test.tsx
  • src/tests/unit/components/Board/RepoCard.test.tsx

📝 Walkthrough

Walkthrough

Event propagation fixes prevent OverflowMenu dropdown interactions and RepoCard keyboard events from bubbling to draggable ancestors (dnd-kit) or triggering unintended modals. Event handlers stop propagation for pointer, mouse, click, and key down events. Regression tests verify isolation and correct callback invocation.

Changes

Cohort / File(s) Summary
OverflowMenu Event Isolation
src/components/Board/OverflowMenu.tsx
Wraps dropdown and alert in a div with stopPointerBubble handler applied to onPointerDown, onMouseDown, onClick, and onKeyDown to prevent event bubbling to draggable ancestors. Adds data-testid for test isolation.
RepoCard Keyboard Event Validation
src/components/Board/RepoCard.tsx
Updates handleKeyDown to return early when event target differs from currentTarget, preventing descendant-originated Enter events from triggering the note modal. Includes documentation of portal-descendant bubbling issue.
Event Propagation Regression Tests
src/tests/unit/components/Board/OverflowMenu.test.tsx, src/tests/unit/components/Board/RepoCard.test.tsx
Adds tests asserting isolation wrapper presence, verifying menu interactions don't bubble to draggable ancestors, confirming keyboard activation of trigger doesn't bubble, and ensuring descendant Enter events don't invoke note callback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🛡️ Events contained, no more spillover,

Bubbles burst before they grow,

Dropdowns dance where roots won't follow,

Tests ensure the chain stays low. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main fix: preventing OverflowMenu events from bubbling to dnd-kit drag listeners, which is the core production bug being resolved.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/overflowmenu-dnd-event-bubbling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.16%. Comparing base (552ef73) to head (e9ad717).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
+ Coverage   70.13%   70.16%   +0.03%     
==========================================
  Files         161      161              
  Lines        4654     4659       +5     
  Branches     1227     1203      -24     
==========================================
+ Hits         3264     3269       +5     
  Misses       1371     1371              
  Partials       19       19              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

🧪 E2E Coverage Report (Sharded: 12 parallel jobs)

Metric Coverage
Lines 93.18%
Functions 21.9%
Branches 18.26%
Statements 31.62%

📊 Full report available in workflow artifacts

@ryota-murakami ryota-murakami merged commit ba9aec3 into main Apr 24, 2026
20 checks passed
@ryota-murakami ryota-murakami deleted the fix/overflowmenu-dnd-event-bubbling branch April 24, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants