Skip to content

Implement keyboard and screen reader accessibility for LED matrix field #10390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
riknoll merged 2 commits into from
Feb 19, 2025
Merged

Implement keyboard and screen reader accessibility for LED matrix field #10390

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b3d9de
Implement keyboard and screen reader accessibility for LED matrix field
microbit-robert Jan 29, 2025
09c91ce
Review feedback
microbit-robert Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 173 additions & 12 deletions pxtblocks/fields/field_ledmatrix.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
private elt: SVGSVGElement;

private currentDragState_: boolean;
private selected: number[] | undefined = undefined;

constructor(text: string, params: any, validator?: Blockly.FieldValidator) {
super(text, validator);
Expand Down Expand Up @@ -80,17 +81,121 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
this.scale = 0.9;
}

private keyHandler(e: KeyboardEvent) {
if (!this.selected) {
return
}
const [x, y] = this.selected;
const ctrlCmd = pxt.BrowserUtils.isMac() ? e.metaKey : e.ctrlKey;
switch(e.code) {
case "KeyW":
case "ArrowUp": {
if (y !== 0) {
this.selected = [x, y - 1]
}
break;
}
case "KeyS":
case "ArrowDown": {
if (y !== this.cells[0].length - 1) {
this.selected = [x, y + 1]
}
break;
}
case "KeyA":
case "ArrowLeft": {
if (x !== 0) {
this.selected = [x - 1, y]
} else if (y !== 0){
this.selected = [this.matrixWidth - 1, y - 1]
}
break;
}
case "KeyD":
case "ArrowRight": {
if (x !== this.cells.length - 1) {
this.selected = [x + 1, y]
} else if (y !== this.matrixHeight - 1) {
this.selected = [0, y + 1]
}
break;
}
case "Home": {
if (ctrlCmd) {
this.selected = [0, 0]
} else {
this.selected = [0, y]
}
break;
}
case "End": {
if (ctrlCmd) {
this.selected = [this.matrixWidth - 1, this.matrixHeight - 1]
} else {
this.selected = [this.matrixWidth - 1, y]
}
break;
}
case "Enter":
case "Space": {
this.toggleRect(x, y, !this.cellState[x][y]);
break;
}
case "Escape": {
(Blockly.getMainWorkspace() as Blockly.WorkspaceSvg).markFocused();
Copy link
Copy Markdown
Member

@riknoll riknoll Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be this.sourceBlock_.workspace?

Copy link
Copy Markdown
Contributor Author

@microbit-robert microbit-robert Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should! Changed here 09c91ce.

return;
}
default: {
return
}
}
const [newX, newY] = this.selected;
this.setFocusIndicator(this.cells[newX][newY], this.cellState[newX][newY]);
this.elt.setAttribute('aria-activedescendant', `${this.sourceBlock_.id}:${newX}${newY}`);
e.preventDefault();
e.stopPropagation();
}

private clearSelection() {
if (this.selected) {
this.setFocusIndicator();
this.selected = undefined;
}
this.elt.removeAttribute('aria-activedescendant');
}

private removeKeyboardFocusHandlers() {
this.elt.removeEventListener("keydown", this.keyHandler)
this.elt.removeEventListener("blur", this.blurHandler)
}

private blurHandler() {
this.removeKeyboardFocusHandlers();
this.clearSelection();
}

private setFocusIndicator(cell?: SVGRectElement, ledOn?: boolean) {
this.cells.forEach(cell => cell.forEach(cell => cell.nextElementSibling.firstElementChild.classList.remove("selectedLedOn", "selectedLedOff")));
if (cell) {
const className = ledOn ? "selectedLedOn" : "selectedLedOff"
cell.nextElementSibling.firstElementChild.classList.add(className);
}
}

/**
* Show the inline free-text editor on top of the text.
* @private
*/
showEditor_() {
// Intentionally left empty
this.selected = [0, 0];
this.setFocusIndicator(this.cells[0][0], this.cellState[0][0])
this.elt.setAttribute('aria-activedescendant', this.sourceBlock_.id + ":00");
this.elt.focus();
}

private initMatrix() {
if (!this.sourceBlock_.isInsertionMarker()) {
this.elt = pxsim.svg.parseString(`<svg xmlns="http://www.w3.org/2000/svg" id="field-matrix" />`);
this.elt = pxsim.svg.parseString(`<svg xmlns="http://www.w3.org/2000/svg" id="field-matrix" class="blocklyMatrix" tabindex="-1" role="grid" aria-label="${lf("LED grid")}" />`);

// Initialize the matrix that holds the state
for (let i = 0; i < this.matrixWidth; i++) {
Expand All @@ -104,9 +209,10 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
this.restoreStateFromString();

// Create the cells of the matrix that is displayed
for (let i = 0; i < this.matrixWidth; i++) {
for (let j = 0; j < this.matrixHeight; j++) {
this.createCell(i, j);
for (let y = 0; y < this.matrixHeight; y++) {
const row = this.createRow()
for (let x = 0; x < this.matrixWidth; x++) {
this.createCell(x, y, row);
}
}

Expand All @@ -132,6 +238,8 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
}

this.fieldGroup_.replaceChild(this.elt, this.fieldGroup_.firstChild);
this.elt.addEventListener("keydown", this.keyHandler.bind(this));
Copy link
Copy Markdown
Member

@riknoll riknoll Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@microbit-robert microbit-robert Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

@riknoll riknoll Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@microbit-robert if other fields don't do it, then it's fine with me!

this.elt.addEventListener("blur", this.blurHandler.bind(this));
}
}

Expand Down Expand Up @@ -179,20 +287,42 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
super.updateEditable();
}

private createCell(x: number, y: number) {
private createRow() {
return pxsim.svg.child(this.elt, "g", { 'role': 'row' });
}

private createCell(x: number, y: number, row: SVGElement) {
const tx = this.scale * x * (FieldMatrix.CELL_WIDTH + FieldMatrix.CELL_HORIZONTAL_MARGIN) + FieldMatrix.CELL_HORIZONTAL_MARGIN + this.getYAxisWidth();
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' });
Copy link
Copy Markdown
Member

@riknoll riknoll Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@microbit-robert microbit-robert Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const cellRect = pxsim.svg.child(cellG, "rect", {
'id': `${this.sourceBlock_.id}:${x}${y}`, // For aria-activedescendant
'class': `blocklyLed${this.cellState[x][y] ? 'On' : 'Off'}`,
'aria-label': lf("LED"),
'role': 'switch',
'aria-checked': this.cellState[x][y].toString(),
width: this.scale * FieldMatrix.CELL_WIDTH, height: this.scale * FieldMatrix.CELL_WIDTH,
fill: this.getColor(x, y),
'data-x': x,
'data-y': y,
rx: Math.max(2, this.scale * FieldMatrix.CELL_CORNER_RADIUS) }) as SVGRectElement;
this.cells[x][y] = cellRect;

// Borders and box-shadow do not work in this context and outlines do not follow border-radius.
// Stroke is harder to manage given the difference in stroke for an LED when it is on vs off.
// This foreignObject/div is used to create a focus indicator for the LED when selected via keyboard navigation.
const foreignObject = pxsim.svg.child(cellG, "foreignObject", {
transform: 'translate(-4, -4)',
width: this.scale * FieldMatrix.CELL_WIDTH + 8,
height: this.scale * FieldMatrix.CELL_WIDTH + 8,
});
foreignObject.style.pointerEvents = "none";
const div = document.createElement("div");
div.classList.add("blocklyLedFocusIndicator");
div.style.borderRadius = `${Math.max(2, this.scale * FieldMatrix.CELL_CORNER_RADIUS)}px`;
foreignObject.append(div);

if ((this.sourceBlock_.workspace as any).isFlyout) return;

pxsim.pointerEvents.down.forEach(evid => cellRect.addEventListener(evid, (ev: MouseEvent) => {
Expand All @@ -217,11 +347,14 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {

ev.stopPropagation();
ev.preventDefault();
// Clear event listeners and selection used for keyboard navigation.
this.removeKeyboardFocusHandlers();
this.clearSelection();
}, false));
}

private toggleRect = (x: number, y: number) => {
this.cellState[x][y] = this.currentDragState_;
private toggleRect = (x: number, y: number, value?: boolean) => {
this.cellState[x][y] = value ?? this.currentDragState_;
this.updateValue();
}

Expand Down Expand Up @@ -262,6 +395,7 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
cellRect.setAttribute("fill", this.getColor(x, y));
cellRect.setAttribute("fill-opacity", this.getOpacity(x, y));
cellRect.setAttribute('class', `blocklyLed${this.cellState[x][y] ? 'On' : 'Off'}`);
cellRect.setAttribute("aria-checked", this.cellState[x][y].toString());
}

setValue(newValue: string | number, restoreState = true) {
Expand All @@ -287,10 +421,9 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom {
this.initMatrix();
}


// 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();
Copy link
Copy Markdown
Contributor Author

@microbit-robert microbit-robert Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds trailing margin to center the field matrix LEDs (more obvious with the Blockly cursor present), which makes this block slightly wider.

Before:
image

After:
image

}

// The return value of this function is inserted in the code
Expand Down Expand Up @@ -365,4 +498,32 @@ function removeQuotes(str: string) {
return str.substr(1, str.length - 2).trim();
}
return str;
}
}

Blockly.Css.register(`
.blocklyMatrix:focus-visible {
outline: none;
}

.blocklyMatrix .blocklyLedFocusIndicator {
border: 4px solid transparent;
height: 100%;
}

.blocklyMatrix .blocklyLedFocusIndicator.selectedLedOn,
.blocklyMatrix .blocklyLedFocusIndicator.selectedLedOff {
border-color: white;
transform: translateZ(0);
}

.blocklyMatrix .blocklyLedFocusIndicator.selectedLedOn:after {
content: "";
position: absolute;
top: -2px;
left: -2px;
right: -2px;
bottom: -2px;
border: 2px solid black;
border-radius: inherit;
}
`)
4 changes: 4 additions & 0 deletions pxtblocks/plugins/renderer/css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ Blockly.Css.register(`
.blocklyDropdownMenu .blocklyMenuItemCheckbox.goog-menuitem-checkbox {
filter: contrast(0) brightness(100);
}

.blocklyVerticalMarker {
fill: none;
}
Comment on lines +8 to +10
Copy link
Copy Markdown
Contributor Author

@microbit-robert microbit-robert Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
image

`)