Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve breadcrumbs accessibility #3990

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Improve breadcrumbs accessibility #3990

merged 1 commit into from
Apr 20, 2023

Conversation

raimund-schluessler
Copy link
Contributor

This improves the accessibility of NcBreadcrumbs:

  • the bread crumbs are now wrapped in a nav
  • the breadcrumbs are now a list, each crumb is a li
  • the Action menu holding the hidden crumbs got an aria-hidden atrribute

Closes #3951.

@raimund-schluessler
Improve breadcrumbs accessibility
acece3d 
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: breadcrumbs Related to the breadcrumbs components labels Apr 19, 2023
@raimund-schluessler raimund-schluessler added this to the 7.11.0 milestone Apr 19, 2023
@raimund-schluessler raimund-schluessler mentioned this pull request Apr 19, 2023
Closed
5 tasks
skjnldsv
skjnldsv requested changes Apr 20, 2023
View reviewed changes
Copy link
Contributor

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

It's missing one important step:

  • The hidden breadcrumbs should still be rendered and hidden visually, so the nav does not change for screen readers

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Apr 20, 2023 •
edited by skjnldsv

It's missing one important step:

  • The hidden breadcrumbs should still be rendered and hidden visually, so the nav does not change for screen readers

No, that was already the case before, so I didn't mention it as a change. See the markup with two crumbs hidden:

grafik

The crumbs are still there and get the vue-crumb--hidden class with display: none;. Or is display: none; not ok to hide them and we need to hide them differently?

@skjnldsv
Copy link
Contributor

No, that was already the case before, so I didn't mention it as a change. See the markup with two crumbs hidden:

Oooh, I did not know that! 👍

@skjnldsv skjnldsv merged commit 9ce5dcb into master Apr 20, 2023
15 checks passed
@skjnldsv skjnldsv deleted the fix/3951/breadcrumbs-a11y branch April 20, 2023 07:25
@raimund-schluessler raimund-schluessler mentioned this pull request May 2, 2023
Merged
@artonge artonge mentioned this pull request May 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@JuliaKirschenheuter JuliaKirschenheuter Awaiting requested review from JuliaKirschenheuter

@artonge artonge Awaiting requested review from artonge

@Pytal Pytal Awaiting requested review from Pytal

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

Assignees
No one assigned
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: breadcrumbs Related to the breadcrumbs components
Projects
None yet
Milestone
7.11.0
Development

Successfully merging this pull request may close these issues.

Create right html structure for NcBreadcrumbs.vue and NcBreadcrumb.vue
3 participants
@raimund-schluessler @skjnldsv @AndyScherzinger