search

ai / keyux

JS library to improve keyboard UI of web apps
https://ai.github.io/keyux/
MIT License
391 stars 18 forks source link

`focus-group` support `toolbar` #17

Open dmitry-kurmanov opened 6 months ago

dmitry-kurmanov commented 6 months ago

based on https://github.com/ai/keyux/issues/9#issue-2151620436

Add role="toolbar" support with horizontal default direction.

As for toolbar it might be a problem because it can contains different types of items e.g. button, spinbutton, checkbox, radio etc - the toolbar example

ai commented 6 months ago

I think we can make test like:

if ((el.target.tagName === 'BUTTON' || e.target.role === 'button' || …) & el.target.closes('[role=toolbar]')) {
dmitry-kurmanov commented 6 months ago

@ai I will try to implement it if you don't mind.

dmitry-kurmanov commented 5 months ago

After some investigation I changed my mind about el.target.tagName === 'BUTTON'. I suppose we shouldn't support it. By default button has submit type, so to make buttons work properly in the toolbar they should have explicitly set of type="button. See the the toolbar example

That is why I propose not to use the tag selector for the button.

ai commented 5 months ago

After some investigation I changed my mind about el.target.tagName === 'BUTTON'. I suppose we shouldn't support it. By default button has submit type, so to make buttons work properly in the toolbar they should have explicitly set of type="button. See the the toolbar example

OK

Moreover, probably instead of a list of available item types we could support any focusable type via the node.tabIndex property.

Adding tabindex on button/input/a looks not very elegant.

dmitry-kurmanov commented 5 months ago

After some investigation I changed my mind about el.target.tagName === 'BUTTON'. I suppose we shouldn't support it. By default button has submit type, so to make buttons work properly in the toolbar they should have explicitly set of type="button. See the the toolbar example

OK

Moreover, probably instead of a list of available item types we could support any focusable type via the node.tabIndex property.

Adding tabindex on button/input/a looks not very elegant.

I removed the comment because the "tabindex idea" will not work. We set tabindex='-1' to prevent extra tab focus while we are inside the focus-group. I will think how to implement it.