Docs Search allows for the container holding the options to be tabbed on in FireFox

[Hugos68]

Current Behavior

When tabbing through the items from the search on the docs site, the first thing you tab onto it the container that holds all the searchable options, this doesn't server any purpose.

Expected Behavior

When tabbing off the search you'd want to tab onto the first element in the list.

Steps To Reproduce

1. Open Firefox
2. Go to https://www.skeleton.dev/
3. hit Ctrl + K
4. hit tab
5. Your tab focus should now be on the container holding the searchable elements

Link to Reproduction / Stackblitz

No response

More Information

This is fixable by setting the tabindex to -1 on the container to make it avoid tabs, this shouldn't concern accessability since it does not serve any purpose

[Mahmoud-zino]

Note:

1. This only happens when the container is scrollable (there are more items than we can display)
2. The same behavior occurs in Autocomplete.

[Hugos68]

Note:

1. This only happens when the container is scrollable (there are more items than we can display)

2. The same behavior occurs in `Autocomplete`.

Oh Interesting, I though this only occured on the DocsSearch because I was convinced it was a custom implementation, should I apply the same fix to the Autocomplete Component?

[Mahmoud-zino]

I really don't know... better ask @endigo9740 in these situations 😄.

[endigo9740]

@Hugos68 also to clarify - the doc search and autocomplete are two separate components. They may act in a similar manner, but they are not the same thing. I might recommend we update the title to reflect this, and consider logging this as two separate issues.

[Hugos68]

@Hugos68 also to clarify - the doc search and autocomplete are two separate components. They may act in a similar manner, but they are not the same thing. I might recommend we update the title to reflect this, and consider logging this as two separate issues.

Will do, it actually first was only an issue targeted at the doc search but @Mahmoud-zino said it also affected the Autcomplete component, I should have probably tested it first but I took his word. I will change it back to only targeting the doc search

[Mahmoud-zino]

@Hugos68 sorry for the confusion 😄, I mentioned that:

The same behavior occurs in Autocomplete.

which is still true, but could be caused by another bug in the code of the Autocomplete and we need to handle that also. 👍

[Hugos68]

@Hugos68

sorry for the confusion 😄, I mentioned that:

The same behavior occurs in Autocomplete.

which is still true, but could be caused by another bug in the code of the Autocomplete and we need to handle that also. 👍

Yeah I agree it works on the autocompletes but for autocompletes it should be handled by the developer implementing the surrounding container

[Mahmoud-zino]

I have spent some more time investigating this and here is what I have found: Both problems (Autocomplete and DocSearch) are indeed related, let me explain:

The problem is that overflow- CSS class behaves differently in Firefox, it sets the tabIndex to 0 by default to support keyboard-only-users. quoting mdn docs:

A scrolling content area cannot be scrolled by a keyboard-only user, with the exception of users on Firefox (which makes the container keyboard focusable by default).

So this bug is not related to the focus-trap and we can solve it by adding tabindex="-1" to every div that has overflow.

@endigo9740 as the CSS wizard, maybe you have a better solution for this... 😄

and it would be noice if we add a note in the docs somewhere to explain this behavior and how we would fix it.

[endigo9740]

@Mahmoud-zino great work, and of course it's due to typical browser vendor BS heh. I can't really imagine a better solution than the tabindex option provided here. I'd say for components we control we set that for users, and for areas that we don't (wrapping elements provided by users) we at least give a small warning.

I'm thinking something like:

Browser Support In Firefox, when wrapping the autocomplete component in an element that has an overflow, make sure to set tabindex="-1". This will ensure tab navigation works as expected.

Something along those lines.

@Hugos68 it sounds like your changes to the focus-trap may not be needed in this case? How would you like to handle the PR? Should we go ahead and close that out for now?

@Hugos68 @Mahmoud-zino - I'd welcome a PR with the above changes. I'd say the first to respond can take it so we're not doubling up on effort here.

I'd encourage all three of us test this when ready though.

[Hugos68]

@Mahmoud-zino great work, and of course it's due to typical browser vendor BS heh. I can't really imagine a better solution than the tabindex option provided here. I'd say for components we control we set that for users, and for areas that we don't (wrapping elements provided by users) we at least give a small warning.

I'm thinking something like:

Browser Support In Firefox, when wrapping the autocomplete component in an element that has an overflow, make sure to set tabindex="-1". This will ensure tab navigation works as expected.

Something along those lines.

@Hugos68 it sounds like your changes to the focus-trap may not be needed in this case? How would you like to handle the PR? Should we go ahead and close that out for now?

@Hugos68 @Mahmoud-zino - I'd welcome a PR with the above changes. I'd say the first to respond can take it so we're not doubling up on effort here.

I'd encourage all three of us test this when ready though.

I think you are confusing 2 seperate problems here Chris, as I stated above I already knew this was fixable with tabindex="-1" the issue and pr related to focus trap is a different problem that requires a differnt solution

[endigo9740]

@Hugos68 that's right, you mentioned some of the changes were related to performance optimization strategies for monitoring changes to the children. In that case let's split off the tabindex issue to it's own dedicated ticket. Given you're handling the focus trap update in the current PR @Hugos68 do you mind if @Mahmoud-zino takes the other? (assuming he's open to it)

[Mahmoud-zino]

@endigo9740 It's already split, there is another issue for the focustrap escaping custom modals (image, embedded, etc..) -> #1613 , but since @Hugos68 already created a 2 PR's for both I would let him complete the work he is doing. but I am here to help at any time.

[endigo9740]

Thanks @Mahmoud-zino, I'm going to modify Hugos PR #1613 to implement the suggestions above. That should be in shortly. Per discussion on Discord - I think there's still relevance to review the focus trap changes he's submitted as well, so I'll review that in isolation soon. Mahmoud, I would appreciate you reviewing those changes as well - given you're familiar with the feature from your recent additions. But no pressure.

[Mahmoud-zino]

Yeah sure 👍