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

Slider accessibility issues #14262

Closed
valadzhov opened this issue May 23, 2024 · 4 comments · Fixed by #14793, #14794 or #14795
Closed

Slider accessibility issues #14262

valadzhov opened this issue May 23, 2024 · 4 comments · Fixed by #14793, #14794 or #14795
Assignees
Labels
🐛 bug Any issue that describes a bug forms slider 🚶 priority: medium ♿ a11y When the issue or PR is related to accessibility ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.

Comments

@valadzhov
Copy link
Collaborator

valadzhov commented May 23, 2024

Description

The following accessibility issues are present in the component at the moment:

  • it's missing ariavalue-now
  • tab-index is 1 (it should be 0)
  • missing accessible name (it needs to be implemented by aria-labelledby
  • all of the ARIA attributes are not on the focusable element (the thumb)
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Jul 28, 2024
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Jul 29, 2024
@georgianastasov georgianastasov added 🛠️ status: in-development Issues and PRs with active development on them and removed 🆕 status: new labels Sep 17, 2024
@georgianastasov georgianastasov added ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed 🛠️ status: in-development Issues and PRs with active development on them labels Sep 19, 2024
@ChronosSF
Copy link
Member

@valadzhov , @georgianastasov , why should the tabindex be 1?

@hanastasov
Copy link
Contributor

@valadzhov , @georgianastasov , why should the tabindex be 1?

Yep, good question. I've missed to look at this initially. It does not make sense to make the value 1. Actually, it is not recommended to set any positive integer value (https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex)

@georgianastasov georgianastasov added 🛠️ status: in-development Issues and PRs with active development on them and removed ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. labels Sep 27, 2024
@valadzhov
Copy link
Collaborator Author

Apologies for the delayed reply, Outlook placed the email in the wrong folder.

@valadzhov , @georgianastasov , why should the tabindex be 1?

Yep, good question. I've missed to look at this initially. It does not make sense to make the value 1. Actually, it is not recommended to set any positive integer value (https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex)

Corrected, should be 0

For the existing igx-thumb-label - that one is equivalent to the aria-valuenow and not needed to be added again as label. In the cases where the labels are overridden (weekdays sample) it's equivalent to the aria-valuetext attribute, so that's not directly usable as a label for the entire slider. That needs to come from elsewhere.

Also see
https://www.w3.org/WAI/ARIA/apg/patterns/slider/
The WebComponents' Slider already implements all of these https://github.com/IgniteUI/igniteui-webcomponents/wiki/Slider-Specification#accessibility
And implementation: https://igniteui.github.io/igniteui-webcomponents/?path=/story/slider--default

@georgianastasov georgianastasov added ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed 🛠️ status: in-development Issues and PRs with active development on them labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment