Skip to content

feat(a11y): allow custom aria label on side navigation [AC-4119]#1340

Merged
Stefan3002 merged 1 commit intocanonical:mainfrom
Stefan3002:a11y-bug-multiple-navs
Apr 6, 2026
Merged

feat(a11y): allow custom aria label on side navigation [AC-4119]#1340
Stefan3002 merged 1 commit intocanonical:mainfrom
Stefan3002:a11y-bug-multiple-navs

Conversation

@Stefan3002
Copy link
Copy Markdown
Contributor

@Stefan3002 Stefan3002 commented Apr 2, 2026

Done

  • Allowed a custom aria-label value to be passed on the side nav element
  • Users would want to change the aria-label of the element, not the enclosing div element. This encapsulation prevents users from changing the aria-label of the nav element itself
  • Why is this important? Because if we use 2 elements with the role of navigation e.g: SideNavigation and TablePagination, we would get a a11y vioaltion: unique-landmark. This is becuase we have two aria-semantic navigations now.
  • The ideea is that we would like to allow the users to give their SideNav element a custom aria-label: "Main / Primary Navigation" so that it is different from other divs with role="navigation"
  • The problem here is the encapsulation: I would normally be able to set aria=label on the encapsualting div, but I need it to be set on the element to avoid a11y violations.

QA

Pinging @canonical/react-library-maintainers for a review.

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

Create a dummy page that uses the SideNav and use it like this (for example):

<SideNavigation
            ariaLabel={"test"}
            dark={dark}
            hasIcons
            items={items}
            navClassName="p-side-navigation"
          />

Percy steps

  • List any expected visual change in Percy, or write something like "No visual changes expected" if none is expected.

Fixes

Fixes: #AC-4119.

image

@webteam-app
Copy link
Copy Markdown

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for customizing the accessible name of the SideNavigation landmark by allowing a consumer-provided aria label to be applied directly to the inner <nav> element, helping avoid unique-landmark accessibility violations when multiple navigation landmarks exist on a page.

Changes:

  • Introduced a new ariaLabel prop on SideNavigation.
  • Applied the provided label as aria-label on the internal <nav> element.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

*/
ariaLabel?: string;
},
HTMLProps<HTMLDivElement>
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are now two different ways a consumer can set a label: passing the standard aria-label attribute (via ...props) will apply it to the outer <div>, while the new ariaLabel prop applies to the inner <nav>. This is easy to get wrong and can still leave the navigation landmark unnamed. Consider supporting the standard aria-label API by extracting props["aria-label"] and applying it to <nav> (and omitting it from the <div> props), or alternatively introduce a navProps object to pass attributes directly to the <nav> element.

Suggested change
HTMLProps<HTMLDivElement>
Omit<HTMLProps<HTMLDivElement>, "aria-label">

Copilot uses AI. Check for mistakes.
Comment on lines 167 to 171
{...props}
>
<nav className={navClassName}>
<nav aria-label={ariaLabel} className={navClassName}>
{children ?? generateItems(items, listClassName, linkComponent, dark)}
</nav>
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior (ariaLabel controlling the <nav> landmark name) isn’t covered by tests. Since this component already has unit tests (src/components/SideNavigation/SideNavigation.test.tsx), please add a test that renders SideNavigation with ariaLabel set and asserts the <nav> has the expected aria-label attribute.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Stefan3002 Stefan3002 force-pushed the a11y-bug-multiple-navs branch from ac94a64 to cd0ebf0 Compare April 6, 2026 08:22
@Stefan3002 Stefan3002 merged commit 5d099a1 into canonical:main Apr 6, 2026
8 checks passed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 6, 2026

🎉 This PR is included in version 4.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants