Fix screenreader list position announcement regression #19605
Conversation
| @@ -499,7 +499,7 @@ export class CommitList extends React.Component< | |||
| {this.renderReorderCommitsHint()} | |||
| <List | |||
| ariaLabel="Commits" | |||
| role={this.props.isInformationalView === true ? 'list' : 'list-box'} | |||
There was a problem hiding this comment.
Yes.. Not only did I goof on checking for undefined against an item that couldn't be undefined... but also I yes switched listbox to list-box (my malapropism acting up..). But it most definitely is listbox https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/listbox_role
| * 'option' because that have selection capability. In that case, a | ||
| * screenreader will only browse to the selected list option. If the list is | ||
| * meant to be informational as opposed for selection, we should use `list` | ||
| * with `listitem` as the role for the items so browse mode can navigate them. | ||
| */ | ||
| readonly role?: `option` | `listitem` | 'presentation' |
There was a problem hiding this comment.
I also for some reason used backtick.. when it would more properly be a single quote... surprised the linter didn't hammer me on that.
| @@ -1188,7 +1188,7 @@ export class List extends React.Component<IListProps, IListState> { | |||
| <ListRow | |||
| key={params.key} | |||
| id={id} | |||
| role={this.props.role === undefined ? undefined : 'listitem'} | |||
There was a problem hiding this comment.
In ListRow the logic is:
sectionHasHeader && rowIndex.row === 0
? 'presentation'
: role ?? 'option'
List logic:
role={this.props.role ?? 'listbox'}
So our two use cases are:
- CommitList which now does define a role as either
listorlistbox. (History or unreachable commit list) - Every other list where role = undefined.
For every other, we want to keep current behavior as listbox and option. This does uphold, if role is undefined, the main grid is "defined or listbox", this logic here says undefined !== list so the ListBox will receive undefined. which will then filter through the sectionHeader logic to the same "definedRole or option".
For the CommitList, it will send in a defined role of list or listbox so it will pass that and set it on the main List. and this condition will say if it list to define a role for the list item as listitem otherwise do undefined which will again follow the above and use the default of option assuming it isn't a sectionHeader row.
Closes #19603
Description
When working on #19341, to make it so non-selectable list items could be browsed, I unintentionally made it so that selectable list items would take the same behavior. My mistake was seeing that the role of the list was still correct after changes.. but the actual list items now had role of
listitemas opposed tooptionfor non-presentational lists.Screenshots
Commit list with correct role of
listboxand items with role ofoption.Commit unreachable list with correct role of
listand items with role oflistitemShowing item number on NVDA:
Release notes
Notes: [Fixed] Screen readers announce the position of the list items in selectable lists such as the history commit list.