fix(Menu): allow Tab to move focus out of non-modal DropdownMenu by yan-ad · Pull Request #2747 · unovue/reka-ui

yan-ad · 2026-06-23T09:06:45Z

Description

Fixes #2296 — Unable to use Tab to move focus outside of open, non-modal DropdownMenu.

Root Cause

MenuContentImpl.vue unconditionally called event.preventDefault() on Tab keydown when inside the menu. This was intended to prevent Tab from navigating between menu items (menus use arrow keys), but it had the side effect of completely disabling Tab — focus could never leave the menu.

Fix

Gate the Tab preventDefault on trapFocus.value. When the menu is modal (trapFocus=true), Tab is still prevented (FocusScope handles focus trapping). When non-modal (trapFocus=false, which is the default for DropdownMenu), Tab is allowed to naturally move focus to the next focusable element on the page, per the WAI-ARIA menubar keyboard interaction pattern.

Testing

All 21 tests pass: Menu (6), DropdownMenu (6), ContextMenu (3), Menubar (6)

Summary by CodeRabbit

Bug Fixes
- Fixed keyboard Tab behavior in non-modal menus so users can move focus forward instead of being trapped.
- Preserved Tab focus handling for modal menus, including nested submenus, preventing unintended focus changes.
Tests
- Added tab-navigation coverage validating Tab behavior for modal vs non-modal menus and nested submenu keydown prevention.

The Tab key was unconditionally prevented in MenuContentImpl, blocking focus from leaving even non-modal menus. Tab should still be prevented for modal menus (trapFocus=true, handled by FocusScope). Now Tab key default prevention is gated on trapFocus.value, so non-modal menus allow Tab to naturally move focus to the next focusable element on the page. Closes unovue#2296

coderabbitai · 2026-06-23T09:07:05Z

📝 Walkthrough

Walkthrough

MenuContentImpl.vue now prevents Tab only for modal menus. DropdownMenu.test.ts adds coverage for Tab key behavior in modal and non-modal dropdowns, including submenu cases.

Changes

DropdownMenu Tab behavior

Layer / File(s)	Summary
Conditional Tab handling in menu content `packages/core/src/Menu/MenuContentImpl.vue`	`handleKeyDown` now guards Tab prevention with `rootContext.modal.value` instead of blocking Tab unconditionally.
DropdownMenu Tab navigation tests `packages/core/src/DropdownMenu/DropdownMenu.test.ts`	Adds an open dropdown test fixture and Tab keydown tests that assert default prevention for modal menus and native Tab behavior for non-modal menus.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A tab once bounced and found its way,
In modal dens it still may stay.
But open paths now softly gleam,
And focus hops like a springtime dream.

🚥 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 clearly states the main behavior change: Tab is allowed to leave non-modal DropdownMenu.
Linked Issues check	✅ Passed	The code and tests address `#2296` by allowing Tab to escape non-modal menus while still preventing Tab in modal menus.
Out of Scope Changes check	✅ Passed	The changes stay focused on the Tab-key fix and targeted regression tests, with no unrelated scope visible.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with 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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

pkg-pr-new · 2026-06-23T09:08:14Z

Open in StackBlitz

npm i https://pkg.pr.new/reka-ui@2747

commit: e64e914

zernonia · 2026-06-24T13:27:15Z

Thanks for the fix — the gating on trapFocus looks correct for the non-modal case 👍

Could you add a regression test before we merge? There's currently no Tab-keydown coverage in the Menu/DropdownMenu suites, so "all tests pass" doesn't actually exercise this behavior. Ideally:

non-modal menu: Tab is not preventDefault'd and focus moves to the next focusable element
modal menu: Tab is still prevented

One more thing to confirm: submenus omit trapFocus, so trapFocus.value is undefined (falsy) for any MenuSubContent. That means Tab will now also escape an open submenu inside a modal menu. Is that intended? A submenu test case would be great to lock down whichever behavior we want.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/core/src/DropdownMenu/DropdownMenu.test.ts (1)
117-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the repository’s test stack conventions for this suite.

These new interaction tests are written with mount; guidelines for packages/core/src/**/*.test.ts require @testing-library/vue (and vitest-axe where applicable). Please align this block to that stack for consistency.

As per coding guidelines, "packages/core/src/**/*.test.ts: Write tests using vitest, jsdom, @testing-library/vue, and vitest-axe; include axe accessibility checks for new components".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/DropdownMenu/DropdownMenu.test.ts` around lines 117 - 170,
The new DropdownMenu interaction tests are using mount instead of the
repository’s required testing stack. Update the DropdownMenuTabTest coverage to
use vitest with jsdom and `@testing-library/vue` queries/events, matching the
conventions used in packages/core/src/**/*.test.ts, and add vitest-axe
accessibility coverage where appropriate. Keep the same Tab prevention
assertions by locating the menu/submenu through user-facing queries rather than
wrapper.findAll/mount-specific APIs.
Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/DropdownMenu/DropdownMenu.test.ts`:
- Around line 116-134: The Tab focus test in DropdownMenuTabTest is
self-fulfilling because it manually focuses the “After” button before asserting
activeElement. Update the test so it verifies actual Tab behavior in
DropdownMenu.test by either removing the activeElement assertion and only
checking that the keydown event is not prevented, or by using a real
tab-navigation helper to move focus out of the non-modal menu before asserting
the focused button.

---

Nitpick comments:
In `@packages/core/src/DropdownMenu/DropdownMenu.test.ts`:
- Around line 117-170: The new DropdownMenu interaction tests are using mount
instead of the repository’s required testing stack. Update the
DropdownMenuTabTest coverage to use vitest with jsdom and `@testing-library/vue`
queries/events, matching the conventions used in packages/core/src/**/*.test.ts,
and add vitest-axe accessibility coverage where appropriate. Keep the same Tab
prevention assertions by locating the menu/submenu through user-facing queries
rather than wrapper.findAll/mount-specific APIs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1e0f432e-8651-42f5-8d2c-bb4ccf70c28b

📥 Commits

Reviewing files that changed from the base of the PR and between 8b077cd and cc63268.

📒 Files selected for processing (2)

packages/core/src/DropdownMenu/DropdownMenu.test.ts
packages/core/src/Menu/MenuContentImpl.vue

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/DropdownMenu/DropdownMenu.test.ts`:
- Line 2: The named import order in DropdownMenu.test.ts violates
perfectionist/sort-named-imports. Reorder the imported testing-library symbols
in the existing import statement so the named imports are sorted consistently,
keeping the change limited to the import line in DropdownMenu.test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 37b2b2f1-89e7-4a84-8dfe-8ab27ca8121b

📥 Commits

Reviewing files that changed from the base of the PR and between cc63268 and e64e914.

📒 Files selected for processing (2)

packages/core/src/DropdownMenu/DropdownMenu.test.ts
packages/core/src/Menu/MenuContentImpl.vue

🚧 Files skipped from review as they are similar to previous changes (1)

packages/core/src/Menu/MenuContentImpl.vue

coderabbitai Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread packages/core/src/DropdownMenu/DropdownMenu.test.ts

fix(Menu): preserve Tab trap in modal submenus

e64e914

yan-ad force-pushed the fix/dropdown-tab-navigation branch from cc63268 to e64e914 Compare June 25, 2026 03:12

coderabbitai Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread packages/core/src/DropdownMenu/DropdownMenu.test.ts

zernonia merged commit 8567787 into unovue:v2 Jun 26, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(Menu): allow Tab to move focus out of non-modal DropdownMenu#2747

fix(Menu): allow Tab to move focus out of non-modal DropdownMenu#2747
zernonia merged 2 commits into
unovue:v2from
yan-ad:fix/dropdown-tab-navigation

yan-ad commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

pkg-pr-new Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

zernonia commented Jun 24, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

yan-ad commented Jun 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Root Cause

Fix

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

pkg-pr-new Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zernonia commented Jun 24, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yan-ad commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading

pkg-pr-new Bot commented Jun 23, 2026 •

edited

Loading