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.

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

Fix / Accessibility improvements for Hero shelf pagination and LayoutGrid #646

Conversation

langemike
Copy link
Collaborator

@langemike langemike commented Nov 18, 2024

These issues were discovered by a https://pagespeed.web.dev/ analysis. These changes could most likely give us a 100% accessibility score.

Changes:

  • aria-colindex / aria-rowindex must count from 1
  • Hero pagination items were not wrapped in a <li> (causing invalid HTML markup)
  • Added a disabled attribute to the button when the pagination dot is non-clickable (ascendant element has aria-hidden="true" attribute)

The latter caused an error message in Chrome devtools. tabIndex={-1} did not solve this on the <li> or <button>, so i resorted to a disabled which seems appropriate considering the situation.

Tickets: OTT-2784 & OTT-2776

Copy link
Collaborator

@royschut royschut left a comment

Choose a reason for hiding this comment

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

That should work!

Should we fix coming merge conflicts ahead of time? The code has changed from my PR. Maybe you can base this PR to that one.

@langemike langemike changed the base branch from develop to feat/hero-shelf-update November 18, 2024 11:51
@royschut royschut deleted the branch jwplayer:feat/hero-shelf-update November 18, 2024 13:35
@royschut royschut closed this Nov 18, 2024
@langemike
Copy link
Collaborator Author

Replacement PR: #647

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants