Implement keyboard and screen reader accessibility for LED matrix field#10390
Implement keyboard and screen reader accessibility for LED matrix field#10390riknoll merged 2 commits intomicrosoft:masterfrom
Conversation
| // The height and width must be set by the render function | ||
| this.size_.height = this.scale * Number(this.matrixHeight) * (FieldMatrix.CELL_WIDTH + FieldMatrix.CELL_VERTICAL_MARGIN) + FieldMatrix.CELL_VERTICAL_MARGIN * 2 + FieldMatrix.BOTTOM_MARGIN + this.getXAxisHeight() | ||
| this.size_.width = this.scale * Number(this.matrixWidth) * (FieldMatrix.CELL_WIDTH + FieldMatrix.CELL_HORIZONTAL_MARGIN) + this.getYAxisWidth(); | ||
| this.size_.width = this.scale * Number(this.matrixWidth) * (FieldMatrix.CELL_WIDTH + FieldMatrix.CELL_HORIZONTAL_MARGIN) + FieldMatrix.CELL_HORIZONTAL_MARGIN + this.getYAxisWidth(); |
There was a problem hiding this comment.
This adds trailing margin to center the field matrix LEDs (more obvious with the Blockly cursor present), which makes this block slightly wider.
Before:
After:
| .blocklyVerticalMarker { | ||
| fill: none; | ||
| } |
There was a problem hiding this comment.
This removes the fill from the Blockly cursor to improve contrast inside the block. This normally covers a smaller area such as an input or a dropdown, but it is unhelpful for this unique, immediately editable field.
The Blockly cursor with fill:
|
The focus indicator we have used for keyboard navigation changes depending on the state of the LEDs. We need to do this to keep the focus indicator contrast acceptable against blocks of different colours, and the colour of the LED. If the LED is off, we use white, otherwise we use black and white together. See the example below with some CSS classes manually applied:
|
pxtblocks/fields/field_ledmatrix.ts
Outdated
| break; | ||
| } | ||
| case "Escape": { | ||
| (Blockly.getMainWorkspace() as Blockly.WorkspaceSvg).markFocused(); |
There was a problem hiding this comment.
should this be this.sourceBlock_.workspace?
pxtblocks/fields/field_ledmatrix.ts
Outdated
| } | ||
|
|
||
| this.fieldGroup_.replaceChild(this.elt, this.fieldGroup_.firstChild); | ||
| this.elt.addEventListener("keydown", this.keyHandler.bind(this)); |
There was a problem hiding this comment.
check to make sure the sourceblock isn't within a flyout before registering these handlers. the outer if statement already handles insertion markers, but we don't want this messing with keyboard navigation inside the toolbox.
There was a problem hiding this comment.
I've made this change as it does no harm 09c91ce. However, I don't think other Blockly core field editors have any checks against this. There is a specific flyout cursor for the both the old and new versions of the keyboard navigation plugin that does not allow the user to move in to the block, so it's not possible to focus the field on which these event handlers are attached.
It would be good to decide what pattern we should follow going forward as we look at more custom fields. Should we always do this check, or rely on the flyout cursor to do the right thing?
There was a problem hiding this comment.
@microbit-robert if other fields don't do it, then it's fine with me!
| const ty = this.scale * y * (FieldMatrix.CELL_WIDTH + FieldMatrix.CELL_VERTICAL_MARGIN) + FieldMatrix.CELL_VERTICAL_MARGIN; | ||
|
|
||
| const cellG = pxsim.svg.child(this.elt, "g", { transform: `translate(${tx} ${ty})` }) as SVGGElement; | ||
| const cellG = pxsim.svg.child(row, "g", { transform: `translate(${tx} ${ty})`, 'role': 'gridcell' }); |
There was a problem hiding this comment.
does this need some sort of disabled aria indicator if the workspace is readonly? you can test a readonly workspace by turning on the debugger
There was a problem hiding this comment.
Ideally, the cellRect one line below with role="switch" should have an aria-readonly attribute that gets updated based on the workspace being readonly. It looks like this attribute should be updated in updateEditable(?), however, this method isn't called when toggling the debugger, though it is called when Blockly is initialized. The difference in the cursor appearance can be noted when hovering over an LED in debugging mode vs a program loaded into the Teacher Tool, for example.
We thought it better not to include aria-readonly if we can't guarantee its value is going to be correct. We would welcome your input on this.
In practice, we think this does no harm, as the field editor isn't focusable in readonly mode, including in the debugger.
2 participants
This change is compatible with the current keyboard navigation experiment and the new work-in-progress one.
To test it with the current experience having created a showLeds block:
When the new experiment is integrated we'll remove the WASD support.
This field is unusual in that it is immediately editable with a stub showEditor implementation.
Detail:
Controls - see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/grid_role:
We expect to use similar navigation for other 2D structures.
This change also widens the showLeds block (and other blocks that use this field) slightly to include margin after the last column of LEDs. This centers the entire LED matrix and was required to correctly position the Blockly cursor outline.
@microbit-matt-hillsdon @riknoll