-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Improve Accessibility Features #3149
Comments
Next and previous should be buttons as they don't take the user to a new page. They also don't need I'm using SiteImprove (an automated a11y review tool) and it's actually penalising me at the moment for the
I disagree with the above slightly because of this IE11 bug but I think for all modern browsers the role isn't required. |
The navigation next and previous buttons have click and keypress events. This causes navigation with a keyboard (pressing enter) to press the button twice. |
Having |
Actually having tabindex="-1" on all focusable elements in invisible slides would be helpful. Currently I use custom code to manage the tabindex myself. I had assumed that a carousel that claims to be accessible would deal with plain links in slides. For those who are wondering why this is an issue: if you tab through the slides you'll reach at one point a hidden slide. Then they will scroll into view. This breaks the layout and the state managment of Swiper, ie the wrong slides are "active". |
BTW aria-roledescription="carousel" is a plain text so it must be translatable if implemented. But aria-roledescription is not welcomed by all a11y advocates. |
Speaking of tabindex="-1", it would be nice if disabled navigation buttons would not be focusable. |
Sadly SwiperJS is yet another library that has a well-meant a11y-code, but no real interest or knowledge to make it actually work without barriers. |
Hi masi, would you mind sharing your solution? I also have links in slides and when I tab trough them it breaks the layout. I'm trying to use their API to slideTo the current focused slide but it's not working. |
Update: I was able to fix this by changing the scrollLeft property of the swiper container to 0 |
Came to the same solution with tabindex="-1" for invisible slides, which logically make sense as well, since if element is not visible it should not be focusable. Also, in IE 11 aria-* attributes are missed on navigation buttons. |
Here's what I use to add First I use a polyfill for the
Then I have this function: const makeAllButCurrentSlideInert = function makeAllButCurrentSlideInert() {
const currentSlideEl = this.slides[this.realIndex];
this.slides.each((index, slide) => {
if (slide !== currentSlideEl) {
slide.setAttribute("inert", "");
} else {
slide.removeAttribute("inert");
}
});
}; And I call it every time the carousel updates or is initiated: const swiperOptions = {
// …
on: {
init() {
makeAllButCurrentSlideInert.call(this);
},
slideChange() {
makeAllButCurrentSlideInert.call(this);
},
slideChangeTransitionEnd() {
const currentSlideEl = this.slides[this.realIndex];
currentSlideEl.setAttribute("tabindex", "-1");
currentSlideEl.focus();
},
},
}; The |
I also used transitionEnd event, but also listened for keyboard navigation ('keydown'), since I was able to reproduce that issue when using keyboard navigation (arrows) and then moving focus quickly with tab/shift+tab |
According to the start post by @fregiese in this issue, the implemented list of new features is part of release 6.3.0.
|
#3834 is the relevant PR for anyone who would like to take a look at the changes |
@philwolstenholme @fregiese are there things left to do? or the issue can be closed? |
I think there are a few extra tasks:
|
I updated the inital comment to show the current status |
I just noticed that Swiper is adding |
Be aware of akiran/react-slick#1535 |
Just a quick question which also relates to accessibility, why dont the properties If youre using a screen reader and press enter on the navigation buttons, it replaces the text in the swiper-notification element with the same text from the properties which does not cause the screen reader to say anything and gives users with screen readers no indication of a slide change. Am I doing something wrong or is there more than can be done for those accessibility properties? |
I send related PR: #4884 |
Going through this now, when using I got it working half way with the keypress event, but this seems to not trigger on a shift+tab key press.
Would be great to get it working the opposite direction, right? Why isn't keyPress fired on shift+tab? Anyways, plan B, let's use an event listener.
There we go, that seems to work. I can tab back and forth between items, I can tab in and out of my slider, and it all seems to work. |
The trick is to make sure all slides that are visible have tabindex="-1" on all focusable elements. Once they are visible restore the former value. It is a bit more difficult than one expect in handlers because of the issues with the index in loop mode. There is no realIndex for the previous element. If you show more than one slide it becomes even more difficult as Swiper does not tell you how many and which elements are or have been visible. |
Requesting addition of |
I know that the React/Vue/Other framework ports aren't really fully supported, but it would be helpful to make a note in the docs that most (if not all) of this a11y work isn't reflected in the DOM elements created by those ports. e.g. Line 23 in 26340c4
|
Hey guys, thanks for the update, but you forgot to mention in the docs, that "watchSlidesProgress" is now required when you have slidesPerView > 1. Otherwise any focus on slides other than the first one will immediately slideTo() this slide. |
@Gr0m Thanks for catching this! 🙏🏻 |
Quick questions:
(brand new to a11y, just curious :) ) |
Because it won't tell the user something knew. If only one slide is visible the one will of course be the current one. In the pagination things are different. A screen reader will read out all bullets and then it makes sense to say which one is the current one.
IMHO it makes no sense. A list is announced with the number of items it contains. When only one slide is shown you will always stay on a lsit with a single item that changes with each next-slide action. It does not get much better if more than one slide is shown. Sidenote: I found an example by W3C/WAI that does not agree with me on that. But my codes without LI has passed professionaly a11y audits. So using LI is at least not a must-have.
I don't know why it is checked. At least for the VanillaJS version the other element is in the developers control. You can make it a The main point IMHO in using section/role is to hadd a hint that the user is about to enter a carousel widget. But the same as for 2. applies.
Yes, with the group role you emphasise that the content within belongs together. This is nice with a DIV though without an acessible name it is pointless. With LI as you point out the grouping is already there. For the slider containser see my anser to 3. |
Is there a follow-up ticket for these to issues?
My customers love slides with interactive elements and tabbing into out-of-view slides is highly undiserably. Only FF does not support the inert attribute. Using it by default would improve the experience for many users without much extra code. I assume that you have to enable visbility tracking to make it work, but that's ok. Disabled elements should not only be not focusable, but really disabled. There is an attribute/state for it. If it were used there wouldn't even be the need for a CSS classed to fake a disabled state. |
Seconding this. Focusable elements in out-of-view slides should NOT be able to be focused on, nor should they toggle a slide transition. That this happens seems to have been deliberate? |
I found a fairly easy solution for hiding the non-visible slides for screen readers: new Swiper(ele, {
// This option adds the `swiper-slide-visible` class to the slides once they are visible
watchSlidesProgress: true,
}); .swiper-slide {
visibility: visible;
transition: visibility 1s linear;
&:not(.swiper-slide-visible) {
// Hide non-visible slide from screen reader
visibility: hidden;
// Add transition to avoid flickering
transition: visibility 1s linear;
}
} |
Thanks a mill, it does the job for a barrier-free ux. 💪 |
This is a (multiple allowed):
bug
enhancement
feature-discussion (RFC)
Swiper Version: v4.5.0
Platform/Target and Browser Versions: EVERY PLATFORM
Expected Behavior
The slider should implement some of the features and html structure of the w3c slider example: https://www.w3.org/TR/wai-aria-practices/examples/carousel/carousel-1/carousel-1.html
The w3c example also has a list of reasons why the features and html structure they used is accessible.
This would make SwiperJS a lot more accessible. Some of the accessibility enhancements can be set with options, but this as the default would help users who don't know how to make it more accessible. It would also be a good reason to use this library instead of other slider libraries, because SwiperJS would be a good option for everyone who wants to apply wcag 2.1 standards. https://www.w3.org/TR/WCAG21/
Some of the features could only be implemented in the accessibility module, for users who don't want to use them.
Actual Behavior
SwiperJS is only partially accessible.
Features to implement:
<section>
as the outer container, instead of an<div>
elementaria-roledescription="carousel"
aria-label="Example Content"
(Label can be set like the next/prev slider buttons text)<a>
-tagsrole="button"
aria-controls="swiperID"
(this connects with the id of the content)id="swiperID"
(this connects with the aria-attribute of the next and previous buttons)aria-live="off"
when autoplay is on andaria-live="polite"
when autoplay is offrole="group"
aria-roledescription="slide"
aria-label="1 of 6"
Related Issues
The w3c example doesn't use clones, but this issue is also important to improve accessibility: #2929
Maybe the improved keyboard accessiblity would also fix issues like this: #2945
The text was updated successfully, but these errors were encountered: