Skip to content

fix(Header/DashboardSidebar/Sidebar): allow auto focus in menu for proper focus trapping#6266

Merged
benjamincanac merged 3 commits intonuxt:v4from
faizkhairi:fix/header-modal-focus-trap
Apr 7, 2026
Merged

fix(Header/DashboardSidebar/Sidebar): allow auto focus in menu for proper focus trapping#6266
benjamincanac merged 3 commits intonuxt:v4from
faizkhairi:fix/header-modal-focus-trap

Conversation

@faizkhairi
Copy link
Copy Markdown
Contributor

@faizkhairi faizkhairi commented Mar 30, 2026

Description

Fixes #6257

When UHeader, UDashboardSidebar, and USidebar open their mobile menu modal via keyboard, focus should automatically move inside the modal content. Instead, the onOpenAutoFocus handler in menuProps called e.preventDefault(), which suppressed reka-ui's Dialog auto-focus behavior. This caused Tab navigation to traverse background page content before the focus trap engaged.

Root Cause

All three components configured the menu's content props with:

content: {
  onOpenAutoFocus: (e: Event) => e.preventDefault()
}

This blocks the default WAI-ARIA Dialog focus management that reka-ui provides.

Fix

Removed the onOpenAutoFocus handler from menuProps defaults in Header.vue, DashboardSidebar.vue, and Sidebar.vue, allowing reka-ui to handle focus management correctly on modal open (auto-focus first focusable element inside the dialog).

Opt-out

Users who need the previous behavior can still suppress auto-focus via the menu prop:

<UHeader :menu="{ content: { onOpenAutoFocus: (e) => e.preventDefault() } }" />

Since defu merges user-provided values with highest priority, this overrides the default.

WCAG Reference

  • WCAG 2.4.3 - Focus Order: Focus must move in a meaningful sequence. Tabbing through hidden background content violates this.

Remove the onOpenAutoFocus preventDefault handler from menuProps
that was blocking reka-ui's Dialog auto-focus behavior.

When the header modal opens via keyboard, focus should move inside
the modal for proper focus trapping. The previous handler prevented
this, causing Tab navigation to traverse background page content
before the focus trap engaged, violating WCAG 2.4.3 (Focus Order).

Users who need the old behavior can opt in via the menu prop:
<UHeader :menu="{ content: { onOpenAutoFocus: (e) => e.preventDefault() } }" />

Resolves nuxt#6257
@github-actions github-actions bot added the v4 #4488 label Mar 30, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

📝 Walkthrough

Walkthrough

This change removes the onOpenAutoFocus event handler from the default content configuration across three sidebar/header menu components. Previously, Header.vue, DashboardSidebar.vue, and Sidebar.vue were injecting a default handler that prevented automatic focus management when opening menus and modals. The modifications remove this override, allowing default focus behavior to operate naturally instead of being suppressed by the injected handler.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the bug, root cause, the fix applied, provides an opt-out example, and references relevant WCAG guidelines for accessibility.
Linked Issues check ✅ Passed The changes fully address issue #6257 by removing the onOpenAutoFocus suppression from Header, DashboardSidebar, and Sidebar to allow reka-ui's auto-focus behavior.
Out of Scope Changes check ✅ Passed All changes are narrowly focused on removing the onOpenAutoFocus handler from menuProps in three related components, directly addressing the linked issue with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately summarizes the main change: allowing auto-focus in modal menus across three components (Header, DashboardSidebar, Sidebar) for proper focus trapping accessibility.

✏️ 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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/components/Header.spec.ts (1)

44-55: Current assertion doesn’t verify the auto-focus contract.

Line 54 only checks render truthiness; it won’t catch regressions in focus movement/focus trap behavior.

Suggested test strengthening
+import { nextTick } from 'vue'
 import { describe, it, expect } from 'vitest'
 import { axe } from 'vitest-axe'
 import { mountSuspended } from '@nuxt/test-utils/runtime'
@@
-  it('does not suppress auto-focus in modal menu props', async () => {
+  it('moves focus inside the modal when opened', async () => {
     const wrapper = await mountSuspended(Header, {
       props: {
         open: true,
         mode: 'modal',
         menu: { portal: false }
       }
     })
 
-    // Verify the modal content is rendered when open
-    expect(wrapper.html()).toBeTruthy()
+    await nextTick()
+    const dialog = wrapper.find('[role="dialog"]')
+    expect(dialog.exists()).toBe(true)
+    expect(dialog.element.contains(document.activeElement)).toBe(true)
   })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/components/Header.spec.ts` around lines 44 - 55, The test only checks
rendering but not autofocus behavior; after mounting Header (with open: true,
mode: 'modal', menu: { portal: false }) await rendering (nextTick or
mountSuspended resolution), then locate the element that should receive
autofocus (e.g., find('input[autofocus]') or querySelector('[autofocus]') / a
known data-testid inside the Header component) and assert that
document.activeElement is that element or that the modal contains
document.activeElement to verify focus moved into the modal; update the spec in
Header.spec.ts to perform this focused-element assertion after mount to catch
regressions in focus trapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/components/Header.spec.ts`:
- Around line 44-55: The test only checks rendering but not autofocus behavior;
after mounting Header (with open: true, mode: 'modal', menu: { portal: false })
await rendering (nextTick or mountSuspended resolution), then locate the element
that should receive autofocus (e.g., find('input[autofocus]') or
querySelector('[autofocus]') / a known data-testid inside the Header component)
and assert that document.activeElement is that element or that the modal
contains document.activeElement to verify focus moved into the modal; update the
spec in Header.spec.ts to perform this focused-element assertion after mount to
catch regressions in focus trapping.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f82ccd49-febe-4a6c-aa0d-0013d7bd8829

📥 Commits

Reviewing files that changed from the base of the PR and between 2f1ad02 and d8cfd18.

📒 Files selected for processing (2)
  • src/runtime/components/Header.vue
  • test/components/Header.spec.ts

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 30, 2026

npm i https://pkg.pr.new/@nuxt/ui@6266

commit: b9a7dc1

Verify the dialog element with role="dialog" is rendered when the
modal menu is open, addressing CodeRabbit review feedback.
@benjamincanac
Copy link
Copy Markdown
Member

benjamincanac commented Apr 2, 2026

When does that happen exactly? You should not be able to navigate to the mobile menu with keyboard on mobile 🤔

@Oui-Dev
Copy link
Copy Markdown

Oui-Dev commented Apr 3, 2026

When does that happen exactly? You should not be able to navigate to the mobile menu with keyboard on mobile 🤔

Even on mobile layouts, the menu can still be used with a keyboard (e.g. via bluetooth keyboards) or assistive technologies like screen readers.

Without a proper focus trap, keyboard and screen reader users (especially visually impaired or blind people) can navigate outside the open menu, which breaks the expected interaction and makes it confusing to use.

Also, since this applies to all viewports below the lg breakpoint, the issue can affect desktop users as well when resizing their browser.

@faizkhairi
Copy link
Copy Markdown
Contributor Author

As @Oui-Dev mentioned, this isn't just about touch-only mobile. The mobile menu shows up for all viewports below lg, so desktop users with narrow windows hit this too.

The issue is onOpenAutoFocus: (e) => e.preventDefault() blocks reka-ui's auto-focus on dialog open. Focus stays on the trigger instead of moving into the modal, so Tab goes through background content before the trap kicks in.

Removing the handler lets reka-ui do its thing - first focusable element gets focused on open, standard dialog behavior.

Anyone who needs the old behavior can still opt out:

<UHeader :menu="{ content: { onOpenAutoFocus: (e) => e.preventDefault() } }" />

Copy link
Copy Markdown
Member

I see, would you mind applying the same changes to DashboardSidebar and Sidebar which have the same behavior then? (I don't think the test is necessary though)

… proper focus trapping

Apply the same fix from Header to DashboardSidebar and Sidebar components.
Remove unnecessary Header modal test per maintainer feedback.
@faizkhairi
Copy link
Copy Markdown
Contributor Author

Done - applied the same change to DashboardSidebar and Sidebar, and removed the test.

b9a7dc1

@faizkhairi faizkhairi changed the title fix(Header): allow auto-focus in modal menu for proper focus trapping fix(Header,DashboardSidebar,Sidebar): allow auto-focus in modal menu for proper focus trapping Apr 3, 2026
@Oui-Dev
Copy link
Copy Markdown

Oui-Dev commented Apr 3, 2026

Is it also possible to modify the doc at the same time regarding this point? Or would you prefer that I open a separate issue?
#6257 (comment)
image

@faizkhairi
Copy link
Copy Markdown
Contributor Author

That doc mismatch looks like a separate concern from the focus trap fix here - probably best as its own issue so it gets tracked properly. Feel free to tag me (@faizkhairi) if you open one, happy to take a look.

@benjamincanac benjamincanac changed the title fix(Header,DashboardSidebar,Sidebar): allow auto-focus in modal menu for proper focus trapping fix(Header/DashboardSidebar/Sidebar): allow auto-focus in modal menu for proper focus trapping Apr 7, 2026
@benjamincanac benjamincanac merged commit 9b91ee4 into nuxt:v4 Apr 7, 2026
18 checks passed
@benjamincanac benjamincanac changed the title fix(Header/DashboardSidebar/Sidebar): allow auto-focus in modal menu for proper focus trapping fix(Header/DashboardSidebar/Sidebar): allow auto focus in menu for proper focus trapping Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v4 #4488

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UHeader accessibility issue in modal mode

3 participants