Skip to content

add role navigation and semantic list to left panel nav #1047

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Appryll
Copy link
Contributor

@Appryll Appryll commented Jun 3, 2025

Enhance accessibility: add role="navigation" and semantic list to left panel


Issue associée : 879

Copy link
Collaborator

@Ovgodd Ovgodd left a comment

Choose a reason for hiding this comment

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

Hello,
Thanks for these great updates!

There are just a few small improvements to make it even better 😊

>
{defaultQueries.map((query) => {
const isActive = target === query.targetQuery;
<nav role="navigation" aria-label="Docs filters">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello Thanks for the semantic update !
You can remove role="navigation", as it’s implicit on the nav element. It’s only necessary if you're using a non-semantic container like a div.

and Also, consider updating the aria-label something like "Document sections" would be more meaningful for screen reader users than "Docs filters".

Suggested change
<nav role="navigation" aria-label="Docs filters">
<nav aria-label="Document sections">

font-weight: ${isActive ? 700 : undefined};
&:hover {
background-color: ${colorsTokens['greyscale-100']};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There’s no visible focus outline, so it's hard to know where we are.

👉 You may need to explicitly add a :focus-visible style on the BoxButton to restore proper keyboard visibility.

Suggested change
}
}
&:focus-visible {
outline: 2px solid ${colorsTokens['primary-500']};
outline-offset: 2px;
border-radius: ${spacingsTokens['3xs']};
}
focus

>
{defaultQueries.map((query) => {
const isActive = target === query.targetQuery;
<nav role="navigation" aria-label="Docs filters">
Copy link
Collaborator

Choose a reason for hiding this comment

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

The visual layout seems slightly broken after this change probably some missing spacing or margin on the filter items.

sidebar

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.

3 participants