fix(core): expose aria-expanded and aria-controls on MobileNav toggle#3721
Open
bhamodi wants to merge 1 commit into
Open
fix(core): expose aria-expanded and aria-controls on MobileNav toggle#3721bhamodi wants to merge 1 commit into
bhamodi wants to merge 1 commit into
Conversation
|
@bhamodi is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
25691b5 to
89c294e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MobileNavTogglerendered aButtonwith anaria-labelandonClick, but exposed noaria-expandedand noaria-controls(packages/core/src/MobileNav/MobileNavToggle.tsx:69). Screen-reader users had no way to tell whether the nav drawer was currently open, nor which element the hamburger button controls.Making
aria-controlsresolve required a shared id, which did not exist: the AppShell mobile context (useAppShellMobile()) exposed no drawer id, and theMobileNav<dialog>had noidat all. This wires that up end to end:AppShellgenerates a stableuseId()and publishes it on the mobile context asmobileNavId.MobileNavapplies that id to its<dialog>(falling back to a locally generateduseId()when used standalone outside AppShell).MobileNavTogglesetsaria-expanded={isMobileNavOpen}andaria-controls={mobileNavId}.aria-controlsis set unconditionally here (unlike the conditionally-mounted region in #3719): the native<dialog>stays in the DOM whenever the layout is below the mobile breakpoint -- it is mounted via React<Activity mode="hidden">(or a plain fragment fallback) and simply hidden when closed -- so the referenced id always resolves. Purely additive; no behavior or visual change.Changes
MobileNav/MobileNavToggle.tsx: forwardaria-expanded+aria-controlsfrom context onto the toggle button.MobileNav/MobileNav.tsx: apply the sharedmobileNavId(with a standaloneuseId()fallback) as the<dialog>id.AppShell/AppShell.tsx+AppShell/AppShellMobileContext.tsx: generate and publishmobileNavIdon the mobile context.AppShell/useAppShellMobile.doc.mjs: document the newmobileNavIdreturn field.MobileNavToggleshowcase blocks +SideNav.test.tsxfixtures: supplymobileNavIdwhere the context value is constructed by hand.Test plan
node_modules/.bin/vitest run --root . packages/core/src/MobileNav packages/core/src/AppShell-- 55 pass (4 files), including three new tests inMobileNavToggle.test.tsx(all confirmed failing before the fix):exposes aria-expanded="false" when the drawer is closedexposes aria-expanded="true" after opening the drawerreferences the nav drawer via aria-controls that resolves to the dialog-- asserts the toggle'saria-controlsresolves viadocument.getElementByIdto the<dialog>element.node_modules/.bin/vitest run --root . packages/core/src/SideNav packages/core/src/TopNav-- 151 pass (they renderMobileNavin drawer mode).node_modules/.bin/vitest run --root . packages/core-- 3848 pass (full core suite, incl. the docs contract test).node_modules/.bin/tsc --project packages/core/tsconfig.json --noEmit-- clean.node_modules/.bin/eslinton changed files -- clean.Notes
Found during a broader a11y audit of core components; scoped to one fix per PR.
Buttonforwards arbitraryaria-*props to the underlying<button>(the same{...props}spread the existing behavior relies on), soaria-controlsreaches the DOM. StandaloneMobileNav(outside AppShell) still gets a valid unique<dialog>id via its ownuseId().