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

why use aria-describedby in slides? #2020

Closed
plweil opened this issue Jan 13, 2016 · 28 comments
Closed

why use aria-describedby in slides? #2020

plweil opened this issue Jan 13, 2016 · 28 comments

Comments

@plweil
Copy link

plweil commented Jan 13, 2016

I'm wondering about the appropriateness or need to use aria-describedby in each slide to refer to the navigation dots. The dots don't provide any sort of descriptive information about the slide.

I'm not which which aria attribute would be appropriate to indicate the relationship between the dots and the slide (if this is the goal). Possibly aria-controls? But aria-describedby doesn't seem right.

Moreover, this attribute is still generated even when the dots are not present , which is the default for Slick. So, unless I'm missing something, it shouldn't even appear by default.

@ccbayer
Copy link

ccbayer commented Jan 14, 2016

@plweil i agree; i had to fake this to pass validation by visually hiding the dots even though i really wanted them gone altogether. if the dots don't exist, then the describedby ID doesn't exist, which is invalid.

additionally, i noticed this workaround didn't fix validation on a center-oriented slick slider.
i'm guessing this is because the code clones the individual slides and possibly iterates their aria-describedby ID - which it shouldn't, since they are clones.

@jayburling
Copy link

I am also dealing with the validation errors caused by this.

We changed the role to "region" and used an 'aria-labelledby' to describe an added element for the slide.

@wemaycode
Copy link

I came up against this issue as well. In my slideshow each slide has a div corresponding to the title of the slide. I set that div to the "aria-describedby" attribute generated by slick to get around the ADA compliance.

$('.slide-title').each(function () {
    var $slide = $(this).parent();    
    if ($slide.attr('aria-describedby') != undefined) { // ignore extra/cloned slides
        $(this).attr('id', $slide.attr('aria-describedby'));
    }
});

@ccbayer
Copy link

ccbayer commented Feb 22, 2016

@wemaycode that's a good alternative / fallback!

@lizlantz
Copy link

I'm also having this issue, would be nice to have a setting for this or remove aria-describedby when dots are set to false.

@benvoynick
Copy link

Also noticed this going through the markup generated. aria-describedby really shouldn't be set if the element it points to doesn't exist.

@plweil
Copy link
Author

plweil commented Apr 29, 2016

And even if the dots exist, why use aria-describedby? The dots don't provide a description. If there is something we want to communicate to the user regarding the relationship between the slide and the dot, let's use the aria attribute that accomplishes what we want.

@plweil plweil closed this as completed Apr 29, 2016
@leggomuhgreggo
Copy link
Collaborator

Good question! I'm not an expert, and I don't have strong feelings, but my guess is that the describedby is less in reference to the dot representation, and more in reference to the markup, in which there's a number corresponding to the slide it navs to. The number is hidden with the theme styles and replaced with that dot. Moreover for implementations that use custom paging, there's often even richer relationship information conveyed.

@ibarsi
Copy link

ibarsi commented May 6, 2016

Just wondering why this issue was closed?

I agree with @surfbird0713 and @benvoynick, if dots are set to false then aria-describedby should either be pointing to a different, perhaps hidden, descriptive element, or just not included at all.

It would be nice to have this issue resolved at some point so we won't have to continue resorting to workarounds.

@martinbethann
Copy link

Also wondering why this was closed? Accessibility is important, especially since we have a legal obligation to make these sites work for all. Slick slider does not pass validation if no dots are used.

@kenwheeler
Copy link
Owner

I'm no a11y expert, so I'm certainly open to suggestion, and more importantly PRs. hit me.

@martinbethann
Copy link

martinbethann commented May 18, 2016

I think @ibarsi, @surfbird0713, @benvoynick had great suggestions of creating an element only visible to screen-readers to describe the slides. Or just remove the broken aria-reference?

@ibarsi
Copy link

ibarsi commented May 18, 2016

I don't mind forking and putting together a PR with a potential solution (likely hidden fields) once I'm back from vacation next week, unless someone beats me to it!

@webegguk
Copy link

webegguk commented Jun 1, 2016

I can't help thinking that in this case, the aria-describedby attribute isn't the way forward. I've been tasked with making a site that uses lots of carousels (slick is the best in every other way) fully accessible and this is really holding me up. Some of the carousels have dots, some don't as there are specific settings for specific uses and sizes of carousel.
From what I can see, the only thing this attribute refers to are id's on dots and even when you do have dots enabled, there'll be plenty of these attributes with no id attached to them as they'll be intermittently referring to every third/fourth/fifth etc slide within the range.
@wemaycode's solution is a great idea and helps though can cause multiple id's unfortunately. I'll keep investigating this issue and hope to find a robust solution.

@ibarsi
Copy link

ibarsi commented Jun 1, 2016

@webegguk If you check the pull request I've created, I've modified the aria-describedby generation logic for the case in which there are more slides than dots. I've ensured that each slide is referencing the dot that makes it visible. However, I'm sure there are many slick configurations that I haven't tested, as I am pretty new to this repo.

Honestly, I really do believe that these dots are not providing any real added description to the slides... To avoid the need for workarounds like what @wemaycode has done, maybe we should remove the generation of aria-describedby tags all together and leave it up to the user to add meaningful tags themselves? That will likely bring more value to those that are trying to make slick accessible.

@webegguk
Copy link

webegguk commented Jun 1, 2016

I tend to agree. Removal of the aria-describedby attributes entirely, or optionally including them seems like a good solution as the only thing they refer to as you say, is the dots, which don't seem to provide any descriptive content hence negating the need to add accessibility.

@michaelspellacy
Copy link

+1 This ever going to be addressed?

@dgpokl
Copy link

dgpokl commented Feb 28, 2017

@plweil posted a comment above that appears to be in support of making the change that would fix this issue, and also was the creator of the issue, but he closed the issue himself on the same day, for some reason. I do not understand why. My bad! Maybe he closed it because of #2346 solving the immediate concern?

@michaelspellacy @webegguk @ibarsi - it seems like most people here agree that the aria-describedby should be removed entirely though. May as well open a new issue to suggest that and just reference this discussion.

@plweil
Copy link
Author

plweil commented Mar 1, 2017

Actually I don't remember closing it, and have no idea why I might have closed it.

@danielkorte
Copy link

+1 to removing aria-describedby

@afishback
Copy link

+1 to removing aria-describedby

2 similar comments
@dgrant85
Copy link

+1 to removing aria-describedby

@Bramanga
Copy link

+1 to removing aria-describedby

@okadots
Copy link

okadots commented Aug 31, 2017

+1 removing aria-describedby

@mpge
Copy link

mpge commented Sep 19, 2017

+1 removing aria-describedby

@C13L0
Copy link

C13L0 commented Nov 20, 2017

+1 removing aria-describedby

@CreateSean
Copy link

I commented out llines 1327-1330 of slick.js and that removed the aria-describedby

@jaclyntan
Copy link

I had to comment out aria-describedby to pass a lighthouse audit :/

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

No branches or pull requests