Skip to content

fix(NavigationMenu): keyboard navigation in vertical orientation#6605

Open
itsmelouis wants to merge 1 commit into
nuxt:v4from
itsmelouis:fix/navigation-menu-vertical-keyboard
Open

fix(NavigationMenu): keyboard navigation in vertical orientation#6605
itsmelouis wants to merge 1 commit into
nuxt:v4from
itsmelouis:fix/navigation-menu-vertical-keyboard

Conversation

@itsmelouis

Copy link
Copy Markdown

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Keyboard navigation is broken in vertical orientation as soon as an item has children.

The reason: the trailing chevron of those items is rendered as <AccordionTrigger as="span">. A <span> is not focusable, but it still gets reka's data-reka-collection-item, and the Accordion uses that attribute for its arrow-key (roving) navigation. So when you press Arrow Up/Down, focus tries to move to that span, can't (it's not focusable), and stays stuck there, you can't reach the next item. A link with children also can't be expanded/collapsed with the keyboard at all (the only toggle is that unreachable span).
I fixed it the same way ContentNavigation already does it: in vertical, items with children are now rendered as a real focusable AccordionTrigger (<button>), and the chevron is just a decorative icon inside it. No more phantom non-focusable trigger → arrow keys work again and the section can be toggled with the keyboard.
One small behavior change: a link that also has children in vertical now toggles the accordion instead of navigating, which matches horizontal and ContentNavigation. type: 'trigger' is no longer required for that (I kept it for backward compat).
I added a regression test that checks every arrow-key target in vertical is a focusable a/button.

Note: I used an LLM to help me dig into the root cause and put the fix + test together

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@github-actions github-actions Bot added the v4 #4488 label Jun 18, 2026
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c3152cd1-fcd2-43af-994e-89ba00a1b33f

📥 Commits

Reviewing files that changed from the base of the PR and between ed4712c and 26d8ad1.

⛔ Files ignored due to path filters (2)
  • test/components/__snapshots__/NavigationMenu-vue.spec.ts.snap is excluded by !**/*.snap
  • test/components/__snapshots__/NavigationMenu.spec.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • src/runtime/components/NavigationMenu.vue
  • test/components/NavigationMenu.spec.ts

📝 Walkthrough

Walkthrough

NavigationMenu.vue is updated so that in vertical orientation, all items with children unconditionally render as AccordionTrigger components, removing the prior dependency on item.type === 'trigger'. The onLinkTrailingClick handler no longer calls stopPropagation() for the vertical && !collapsed case; preventDefault() is now applied only for horizontal orientation. The trailing element wrapper changes from a dynamic component to a plain <span> with an updated visibility condition. A regression test is added to NavigationMenu.spec.ts verifying that every [data-reka-collection-item] in vertical orientation is a natively focusable element (A or BUTTON) with no nested interactive controls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main fix: improving keyboard navigation in the NavigationMenu component specifically for vertical orientation.
Description check ✅ Passed The description provides a comprehensive explanation of the keyboard navigation bug, the root cause, the solution, behavioral changes, and includes testing details.
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 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.

@pkg-pr-new

pkg-pr-new Bot commented Jun 18, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/@nuxt/ui@6605

commit: 26d8ad1

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.

1 participant