feat: make Toggle support keyboard #417
Conversation
| return; | ||
| } | ||
|
|
||
| input?.click(); |
There was a problem hiding this comment.
For a normal checkbox, one would be able to tab to it to focus it and press space to toggle it, right?
Why is that not possible here, given that the Toggle component is implemented with a checkbox?
Is it because the checkbox has display: none?
There was a problem hiding this comment.
Is it because the checkbox has display: none?
Yes.
There was a problem hiding this comment.
Could it be solved by making the checkbox visibility: hidden and/or height: 0 instead, or by giving it tabindex="0"?
I think it would be preferable if we could rely on the default browser behavior instead of implementing a keydown listener.
There was a problem hiding this comment.
Not sure it is a better solution. If we do so, assuming it would work out, then we would need extra CSS and maybe even a state too to highlight correctly focus, hover and other feedback of the related UI components. One or the other...
There was a problem hiding this comment.
Also probably more development effort.
There was a problem hiding this comment.
That said, here I just realized disabled is not handled and should been added to handle the keyboard event in such case.
Let me know if you rather like going CSS etc. and we close the PR (and comeback to this when we've got time) or otherwise I also add a if to handle disable state and improve this PR.
I'm fine either way, your call.
There was a problem hiding this comment.
Either way is fine with me.
Motivation
I noticed that
Togglecouldn't be checked per keyboard navigation. This extend their accessibility for keyboard support.Changes
keydownsupport and accessibility toTogglecomponentwidth: fit-contentgiven that the toggle can receive focus - i.e. scope active outline style to the element