-
Notifications
You must be signed in to change notification settings - Fork 638
get ActionBar axe clean #4388
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
get ActionBar axe clean #4388
Conversation
🦋 Changeset detectedLatest commit: 8bdbb17 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
size-limit report 📦
|
<ActionBarContext.Provider value={{size, setChildrenWidth}}> | ||
<Box ref={navRef} sx={getNavStyles()}> | ||
<NavigationList sx={ulStyles} ref={listRef} role="list"> | ||
<NavigationList sx={listStyles} ref={listRef} role="toolbar"> |
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.
Nice, toolbar makes total sense here. I tested the arrow focusing and it looks fine to me 👍
it('should have no axe violations', async () => { | ||
const {container} = HTMLRender(<SimpleActionBar />) | ||
const results = await axe(container) | ||
expect(results).toHaveNoViolations() | ||
}) |
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.
Just wanted to share that axe is also available in the e2e tests but with the additional github rules if that's of interest! Wasn't sure how obvious it was.
* get ActionBar axe clean * changeset * fix type * fix type
Closes #4387
Changelog
New
Changed
ActionBar is now
role=toolbar
, and produces valid HTML.Removed
Rollout strategy
Testing & Reviewing
Merge checklist