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

New Feature: Accessibility #6

Closed
SLMNBJ opened this issue Apr 6, 2019 · 23 comments
Closed

New Feature: Accessibility #6

SLMNBJ opened this issue Apr 6, 2019 · 23 comments
Labels
feature request New feature or request not planned Won't be investigated unless it gets lots of traction

Comments

@SLMNBJ
Copy link

SLMNBJ commented Apr 6, 2019

Would be nice to have the slider accessibile, maybe with custom labels (i18n).

@davidjerleke davidjerleke added the feature request New feature or request label Apr 6, 2019
@davidjerleke
Copy link
Owner

davidjerleke commented Apr 10, 2019

Hi @SLMNBJ ,
Thank you for reporting this issue!

What do you mean by accessibility? Like aria-hidden="" labels?
Please elaborate.

Thanks in advance!

@SLMNBJ
Copy link
Author

SLMNBJ commented Apr 12, 2019

Hi @davidjerleke,
Yes, I'm talking about the aria-labels. For example, when i navigate using the VoiceOver, i would like to header next slide, previous slide (maybe with custom labels for multiple languages). Same for the dots navigation.

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 30, 2019

Hi @SLMNBJ,
Thanks for the clarification!

I think the accessibility implementation should be up to every user. Embla Carousel’s purpose is to provide tools for building great carousels rather than having a bunch of features out of the box, so I'm not going to add this into the Embla core unless it does get a lot of traction. I hope this makes sense 🙏?

With that said, would you like me to create a CodeSandbox example of how to achieve this?

Cheers 👍!

@davidjerleke davidjerleke added awaiting response Issue is awaiting feedback and removed question labels Jun 7, 2019
@davidjerleke
Copy link
Owner

I’m closing this due to lack of response. Thank you for reporting this 👍🏻!

@davidjerleke davidjerleke removed the awaiting response Issue is awaiting feedback label Jun 18, 2019
@davidjerleke davidjerleke added the not planned Won't be investigated unless it gets lots of traction label Aug 15, 2019
@hotgeart
Copy link

The features needed for a carrousel a11y are described here : https://www.w3.org/TR/wai-aria-practices-1.1/examples/carousel/carousel-1.html

@mantagen
Copy link

I'd be surprised if adding a11y attributes added much too much to the bundle (though 1. I could be wrong, and 2. it's subjective).
a11y's perceived importance is growing, and it could be even be an opt-out/opt-in import which with tree shaking wouldn't affect bundle size if not used.
If it gains traction enough for it to go ahead I'd be very happy to contribute.

@davidjerleke
Copy link
Owner

Hi @mantagen,

Thank you for sharing your thoughts. It depends on how much accessibility we want to implement into the core. If it's only the aria-hidden labels, it may not add much to the bundle size.

Recently, I've considered adding accessibility stuff to the library core. I'm thinking something along these lines:

...and more. But if I'm to proceed with that suggestion, I'm considering to remove the following options:

  • selectedClass
  • draggableClass
  • draggingClass

...in order to keep the library lightweight. I've raised this before releasing major version 3 here, and at that time, Embla users voted to keep these. But only 10 people voted so it's not like people seem to care too much about it. Also, it seems like React users don't utilize these options much.

What's your thoughts about my suggestion?

Best,
David

@mantagen
Copy link

This sounds good to me. I would remove those classes, however I'm pretty much exclusively using css-in-js at the moment, whereas if I was using classes I may be of a different opinion! To me, some small accessibility additions seem more like core functionality than classes.

Following from what @hotgeart has said, the basic aria attributes to add would be

  • aria-roledescription="carousel" on the "viewport" element (the root element to which emblaRef is passed)
  • aria-roledescription="slide" on each slide element
  • aria-label="{i} of {n}" on each slide element (where aria label hasn't been provided by the user?)
  • aria-live="polite" on the element containing the slides
    • however, if the user has put the carousel on a timer, so that it slides automatically, then this should be aria-live="off", otherwise the user is constantly alerted to slide changes
    • this makes me wonder if it should be up to the user to set this attribute
  • aria-hidden="true" on slides which are not visible
    • for most cases this will apply to all but the current slide, however, in the cases in which more than one slide is visible, this logic is no longer correct
    • perhaps in this case the user could override the aria-hidden attribute?

I've seen that swiper are working on improving accessibility too, nolimits4web/swiper#3149, though obviously it is not a lightweight solution -- eg. includes navigation etc etc -- so not all of their criteria apply here, it might be worth looking at their discussion.

I'm wondering if aria-live and aria-hidden might be beyond the scope of this package, since without proper use by the person implementing the slider, they could actually make accessibility worse.

@hotgeart @SLMNBJ what do you think?

@davidjerleke
Copy link
Owner

davidjerleke commented Nov 29, 2020

Thank you for your input @mantagen.

If you don't mind me asking, where did you get that nice list of aria attributes? Also, thanks for the link to the Swiper accessibility issue, that will probably help a lot.

I'm wondering if aria-live and aria-hidden might be beyond the scope of this package, since without proper use by the person implementing the slider, they could actually make accessibility worse.

I think aria-live is beyond scope because autoplay is actually not an Embla Carousel core feature. This is because the autoplay setup on the examples page is a demonstration of how to extend Embla with that feature.

But Embla tracks slides in and out of view so I'm thinking that maybe the aria-hidden attributes should be a core feature. Do you have any additional information about how to use this incorrectly and make accessibility worse, so we can avoid doing it wrong?

Best,
David

@mantagen
Copy link

Hi David,

That list is essentially from https://www.w3.org/TR/wai-aria-practices-1.1/examples/carousel/carousel-1.html

There are two potential problems with adding aria-hidden that I can think of:

  1. For a carousel in which the width of the slide is eg. 20% (i.e. multiple slides fit within the carousel viewport), all the visible slides should have aria-hidden="false". It would need a programmatic way of determining whether the slide is visible, for instance: gatsby-image handles it like:
  1. If slides contain any focusable elements like a or button, and the slide has aria-hidden="true" then the browser can focus on these elements, but the focus will not be announced by a screen reader. See:

I'd say that perhaps (1) is not such a big deal, but that (2) is. If the library added a tabIndex: -1 to all focusable elements of the inactive slides, and there was a something in the documentation saying that the carousel was intended only to have one slide per view, then maybe this would get around both these issues, but this is presumably beyond core scope.

This is why I'm currently of the opinion that adding aria-hidden to the core of this library could not be a good idea.

@mantagen
Copy link

For further reference: https://act-rules.github.io/rules/6cfa84

Also, I've just seen that the carousel slides on tab press if new focus is within the following slide. I'm not sure what the implications of this are... as in, I'm not sure if it is okay to dynamically change from aria-hidden="true" to aria-hidden="false" as an element receives focus.

@davidjerleke
Copy link
Owner

Thank you for the further information @mantagen.

That's helpful 👍. I'm thinking that we should create a CodeSandbox with all the desired accessibility features. For example, we can use the default React sandbox as a starting point. I'm hoping that by the time we're done with the sandbox, we should have a clear picture of what should go into the Embla core and not.

Also, I've just seen that the carousel slides on tab press if new focus is within the following slide. I'm not sure what the implications of this are... as in, I'm not sure if it is okay to dynamically change from aria-hidden="true" to aria-hidden="false" as an element receives focus.

When I implemented this, I thought it would be a good idea to mimic the browser behavior to some extent, by brining the focusable element into view. But I'm open to other suggestions.

Best,
David

@mantagen
Copy link

mantagen commented Jan 6, 2021

Yes I believe that content of non-visible slides should not be tabable:

The user cannot tab to any of the interactive elements in hidden panels. The tabindex="-1" attribute should be added dynamically to these elements. The attribute would be removed when the associated panel is visible.
https://www.accede-web.com/en/guidelines/rich-interface-components/carousels/

This article also suggests to add aria-hidden="true" to non-visible slides.

Yes, let's start from the sandbox, and see where we get.

@Wikgabja
Copy link

Wikgabja commented Nov 4, 2021

Hello,
I would like to change the aria-hidden value from false to true for selected slide, I mean the slide with "is-selected" class. Could you let me an example how to do that in vanilla script?

@davidjerleke
Copy link
Owner

davidjerleke commented Nov 4, 2021

Hi @Wikgabja,

You can use the same approach as the selected class toggling functions here. And don’t forget to attach this function to the select and pointerUp event like this.
Obviously, you have to replace the addClass and removeClass functions with a function that adds/removes aria-hidden attributes.

I hope that helps.

I’ll soon release v6 which I think can be somewhat of a game changer for all Embla users that are frustrated about missing features (like accessibility). v6 comes with a plugin system that will enable Embla users to use plugins to extend it beyond its core functionality. And anyone will be able to write and share their own plugins too:

More information coming as soon as v6 has been released. You’ll find it on the documentation website then.

Best,
David

@patricknelson
Copy link

Just curious since I'm reviewing this carousel for use on our site: Are there any good a11y plugins available now? I noticed the plugin route was suggested but none were mentioned here or on the site, which is why I figured I'd ask.

Keyboard a11y isn't too bad (as long as you're sighted) but screen readers just identify everything right now as "button" so it's not helpful yet. Not sure if you have to dig into the guts in order to announce which bullet/dot you're on, for example.

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 8, 2023

Hi @patricknelson,

No plugin yet. But the secret project will include accessibility:

I can’t give an ETA on when it’s ready though because it’s under development. If you have to build something soon and want to use this library you have to do it yourself.

Best,
David

@patricknelson
Copy link

Ok. Thanks for the feedback.

Just curious: What’s the point to having a “secret” project in open source? Why not just describe what it is and be frank that it’s just incomplete or the concept is under development?

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 8, 2023

@patricknelson because in this case I don't want feedback and any interference before I have a working proof of concept or an experimental feature ready.

@davidjerleke
Copy link
Owner

Calling all accessibility experts 👋! Could anyone help me with creating a complete list of things that are required to create a fully accessible carousel? It's for the accessibility option that will be available in the carousel generator.

@hotgeart
Copy link

hotgeart commented Jul 8, 2023

@davidjerleke everything you need is in the W3C example:
https://www.w3.org/WAI/ARIA/apg/patterns/carousel/examples/carousel-1-prev-next/

@patricknelson
Copy link

@davidjerleke Makes sense. The updated description helps a bit too. Looks like an interesting project.

@nwidynski
Copy link
Contributor

nwidynski commented Oct 31, 2023

Hey guys,

thought this is a great use-case for a plugin, so here is my attempt: https://github.com/nwidynski/embla-carousel-aria

  • ARIA-spec accessibility adhering to W3 ARIA carousel
    • "role"
    • "tabindex"
    • "aria-live"
    • "aria-orientation"
    • "aria-roledescription"
    • "aria-multiselectable"
    • "aria-selected"
    • "aria-hidden"
  • Maintenance of tabindex order
  • Localization of all ARIA properties
  • Adobe-inspired automatic focus shift
  • Full screen-reader support
  • Unit tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request not planned Won't be investigated unless it gets lots of traction
Projects
None yet
Development

No branches or pull requests

8 participants
@hotgeart @patricknelson @davidjerleke @mantagen @nwidynski @SLMNBJ @Wikgabja and others