aria listbox pattern full implementation #9
Conversation
| href="" | ||
| :id="buttonId" | ||
| :data-expanded="isExpanded" | ||
| v-on:click.prevent="selectButton" |
There was a problem hiding this comment.
no need for a click action anymore, even on the <li> ?
There was a problem hiding this comment.
if not then the selectButton method can be removed
There was a problem hiding this comment.
Did some manual testing and it still needs the click action so I brought it to the li
| const itemSelector = `ckeditor5-toolbar-item ckeditor5-toolbar-item-${props.id}`; | ||
| const buttonSelector = `ckeditor5-toolbar-button ckeditor5-toolbar-button-${props.id}`; | ||
| const buttonId = `${props.id}-button-${Math.random().toString(36).substring(7)}`; |
There was a problem hiding this comment.
just added this buttonId, in the last PR, there's no more use for it?
There was a problem hiding this comment.
No more use for it. The previous issue addressed a duplicate ID problem. In this issue, the refactoring included changing focus behavior so IDs aren't needed, which was the only thing the IDs were needed for.
There was a problem hiding this comment.
Keeping the focus on the active button when moving between lists between the keyboards is much better now, switching back to main and it seems obviously broken. Now it's possible to really quickly move items between available and active, and stay on the same item the entire time. Nice work!
2 participants
Upon further review the work in #8 needed to be elaborated upon.
The listbox role is what allows the button lists to be identified as active elements within the form. A Listbox requires the immediate children to be focusable, and those must have no focusable children of their own. Fortunately it was relatively easy to change the
<a>"buttons" to<span>and move the keyboard and focus handlers to the<li>. It's a big refactor so there may be a missing use case in there...