reactjs / react-tabs

An accessible and easy tab component for ReactJS.
https://reactcommunity.org/react-tabs/
MIT License
3.1k stars 447 forks source link

keyboard accessibility of https://reactcommunity.org/react-tabs/ #479

Open patrickhlauke opened 2 years ago

patrickhlauke commented 2 years ago

currently, there are two quite fundamental keyboard access issues on the demo page at https://reactcommunity.org/react-tabs/

Video showing the issue (using Chrome on Windows)

https://user-images.githubusercontent.com/895831/173188543-011a8b79-d55b-4eeb-b907-1fb9b0033cf9.mp4

0:00 - 0:25 tabbing through the page. note the focus disappears after the "View source" checkbox. ticking the checkbox with the mouse, and carrying on tabbing, shows that tab was in the textarea (and it's stuck in there) 0:25 - 0:32 focus is on the tab/tablist, can use cursor keys to change active tab. but there's no visible indication that focus is indeed on that tab/tablist

patrickhlauke commented 2 years ago

for the focus indicator issue at least, suggest adding some :focus-visible styles (so they won't get in the way of mouse/touch users, but will show when keyboard users navigate). possibly, this is something more fundamental that should be added to the default styles for this component itself.

patrickhlauke commented 2 years ago

having played around with this some more, it seems :focus-visible won't work here - the styles show even for mouse clicks. I suspect, though haven't jumped into the underlying code of the component, that it's because the <li>s are completely ripped out of the document and then regenerated, and focus is then programmatically moved to the newly selected tab ... which the browser then counts as being a case where :focus-visible would show.

joepvl commented 2 years ago

Hey Patrick, thanks for the research! Kudos for posting a video with your issue. Is this something you'd like to create a PR for? I'm not sure if it's just an issue with the docs; it could be that the library itself has some issues with focus management (it's one of the more complex pieces of code in this repo if I recall correctly).

patrickhlauke commented 2 years ago

the first part (focus being swallowed by the textarea that's been extended to be a full code highlighted/editor, and the textarea also receiving focus even when not visible) is definitely an issue with the docs/demo.

the lack of focus indication, on the other hand, seems to be fundamentally a react-tabs problem.

I can have a look at both, but can't make promises on when/if I manage to crack those two problems. we'll see...

patrickhlauke commented 2 years ago

had another closer look, and purely for the demo/example, the issue of the code editor swallowing Tab seems to be something that was fixed upstream in https://github.com/satya164/react-simple-code-editor - using the slightly unusual Ctrl+M shortcut, I can switch tab capture off and proceed out of the code editor. Suggest this should be provided as part of the hint text, so users know (as it's not a common thing/pattern). I can make a small PR just for this aspect.

the fact that the code editor is focusable even when not shown is due to the styles used to hide the editor only visually hiding, rather than fully using display:none. This is likely because display:none can't be animated/transitioned. So you'd either want to add display:none when it's closed, and quickly change to display:block just before it starts animating (and vice-versa, once the closing animation finished, swapping it back from display:block to display:none). alternatively, if there's some way to make the editor non-focusable when not visible - for instance having disabled attribute on the <textarea> - and then making it focusable when visible - removing disabled - that would solve this as well, and would probably be easier than timed style changes. this is the part where my weak react skills unfortunately won't help, so won't be able to tackle this.

as for the focus indication, i can propose something in a PR. however, my initial idea of using :focus-visible to only have it for keyboard users won't work, as mentioned above (due to the way the whole <li> is removed, and then re-added to the DOM and focus programmatically moved, the :focus-visible heuristic in the browser is overridden and it will always show, even as a result of a mouse click or touch tap). so it'll have to be some kind of focus indication that will also show on click/tap i'm afraid. but that can probably be hashed out in the separate PR.