Skip to content

Conversation

@SriHV
Copy link
Contributor

@SriHV SriHV commented Sep 26, 2025

What is the context of this PR?

ONSDESYS-472

As per the ticket:

  • Removed "Go to" from "aria-label" attributes.

  • When "aria-current" is set to true a default "aria-label" will only have the text set in position variable which is "Page " ~ currentPageIndex ~ " of " ~ totalPages(Eg When aria-current is over the 3rd page, voice over only reads Page 3 of 4 instead of previous Current Page(Page 3 of 4))

  • The ticket also mentions about adding "aria-hidden" to svg icons. We have this already so we don't need to make any changes to it

Also user who raised this issue used total validator to check the issue. As total validator works with a paid tier. I've used wave to test the page.

How to review this PR

  • Use Voice over and check that everything mentioned above is reflected and is as per the requirement
  • Use wave extension to check that there are no issues

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

@SriHV SriHV requested a review from a team as a code owner September 26, 2025 08:31
@netlify
Copy link

netlify bot commented Sep 26, 2025

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit fbf588b
🔍 Latest deploy log https://app.netlify.com/projects/ons-design-system-preview/deploys/68e3b8fb4daab200088577a4
😎 Deploy Preview https://deploy-preview-3737--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@SriHV SriHV self-assigned this Sep 26, 2025
@SriHV SriHV added the Accessibility Issues discovered through accessibility testing label Sep 26, 2025
@rmccar
Copy link
Contributor

rmccar commented Oct 2, 2025

It looks like we do have some svgs in the icon macro that return an svg that doesn't have aria-hidden and doesn't have aria-labeledby so you might need to take a look into this. One example is the "crest" svg.

@SriHV
Copy link
Contributor Author

SriHV commented Oct 2, 2025

It looks like we do have some svgs in the icon macro that return an svg that doesn't have aria-hidden and doesn't have aria-labeledby so you might need to take a look into this. One example is the "crest" svg.

I have aria-hidden=true for all the icons which doesn't have aria labelled by. Let me if that's okay

Copy link

@sveltifier sveltifier left a comment

Choose a reason for hiding this comment

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

Looks great!

@SriHV SriHV merged commit f0e8360 into main Oct 8, 2025
14 checks passed
@SriHV SriHV deleted the fix/pagination-accessibility-issue branch October 8, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accessibility Issues discovered through accessibility testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants