Improve toolbox behaviour for keyboard and screen reader users#11189
Improve toolbox behaviour for keyboard and screen reader users#11189microbit-robert wants to merge 7 commits intomicrosoft:masterfrom
Conversation
This improves screen reader output as the toolbox item count is constant. This removes the notion of expanded, since there are no controls to expand or collapse tree items, they are just shown when navigated to.
Remember last selected category when focused. Fix toolbox blur behaviour. Fix bug when returning to the toolbox from the search flyout.
This still prevents Enter / Space from toggling the flyout open / closed.
This is special casing the namespace as a low effort way to achieve this. Translations already handle this in a better way than English.
The screen reader output is faster and less ambigious this way
| iconContent = undefined; | ||
| } | ||
| const rowTitle = name ? name : Util.capitalize(subns || nameid); | ||
| const rowTitle = name ? name : nameid === 'led' ? nameid.toUpperCase() : Util.capitalize(subns || nameid); |
There was a problem hiding this comment.
this seems a little too hacky for me... should we not just rename the category to LED?
There was a problem hiding this comment.
Agreed. This is in a few places which I can remove in favour of name then nameid.
I guess renaming the namespace breaks a lot of things. Adding block="LED" to the namespace will do the right thing but break existing translations?
| {treeRow.subcategories?.map(subTreeRow => | ||
| <CategoryItem | ||
| key={subTreeRow.nameid + subTreeRow.subns} | ||
| className={classList(expandedItem != treeRow.nameid && "sr-only")} |
There was a problem hiding this comment.
where is this sr-only class defined?
There was a problem hiding this comment.
I had (incorrectly) assumed it was a semantic CSS built-in class. Here is the definition. Should it be moved/copied elsewhere?
| selected={selectedItem == (subTreeRow.nameid + subTreeRow.subns)} | ||
| treeRow={subTreeRow} | ||
| onCategoryClick={this.onCategoryClick} | ||
| ariaLevel={1} |
There was a problem hiding this comment.
if everything is now at the same ariaLevel, can we just remove the attribute?
There was a problem hiding this comment.
In theory, yes. In practice, it depends on whether screen readers correctly infer the default value of 1. I'll test this.
2 participants
This PR:
idto the toolbox to ensure that code related to Blockly's focus manager runs (see fix: Short-circuit node lookups for missing IDs RaspberryPiFoundation/blockly#9174)