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

Aria describedby Tags - Hide if dots are disabled, evenly distribute when more slides than dots #2346

Merged
merged 2 commits into from
Jun 22, 2016

Conversation

ibarsi
Copy link

@ibarsi ibarsi commented May 31, 2016

Description

I have identified 2 scenarios in which I believe there are issues in the way "aria-describedby" tags are generated and applied to slides.

  1. When the "dots" setting is turned off, "aria-describedby" tags are still added to slides, but are referencing the "would be" ids that the dots DOM elements would have if they were enabled. This breaks WAVE and A Checker tests, as those tags must reference the ids of elements that exist in the DOM.
  2. When there are more slides than there are dots, "aria-describedby" tags are still generated and applied to slides as if there were dots available for each slide (ex. if there are 2 dots but 6 slides, each slide has an "aria-describedby" tag with a unique id, 4 of which do not actually exist as dots on the page). This also breaks WAVE and A Checker tests for the same reasons.

Issue: #2020

Resolution

To resolve these issues, I've applied the following changes:

  1. When "dots" is set to false, I do not add "aria-describedby" tags to the slides, as there are no longer any elements that could reasonably describe the slides (see the note below for my opinion on this).
  2. When there are more slides than there are dots, I set every visible slide to the currently selected dot. This should ensure that all slides have an "aria-describedby" tag that points to an existing DOM element.

Fiddle: https://jsfiddle.net/ibarsi/4khqcw9t/5/

NOTE

I just wanted to mention that we may want to consider removing the generation of "aria-describedby" tags entirely... After these changes, accessibility checkers won't give the DOM generated by slick a failing grade, however I don't believe the dots provide a useful description of their referenced slides. Elements referenced as describers should really contain some sort of text that provides screen readers additional details that may not be apparent. This should probably be something that users of slick tag and create themselves, if they choose to.

Spec Reference: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-describedby_attribute

@dgpokl
Copy link

dgpokl commented Jun 1, 2016

My vote: 👍 Strongly agree with the "note" above as well. Thank you @ibarsi - I'm going to use your fork for now; hoping to see it merged in!

@leggomuhgreggo
Copy link
Collaborator

Looks good to me. Thanks @ibarsi. Appreciate the assessment in your note.

Test for good measure

@leggomuhgreggo leggomuhgreggo merged commit 6db519a into kenwheeler:master Jun 22, 2016
@ibarsi
Copy link
Author

ibarsi commented Jun 22, 2016

No problem, glad my contribution helped!

@ibarsi ibarsi deleted the aria-describedby-tags branch June 22, 2016 01:46
@sassycoder
Copy link

Hey, many thanks for this. I had to add another condition in case the dots aren't rendered on initial page load e.g. dots not needed at desktop, but will be needed in mobile:

if (_.options.dots === true && _.slideCount > _.options.slidesToShow) {

@signal-intrusion
Copy link

How close are we to having this included in a release?

@tyleryoungblood
Copy link

How close are we to having this included in a release?

Looks like 2346 was included in the 1.71 release on Jul 30, 2017.

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.

6 participants