fix(CQ-4295097): Accessibility - Spectrum SelectList user is not able to select next group with keyboard #127
Conversation
…select next group with keyboard CQ-4295097 refactor _tabTarget setter and _resetTabTarget method With SelectList groups only one item should receive focus at a time. When resetting tabTarget, prioritized selected items over selectable items, but prioritize the last focused item over the first of the items in the array. In Select component, we _resetTabTarget before the list opens, prioritizing the first selected item over the first selectable item.
|
@icaraps and @hameed87 This PR supersedes #124, combining changes from #124 and hameed87#1 into a single commit. |
…extSelectable Fixes TabList unit test failures.
|
@majornista Can you please add a test case ? |
majornista
changed the title
| if (items.length > 0) { | ||
| items[0].focus(); | ||
| const firstFocusable = items.find(item => item.tabIndex === 0); | ||
| (firstFocusable || items[0]).focus(); |
There was a problem hiding this comment.
Why do we have this check item.tabIndex === 0 ?
What if the first item is button or anchorlink which are focusable but don't require tabindex=0?
the itemSelector at L-68 has 'button[is="coral-buttonlist-item"], a[is="coral-anchorlist-item"]'
Shouldn't we fix the same at _focusFirstItem and _focusLastItem etc.. which are called on key:home and key:end?
There was a problem hiding this comment.
We want to prioritize the active or first selected item over the first item.
There was a problem hiding this comment.
We are identifying firstFocusable by "item.tabIndex === 0" but items like buttons and anchorlinks won't have tabindex=0, they are by default focusable.
Lets say there are list items in following order .
button,
coral-seelct-list
coral-seelct-list
AFAIK here firstFocusable should be button, but button won;'t have tabindex=0.
So we will be focusing on second item 'coral-seelct-list' instead of button.
Please let me know if my understanding above isn't correct?
We want to prioritize the active or first selected item over the first item.
Do we have any other reliable way to detect last active/selected item ?
| this._tabTarget = forceFirst ? items[0] : (items.find(item => item.tabIndex === 0) || items[0]); | ||
| this._tabTarget = forceFirst ? items[0] : (items.find(item => item.tabIndex === 0) || items[0]); | ||
|
|
||
| if (this.groups && this._groups.getAll().length > 0) { |
There was a problem hiding this comment.
Can the _tabTarget be outside of groups?
If not then in the code below line-415 will always set the firstFocusable tabindex to -1 and we won't need to set it to 0 .
What is the intention of code below line-415?
AFAIK we want to set tabindex=-1 on all but first item across all groups, so only one item can be focused across all the groups.
Doesn't line-227 do the same thing? (set tabindex = -1 on all but one item)
## [4.10.6](v4.10.5...v4.10.6) (2020-10-19) ### Bug Fixes * **CQ-4295097:** Accessibility - Spectrum SelectList user is not able to select next group with keyboard ([#127](#127)) ([c80e629](c80e629))
|
🎉 This PR is included in version 4.10.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
CQ-4295097 refactor _tabTarget setter and _resetTabTarget method
With SelectList groups only one item should receive focus at a time.
When resetting tabTarget, prioritized selected items over selectable items, but prioritize the last focused item over the first of the items in the array.
In Select component, we _resetTabTarget before the list opens, prioritizing the first selected item over the first selectable item.
Related Issue
#124 and https://jira.corp.adobe.com/browse/CQ-4295097
Motivation and Context
Correct behavior of a select list with option groups, by ensuring that only one item in the select list is tabbable at a time.
How Has This Been Tested?
Behavior has been tested in example documents for SelectList and Select.
Screenshots (if appropriate):
Types of changes
Checklist: