Add NavList.Heading slot for accessible section-nav headings#8031
Add NavList.Heading slot for accessible section-nav headings#8031janmaarten-a11y wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: 9533be7 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 |
|
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
Introduces an optional NavList.Heading that names the navigation region, labels the <nav> landmark, and coordinates heading levels so group headings nest correctly beneath it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b945940 to
9f623ad
Compare
There was a problem hiding this comment.
Pull request overview
Adds an optional NavList.Heading slot to @primer/react’s NavList to provide a single, consistent navigation-region heading (default h2, optionally h3), automatically label the <nav> landmark via aria-labelledby, and derive group heading levels one level deeper for a correct heading outline.
Changes:
- Add
NavList.Headingslot (withvisuallyHiddensupport) and internal context to derive group heading levels. - Update
NavList.Group/NavList.GroupHeadingdefaults to depend on the presence/level ofNavList.Heading. - Add unit tests, Storybook feature stories, docs metadata, and a changeset for a minor release.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/NavList/NavList.tsx | Implements NavList.Heading, nav landmark labeling, and derived group heading levels via context. |
| packages/react/src/NavList/NavList.test.tsx | Adds unit coverage for heading rendering, landmark labeling, level derivation, and visually-hidden behavior. |
| packages/react/src/NavList/NavList.module.css | Adds styling for the new heading alignment/spacing. |
| packages/react/src/NavList/NavList.features.stories.tsx | Adds “With Heading” and “With Heading (hidden)” stories to demonstrate the new slot. |
| packages/react/src/NavList/NavList.docs.json | Documents the new NavList.Heading subcomponent and updates group heading docs. |
| .changeset/navlist-heading-slot.md | Declares a minor bump for the new additive API. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 2
Merge intrinsic h2 element props into NavListHeadingProps so consumers can pass aria-*, data-*, title, and other standard heading attributes without a type assertion, and document the id prop in NavList.docs.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Integration test results from github/github-ui PR:
All checks passed! |
Overview
Many GitHub pages built with
NavList— user settings, repository settings, and similar — end up with a confusing or broken page heading structure. This PR adds an optionalNavList.Headingslot that gives the whole navigation region a single heading, names the<nav>landmark, and automatically keeps group headings nested correctly beneath it.Why this is needed
Today,
NavList.Groupheadings default toh3(and some are hand-tweaked toh2). On a typical settings page that means a subset of the left-hand section-navigation links is exposed at the same heading level as the main page-content sections. For someone navigating by heading with a screen reader, the navigation links and the page content look like siblings in the document outline, which misrepresents the actual hierarchy and makes the page slower to understand and move through.What usually makes more sense is:
h2heading for theNavListas a whole, withh3subheadings for eachNavList.Group.That produces a correct
h2 → h3outline, lets screen reader users jump straight to the navigation region, and makes the visual/content hierarchy match the semantic one.Right now teams have to wire all of this up by hand — add an accessible name to the
<nav>, pick heading levels for each group, and remember to keep them consistent. It's easy to forget and easy to get wrong. This PR makes the correct structure the easy, built-in path.How it works
NavList.Headingis a new slot. Add it as a child ofNavListand it renders the heading for the region. It defaults toh2.<nav>landmark automatically viaaria-labelledby, so the navigation gets an accessible name without extra work. If you already pass your ownaria-label/aria-labelledby, yours wins — we don't override it.visuallyHiddenvariant for pages where the heading shouldn't be shown visually but should still exist for assistive tech and as the landmark's name.NavList.Headingis present,NavList.Group(andNavList.GroupHeading) default to one level deeper than the heading —h3under anh2,h4under anh3. With noNavList.Heading, groups keep today'sh3default, so nothing changes for existing usage.h2orh3(noh1— that's the page title — and group headings never go deeper thanh4), keeping the outline shallow. If you need a specific level on a single group, passNavList.GroupHeading as={...}as an explicit override.Implementation notes
NavListalready wraps its children inActionList, which has a heading slot. The new work is a NavList-level slot that (a) names the<nav>landmark and (b) coordinates heading levels with groups via a small internal context. It's mostly wiring rather than net-new machinery.visuallyHiddenvariant, the visually-hidden styles are applied directly to the heading element (reusing the existing_VisuallyHiddenCSS) rather than wrapping it in a<span>— a heading isn't valid phrasing content inside a span.ActionList.Heading, which currently has the same span-wrapping pattern; it's intentionally left out of this PR to keep the scope tight.Changelog
New
NavList.Headingslot: names the navigation region, labels the<nav>landmark viaaria-labelledby, supports avisuallyHiddenvariant, and defaults toh2(configurable toh3).NavList.Group/NavList.GroupHeadingnow default their heading level to one below aNavList.Heading(e.g.h3under anh2).Changed
NavList.Headingis present,NavList.Grouptitle headings derive their level from it instead of always renderingh3. With noNavList.Heading, theh3default is unchanged.Removed
Rollout strategy
Purely additive. Existing
NavListusage that doesn't include aNavList.Headingrenders exactly as before.Testing & Reviewing
h2heading,as="h3"override,<nav>labelled by the heading, consumeraria-labelnot being overridden, group headings derivingh3/h4, theh3fallback with no heading,NavList.GroupHeading asoverride, and the visually-hidden heading staying in the a11y tree without a wrapping span.h2 → h3outline and that the<nav>is named by the heading.Merge checklist