-
Notifications
You must be signed in to change notification settings - Fork 54
Improve keyboard navigation in SubMenu anchor variant #1084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 0ff254a The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🟢 No design token changes found |
531bdbb
to
2dec325
Compare
2dec325
to
0ff254a
Compare
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves keyboard navigation for the SubNav
anchor variant by making anchor-based submenus visible when focused and adding a Storybook interaction test to verify focus order.
- Refactors the anchor variant story to use
StoryFn
typing and a bind-based template - Adds a new interaction test (
AnchorNavVariantKeyboardNavigation.play
) that tabs through each link and verifies focus - Updates the changeset to document the keyboard navigation enhancement
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
packages/react/src/SubNav/SubNav.features.stories.tsx | Refactored story definitions, renamed template, added keyboard navigation play test |
.changeset/breezy-shrimps-sell.md | Added changeset entry describing improved keyboard navigation |
Files not reviewed (1)
- packages/react/src/SubNav/SubNav.module.css: Language not supported
Comments suppressed due to low confidence (1)
packages/react/src/SubNav/SubNav.features.stories.tsx:494
- Consider also asserting that the anchor navigation container is visible when focused (e.g., using
.toBeVisible()
) to fully validate the visibility behavior introduced in this PR.
expect(getAllByRole('button', {name: 'Learn more'})[0]).toHaveFocus()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great @joshfarrant 👍
Summary
visibility: hidden
from anchor nav to allow it to appear after the mainSubNav
items in the focus orderWhat should reviewers focus on?
Steps to test
Supporting resources (related issues, external links, etc)
Contributor checklist
update snapshots
label to the PR)Reviewer checklist