Move focusgroup functionality to a separate polyfill#51
Conversation
|
|
||
| let insideFocusGroup = | ||
| event.target.role === 'menuitem' || | ||
| event.target.role === 'button' || |
There was a problem hiding this comment.
Why we added all of these?
Should we also add focusgroup?
There was a problem hiding this comment.
When focus is on an element within a focusgroup, hotkeys are restricted to that group only.
The filtering of elements follows the same logic as implemented in the getItems
function getItems(group) {
if (group.hasAttribute('focusgroup')) {
let items = [...group.querySelectorAll('*:not([focusgroup="none"])')]
return items.filter(item => {
return (
item.role === 'button' ||
item.type === 'button' ||
item.role === 'checkbox' ||
item.type === 'checkbox'
)
})
}
}There was a problem hiding this comment.
We should disable one-letter hotkeys only for cases where we will have a conflict.
In menuitem we have a search to find next element. This is why we disable only one-letter hot keys, because they will conflict with this search.
As I understand focusgroup has no search, so there will be no conflicts.
There was a problem hiding this comment.
Looks like you're right, there's nothing about search in the spec. In that case, I'll remove all search-related functionality from the polyfill and update the API to focusGroupPolyfill() without the FocusGroupKeyUXOptions.
| @@ -0,0 +1,477 @@ | |||
| import { JSDOM } from 'jsdom' | |||
There was a problem hiding this comment.
Can we add a few tests for focusGroupPolyfill and focusGroupKeyUX to be sure that there are no conflicts between them?
2 participants
Separated focusgroup attribute functionality from roles and implemented it as an independent polyfill.