accessible slick as tablist widget

[cielt]

the proposed changes here re-work the accessible slick carousel as an implementation of the aria tabpanel widget (note: this applies where dots: true in options). specifically

• .slick-dots assigned role=tablist
• slick dot buttons (child elements of .slick-dots > li or, the elements listening for the click event) are assigned the following attributes concerning a11y:
  • role=tab
  • id=slick-slider-control[_.instanceUid][slickDotCounter] where slickDotCounter is the (zero-based) index of this slick dot (out of the total number of slick dots)
  • aria-controls=slick-slide[_.instanceUid][controlledSlideIndex] where controlledSlideIndex is the (zero-based) slide index of the slide it advances to (e.g., if single item slick, dot aria-controls corresponds 1-1 to slick slide indices; if multiple items, dot aria-controls corresponds to every first slide in each "segment" [segment here meaning a set of slick slides displayed at one time on advancing through the carousel])
  • aria-selected='true' || null depending on whether the slide / slide group it controls is active / has focus
  • aria-label="x of [total]" where x is (one-based) index of current slick dot, within the total set
  • tabindex=0 || -1 depending on whether this slick dot is currently selected / active
• slick slide elements are assigned the following attributes that have to do with a11y:
  • role=tabpanel
  • id=slick-slide[_.instanceUid][_.instanceUid][slideIndex] where slideIndex is (zero-based) index of this slick slide within the set belonging to this slick instance
  • aria-describedby=slick-slide-control[_.instanceUid][controllingDotIndex] where controllingDotIndex is the (zero-based) index of the slick dot associated with this slick slide _if there is a slick dot associated with this slide (will not be the case in multiple items)
• on advancing to a new slide (slide group, in multiple slide instances), "focusable" child elements of the new current slide are made focusable (tabindex=0) by tabbing once again (in inactive slides, these elements have tabindex=-1) [used to set focus to the new current slide, this is NO LONGER the case]
• arrow key handler added to slick dots (where accessibility is true) so that, when one of the tabs has focus, user can traverse slick slides by hitting the right and left arrow keys

demo on jsfiddle // accessibility: true
http://jsfiddle.net/cielt/bjg4p066/36/

⬆️ note: for ease of checking this, the demo fiddle slides have content with a link in them, which should receive focus on tabbing once the slide has changed

any questions please let me know. thanks!

notes
accessible js "widgets", focus management, relevant aria- attributes:

• wai-aria spec for tab (and tabpanel)
• on accessible widgets

accessible tabpanel examples:

• accessible tabpanel
• another accessible tabpanel

[sboerjan]

I was writing the same solution untill I saw this Pull Request

Nice fix! :)

[cielt]

thanks @sboerjan :) please shout any thoughts / ideas you have -- would love to know what implementation details feel sound to other devs / users.

[marcoborsoi]

Nice work @cielt !! I was about to start writing a solution for the slick-dots and I found your PR. I hope this will be included very soon on the next version.

[cielt]

@marcoborsoi: great -- encouraged to know that you were thinking along these lines. would love to hear any further suggestions that may occur to you as we figure out these design patterns. cheers!

[scottaohara]

I was going through slick and planning to make a PR with many of the same edits. Glad to see it's already being suggested! Nice work here

[cielt]

thanks, @scottaohara! please shout if any thoughts / suggestions :)

[Gofilord]

This is great. How can I toggle these accessible features on and off for specific sliders? Currently it affects all the sliders I initialise.
Thanks!

[scottaohara]

@Gofilord why would you want a slider to be inaccessible?

[cielt]

@Gofilord: ⬆️ i was wondering that too ;) but, fair enough, since it is an option ... have you tried including accessibility: false in your slick settings when you create your slider? this should free you of the behaviors introduced in this PR. let me know if that doesn't work for you.

[Gofilord]

@scottaohara Sometimes this accessibility PR creates issues. For me - I had an auto sliding gallery at the top of the page. When I scrolled down and a new slide moved it, the whole page jumped up to where the slider is. I couldn't fix it, and it only happened with this upgraded accessibility slider; not with the standard one.

@cielt I have tried setting the accessibility to false, but it didn't help. However, I have found a solution:
I included both the standard slick and the accessible one. Then, in the accessible file, I changed the $.fn.slick to $.fn.accessibleSlick, so now I can apply the accessibility only to the sliders I want.

I know it's not very efficient because you have to include a library twice, but it fixes the jumping.

[cielt]

@Gofilord: i understand your issue now, and appreciate that one would not want all of these accessibility features 'on' across the board in this case. i'm not sure that i have a good solution yet, but in case it helps i'll offer my rationale for having done things as i have, and try to explain what's going on:

re the page jumping to the auto sliding instance when its current slide changes: if accessibility === true, i set focus to the newly current slide after the update. this is a convention that is part of the accessible "tablist" design pattern, wherein, on selecting a new "tab", focus is then set to its corresponding tabpanel. thinking through the UX for a screenreader this focus is meant to guide a user to content, newly revealed as a result of a selection.

that said, if one has a carousel on autoplay, and configured to behave thus (accessibility: true) and is elsewhere on the page then the next time a slide advances it will then receive focus and the browser will move (jump) to that part of the page because as far as it knows, this is where the current element of interest is. there are no scenarios i can think of, in which a user would want browser focus to be hijacked (and, possibly, for the window to jump to that content, if he is no longer interacting with it) every time the carousel updates.

arguably, a carousel on autoplay is inherently not accessible -- since the user does not have full control over the interaction, and it is hard to communicate the continually updating content in a non-intrusive manner to screen reader users. i revisited this article on the challenges of making carousels accessible and am struck by a number of requirements not yet covered in this work. some of this accessibility (e.g., button size, slick dots coming ahead of slides in HTML) involves style and markup decisions that are best left to those implementing a specific instance in a given context but it would be good to provide a foundation in the plugin that enables as much accessibility as possible (thinking about how to handle this given the scope of work here ...).

in response to your question, specifically -- about how to fix the page jump issue -- i would refine my earlier note and ask, have you tried setting accessibility: false for every slick instance that has autoplay: true? just now i tied marking up a scenario in which multiple slick instances live on a page, and if the autoplay ones are set to accessibility: false the page should not jump:
http://jsfiddle.net/cielt/4zr1osak/11/

change the setting, and you'll experience that jump issue you described.

this is an important caveat for this setting here and bears highlighting -- appreciate your calling attention to it. trying to work out how far to go with this (e.g., should accessibility be overridden by default if autoplay is true? should focusHandler be updated to respond differently for accessible slick?) but, in the mean time, i hope you have some clarity about your issue.

any questions / suggestions please let me know. thanks!

[Gofilord]

@cielt Thanks for the elaborate explanation and all the help.
Setting accessibility: false actually does solve my problem, turns out I had a typo.

[cielt]

Hi all,

many thanks for the work & input here(!) Apologies for my delay in getting to much of this feedback. In order, roughly, re what's new to me ...

@jasonday: the mechanics of selection and navigation are based on what i've read and understood to be conventions for this kind of UI component. i have no doubt that there is room to improve on the tablist implementation here, and there may be behavioral adjustments, as you point out, that make it work better in context as a carousel. will review your suggestions on the PR -- and thanks!

@janwidmer @scottaohara: re slick behavior where autoplay, pauseOnFocus and accessibility all are true, and arrows and dots (dots being a premise of this whole tablist setup), that's slightly tricky to me, figuring out how the prev / next arrows fit in with the tablist pattern. tbh i don't know what works best for screen readers and autoplay with all of the above settings but agree that for keyboard-only users the pause on arrows behavior would be an improvement, so will see about implementing this.

@Gofilord @leggomuhgreggo: re the problematic jump, where an autoplay slick instance has accessibility set to true and is autoplaying outside of the viewport, as noted earlier, autoplay is at odds with a11y because it wrests control from a user, imposes potentially disruptive animation, etc.. The focus() here is intended mostly to assist screen reader users (who likely wouldn't perceive the visual jump -- they are being made aware of what the browser is calling out to them by aural means). But agreed that this makes for disruptive UX for sighted users and the suggestion of disabling the focus() when a slick instance falls outside of the viewport makes sense to me. Will add this here.

As suggested by @scottaohara even better to disable the autoplay where carousels are not on the page (for non-sighted and sighted users alike) but it does get tricky knowing when to re-activate for the screenreader (autoplay!). Sounds like handling this would make for a solid future PR?

@leggomuhgreggo @scottaohara: i noted that the changes in master were merged in for a reason and didn't want to undo here, but your points are well taken.

As i understand it, TODOs include

• check kenwheeler.github.io/slick/ sliders are animating css 'left' instead of 'transform' #2078
• pause autoplay-ing slick where the focus is on an prev / next arrow and pauseOnFocus is true
• in accessible autoplay-ing slick, only focus() when instance is in viewport do not enact focus() behavior

Thanks!

[leggomuhgreggo]

@scottaohara I definitely see the logic in having a button to add that autoplay control, but I'm apprehensive about additional elements because it adds complexity to the API (enable bool, etc.), and risks a more heavily opinionated structure to contend with. I actually like the idea, the more I think about it, but I think it'd need to be hidden except for screen readers by default and probably should be it's own PR, subsequent to this one.

To me, the simplest (and ideal for the short term) solution would be to simply disable the focus when autoplay: true.

The alternative of having a check to see if the slider is offscreen probably a little bit closer to satisfying all use cases, but I think it'd be hard to find an elegant way to do that, and I feel like "why add the additional complexity, when autoplay isn't exactly compliant in the first place."

@cielt Thanks for the bang up job regrouping on this!

We're fixin to release 1.7.0 soon. I'd love to get the tabbed widget enhancement in there. I think at a minimum it needs to have a "focus jump" solution and solid testing to make sure tabbing and focusing are working as expected. Definitely not out of the question :)

Thanks!

[leggomuhgreggo]

@cielt nice!

Here're some test cases using this branch. And here're the tests using the current master if there's any question of an issue being related to this PR.

I went through quickly and didn't notice anything, but it'd be awesome if someone with more a11y expertise could go through and make sure things are working as expected.

Thanks!

[cielt]

@leggomuhgreggo thanks for this! 👍 will have a close look myself as soon as i can (realistically, in a day or 2) and make sure it holds solidly through allthecases. will also seek to get more feedback -- especially re the screenreader UX. carousels are often decried for being at odds with accessible UX but the goal here is to remove barriers where we can, and layer on some semantic cues and predictability in behavior that will improve access for all users, especially those relying on assistive technologies.

i'm sure you're aware, but wanted to highlight some challenges here, stemming from:

1. lack of 100% agreement about the conventions of how role=tablist (tab, tabpanel) key controls should work and, by extension, a solid mapping of this behavior to the components of slick carousel. the informing examples here, by my assessment, make sense and, in practice, afford a meaningful interaction with the widget as implemented (examples cited at the beginning of this thread);

2. inconsistency in how different screen readers mediate interaction with the role=tablist (tab, tabpanel) and aria, etc. attributes generally.

Both open questions bear further discussion but, in the absence of authoritative and complete precedents, here's what i'm thinking in my approach to this:

• do the proposed updates add predictable keyboard driven behavior, improve navigation mechanics and access to the controls and content of the carousel for keyboard-only users?
  => to a great extent, all keyboard users can check and weigh in on this point by using our keyboards only and verifying that the carousel is entirely accessible in this way

• do the proposed updates present to screen readers content substantively enhanced, according to a pattern recommended by the spec? do they help purposefully manage focus to convey interaction with the tabs-like interface, and improve the means of navigating and interacting with the carousel for screen readers?
  => the literature and basic screenreader testing is a good start, but i would love for more a11y specialists and experienced screen reader users to weigh in.

Will do some testing and let you know what i find out. Thanks!

[leggomuhgreggo]

Dope! Really appreciate your helping lead these efforts @cielt. Pumped to hear how things go.

Thanks!

[jasonday]

I'll jump in and do some testing across a few different devices & screenreaders and report back. I believe I tested my original pull request in NVDA/Firefox, Window Eyes/IE, and Voiceover/Safari (ipad). I don't have JAWS currently, so it would be great if someone could help out there.

…

On Mon, Jun 19, 2017 at 4:14 PM, leggomuhgreggo ***@***.***> wrote: Dope! Really appreciate your helping lead these efforts @cielt <https://github.com/cielt>. Pumped to hear how things go. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2119 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAtp67_AqWQ2Ydqsx7fsJx_EETlem9Ltks5sFta4gaJpZM4HZZwI> .

[cielt]

huzzah -- thanks @jasonday! 👏 will see if i can track down some help with JAWS.

[leggomuhgreggo]

Just checkin in. Squeaky wheel gets the oil, yada yada

[cielt]

Always has 🛢️

Apologies, it's been a minute. Only small updates as yet:

• one UX discrepancy / snag is that, with the auto focus happening in accessibility: true, if the carousel is partially off screen, the auto focussing brings the current slide back into the viewport, potentially causing a jump for sighted users. it's the same symptom we tried fixing for autoplay: true

• from my (quite novice) testing with Voiceover + Safari, the aim of making content within the tabpanels (i.e., slick slides) reachable. would love additional opinions, though.

• in hopes of understanding semantic / access gains (or losses) from the tablist framing, i reached out to a couple of screen reader users but haven't heard back. i will ping them again and see if i can track down some more, but not sure how soon i can get enough to go on.

All i got at the moment. It may be some time so, for your planning purposes, getting answers soon seems optimistic. But seems important to verify that these changes are actually on the right track for a11y.

Thanks for touching base!

[miconley]

@cielt I tested this in NVDA/Firefox, JAWS/IE as well as VoiceOver and personally I find it much more useable compared to the demo carousels on the main site. Perhaps the single most important fix is the removal of aria-live from the container. My experience with this carousel is it's gets too noisey and conflicts with parent containers with aria-live already present.

My recommendations (and anyone feel free to add):

• Include default focus state for all actionable elements (outline, etc)
• When focus auto-updates after "prev/next", it should be to the tab panel itself instead of the inner content (a bit confusing given focus moves to the inner link)

Great work! Hopefully this will be merged.

[cielt]

@miconley: thanks a million for the feedback & SR testing assist! The removal of aria-live was introduced in a separate PR [#2843] but agreed it should be deployed with discretion.

Couple of updates:

• where accessibility: true, set focus to the newly current .slick-slide[role=tabpanel]
• if prev / next arrow button has focus, will pause an autoplay-ing carousel with pauseOnFocus: true (this is independent of the accessibility option, but agree the behavior makes sense)
• where accessibility: true, if prev / next arrow button has focus, the < > key controls are enabled

Please lmk if any questions / suggestions. Thanks!

[leggomuhgreggo]

@cielt dude nice work! You think this is good for a merge?

[cielt]

@leggomuhgreggo: just wanted to add a brief update to the README. Will gladly hear out concerns / suggestions from anyone who's got them, but here's the current final snapshot of what i'm proposing. Thanks!

[leggomuhgreggo]

@cielt helll yeee. Did some testing. Looks good. Merged. Will get the release out soon. Thanks to everyone who helped with this enhancement!

[cielt]

Excellent, @leggomuhgreggo! Echoing thanks to all here for your suggestions & help with this! 👏👏

[miconley]

So awesome to see this, great work @cielt 🍻

[janwidmer]

Thanks for the great work.. any idea, if those changes will land in a release prior to 2.0?

[leggomuhgreggo]

@janwidmer it will be in a release that's coming out in the next couple days

[janwidmer]

Hey..
I just tested our Site containing a slick slider with an accessibility expert and also a blind test person.

They mentioned two things which could get improved:

1. The attribute role="toolbar" on the pagination wrapper div is not needed and seems to confuse the screenreader (both NVDA and JAWS)
2. The attribute aria-live="polite" on the div with the class "slick-list" is not needed since it also distracts the user. They would rather use it for small updates, e.g. a result amount..

@miconley did you also notice these "problems" when testing?

[cielt]

@janwidmer: Very good to know. Thanks so much for putting the current build to the test with true a11y specialists -- and following up! I can appreciate that aria- attributes should be used sparingly and intentionally and believe that contributors have made their best efforts to do this. These attributes must have been included for a reason but it seems that they are almost competing with (or at least extra on top of) other attributes serving specifically to implement slick as tablist which was the original focus of this PR.

Would be very interested to hear what folks think. i do not see any open PRs proposing feature:a11y updates at the moment but if this sounds like a solid way forward, would encourage creating one.

Thanks!

[janwidmer]

@cielt of course the contributors have made their best effort and that's really nice. The current state is already a massive improvement regarding a11y.

I would also be interested in other folks thoughts since the whole a11y topic is sometimes very subjective..