Skip to content

docs(pagination): put current page on link element for a11y #41154

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

Merged
merged 3 commits into from
Apr 29, 2025

Conversation

MarkoOleksiyenko
Copy link
Contributor

Description

Update the Pagination examples to have the aria-current="page" on the link elements.
Modify the part where the active pagination element can be replaced with span.

Motivation & Context

Focusable pagination element should be marked with aria-current for the SR to read it properly (NVDA works with the attribute set on li but JAWS needs it to be on element).

Active pagination element should not be a span as assistive technology won't read it as active page.

reference of the aria-current and recommended usage:

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • [] I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

@mdo
Copy link
Member

mdo commented Apr 11, 2025

@patrickhlauke Mind giving your guidance here?

@patrickhlauke
Copy link
Member

only partially agree here. yes, aria-current should be on the links, not their parent list items.

however, it's the author's choice whether or not they want to have the current page's pagination as an active link or not. it's perfectly valid to decide not to make the current page's indicator a link at all. at that point, you can also omit the aria-current altogether.

@MarkoOleksiyenko
Copy link
Contributor Author

Hello @patrickhlauke and @mdo, thank you for the replies :) Indeed it's the author's choice to put the pagination elements as link or not. The idea here is to provide an accessible example in Bootstrap that is aligned with major SRs (JAWS, NVDA) so that by default their code is a11y compliant.

@mdo mdo added this to v5.4.0 Apr 25, 2025
@github-project-automation github-project-automation bot moved this to Needs review in v5.4.0 Apr 25, 2025
@mdo
Copy link
Member

mdo commented Apr 25, 2025

Slating for v5.4.0 for me to revisit. This will need a rebase to bring it up to speed with the move to `.mdx.

@MarkoOleksiyenko MarkoOleksiyenko force-pushed the a11y-pagination-current-page branch from dcbdb96 to 47896df Compare April 28, 2025 13:30
@MarkoOleksiyenko MarkoOleksiyenko force-pushed the a11y-pagination-current-page branch from 47896df to ca64e71 Compare April 28, 2025 13:34
@MarkoOleksiyenko
Copy link
Contributor Author

Hello, @mdo , PR rebased and aligned with .mdx move :)

@mdo mdo added this to v5.3.6 Apr 29, 2025
@mdo mdo removed this from v5.4.0 Apr 29, 2025
@github-project-automation github-project-automation bot moved this to To do in v5.3.6 Apr 29, 2025
@mdo
Copy link
Member

mdo commented Apr 29, 2025

Wonderful! Pushed some copy edits to make the examples and optional span/a swap a little clearer. Re-slating for v5.3.6.

@mdo mdo merged commit ba5b5a6 into twbs:main Apr 29, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from To do to Done in v5.3.6 Apr 29, 2025
@patrickhlauke
Copy link
Member

Indeed it's the author's choice to put the pagination elements as link or not. The idea here is to provide an accessible example in Bootstrap that is aligned with major SRs (JAWS, NVDA) so that by default their code is a11y compliant.

coming back in late to say: a page that chooses not to make the marker for the current page an actionable element is not somehow "non-compliant"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants