Splidejs / splide

Splide is a lightweight, flexible and accessible slider/carousel written in TypeScript. No dependencies, no Lighthouse errors.
https://splidejs.com
MIT License
4.83k stars 418 forks source link

tabindex="-1" removed from elements inside visible slides #1185

Open foobear opened 1 year ago

foobear commented 1 year ago

Checks

Version

4.1.4

Description

First of all, thanks for Splide. It's a really great library to work with, and I love it! :sparkling_heart:

Today I encountered an issue with slides which contain elements that have tabindex="-1". The tabindex attribute is lost even though it was intentionally set to -1.

I understand this is because of the focusableNodes option and that focusable nodes in invisible slides need a negative tabindex.

Reproduction Link

https://codepen.io/foobear/pen/VwVwjKe

Steps to Reproduce

  1. Have elements in slides with negative tabindex

Expected Behaviour

Ideally, Splide should store any previous tabindex value (maybe simply in an HTML attribute on the element) and, instead of removing tabindex when slides become visible, restore such a previous value.

JonKaric commented 1 year ago

@foobear Could you describe why you need the element to be unfocusable?

foobear commented 1 year ago

@JonKaric Sure. In my case, slides contained two links to the same target: one around the article image, and one around the article title. I wanted only the title link to be keyboard-focusable, as users Tab-navigating should skip over the linked image, so that link had a negative tabindex set.

<div class="slide">
  <a href="/example" tabindex="-1"><img src="article-image.jpg"></a>
  <a href="/example">Article Title</a>
</div>

So not the slide element itself would have a negative tabindex, but an element inside of it.

Egpereira commented 1 year ago

Hi @foobear! I had a similar problem in my work, and I solved it by changing the focusableNodes option to be 'a:not([aria-hidden]), button, textarea, input, select, iframe'.

For this to work, you will also have to add aria-hidden="true" to your link with the image, but I guess that isn't much of a problem. This makes splide only handle "regular" links that you didn't manually set to not focusable with tabindex and aria-hidden.

foobear commented 1 year ago

Thanks for the suggestion @Egpereira. I do not want to hide the image inside the link element from the accessibility tree, so in this case it's unfortunately not an option for me. However, your comment made me realize that I can use the focusNodes option to achieve what I want. 🙂

I now set a custom attribute avoid-focus on all elements with a negative tabindex before initializing Splide.

slides.querySelectorAll('[tabindex="-1"]').forEach((element) => {
  element.setAttribute('avoid-focus', true)
})

Then we can initialize Splide with focusableNodes set to 'a:not([avoid-focus]), button:not([avoid-focus])' (in my case there won't be any form controls or frames inside slides) and Splide will keep (or assign) tabindex="-1" on those elements that should not be focused.

A built-in solution would still be nice, but I'm happy enough with my workaround. :smile:

JonKaric commented 1 year ago

You could cut out the JS and do it straight in the focusableNodes option, querySelector and focusableNodes both take a css selector list it seems. a:not([tabindex="-1"])

Maybe a stretched link might serve you better though so you don't have to mess around with focusableNodes at all :)

foobear commented 1 year ago

You could cut out the JS and do it straight in the focusableNodes option, querySelector and focusableNodes both take a css selector list it seems. a:not([tabindex="-1"])

Unfortunately, using a:not([tabindex="-1"]) for the focusableNodes will make all links unfocusable in slides that are insvisible initially, because Splide will not remove the negative tabindex that it set inside invisible slides once they become visible.

Here is an updated Codepen to visualize: https://codepen.io/foobear/pen/rNQLaJv

I understand why Splide does everything the way it does, and I think that's generally the correct way to do it. If it just were a bit smarter when restoring elements in visible slides... :)

Maybe a stretched link might serve you better though so you don't have to mess around with focusableNodes at all :)

Thanks for the suggestion. A strechted link was indeed an option initially, but we have to provide extra accessibility support attributes (since the stretched element has no "proper" content) and re-implement hover/focus styles because users will not actually hover a "real" link. Not impossible, but more work and IMO not much better than the workaround I have. (Ideally, the whole slide should just be an <a> itself, but it contains other links with different targets, so that's not an option.)