diff --git a/core/block_svg.ts b/core/block_svg.ts index a865593a2ff..ae8391963c8 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -234,67 +234,19 @@ export class BlockSvg * @internal */ recomputeAriaLabel() { - if (this.initialized) { - const childElemIds: string[] = []; - for (const input of this.inputList) { - const connection = input.connection as RenderedConnection | null; - if (input.isVisible() && connection) { - if (connection.type === ConnectionType.NEXT_STATEMENT) { - let currentBlock: BlockSvg | null = connection.targetBlock(); - while (currentBlock) { - if (currentBlock.canBeFocused()) { - childElemIds.push(currentBlock.getBlockSvgFocusElem().id); - } - currentBlock = currentBlock.getNextBlock(); - } - } else if (connection.type === ConnectionType.INPUT_VALUE) { - const inpBlock = connection.targetBlock() as BlockSvg | null; - if (inpBlock && inpBlock.canBeFocused()) { - childElemIds.push(inpBlock.getBlockSvgFocusElem().id); - } - if (connection.canBeFocused()) { - childElemIds.push(connection.getFocusableElement().id); - } - } - } - for (const field of input.fieldRow) { - if (field.getSvgRoot() && field.canBeFocused()) { - // Only track the field if it's been initialized. - childElemIds.push(field.getFocusableElement().id); - } - } - for (const icon of this.icons) { - if (icon.canBeFocused()) { - childElemIds.push(icon.getFocusableElement().id); - } - } - } - - const nextConnection = this.nextConnection as RenderedConnection | null; - if ( - nextConnection && - nextConnection.canBeFocused() && - nextConnection.type === ConnectionType.NEXT_STATEMENT - ) { - childElemIds.push(nextConnection.getFocusableElement().id); - } - - aria.setState(this.getBlockSvgFocusElem(), aria.State.OWNS, childElemIds); - } - if (this.isSimpleReporter(true, true)) return; aria.setState( this.getFocusableElement(), aria.State.LABEL, - this.computeAriaLabel(), + !this.isInFlyout + ? this.computeAriaLabel() + : this.computeAriaLabelForFlyoutBlock(), ); } - private getBlockSvgFocusElem(): Element { - // Note that this deviates from getFocusableElement() to ensure that - // single field blocks are properly set up in the hierarchy. - return this.pathObject.svgPath; + private computeAriaLabelForFlyoutBlock(): string { + return `${this.computeAriaLabel(true)}, block`; } computeAriaLabel( @@ -359,7 +311,7 @@ export class BlockSvg private computeAriaRole() { if (this.workspace.isFlyout) { - aria.setRole(this.pathObject.svgPath, aria.Role.TREEITEM); + aria.setRole(this.pathObject.svgPath, aria.Role.MENUITEM); } else { const roleDescription = this.getAriaRoleDescription(); aria.setState( @@ -413,7 +365,6 @@ export class BlockSvg this.workspace.getCanvas().appendChild(svg); } this.initialized = true; - this.recomputeAriaLabel(); } /** @@ -518,12 +469,6 @@ export class BlockSvg this.applyColour(); this.workspace.recomputeAriaTree(); - this.recomputeAriaLabelRecursive(); - } - - private recomputeAriaLabelRecursive() { - this.recomputeAriaLabel(); - this.parentBlock_?.recomputeAriaLabelRecursive(); } /** diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index aecead2e80c..a0a3804535a 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -122,13 +122,18 @@ export class FieldCheckbox extends Field { private recomputeAria() { const element = this.getFocusableElement(); - aria.setRole(element, aria.Role.CHECKBOX); - aria.setState( - element, - aria.State.LABEL, - this.getAriaTypeName() ?? 'Checkbox', - ); - aria.setState(element, aria.State.CHECKED, !!this.value_); + const isInFlyout = this.getSourceBlock()?.workspace?.isFlyout || false; + if (!isInFlyout) { + aria.setRole(element, aria.Role.CHECKBOX); + aria.setState( + element, + aria.State.LABEL, + this.getAriaTypeName() ?? 'Checkbox', + ); + aria.setState(element, aria.State.CHECKED, !!this.value_); + } else { + aria.setState(element, aria.State.HIDDEN, true); + } } override render_() { diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index de0955b9331..be338108210 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -208,17 +208,21 @@ export class FieldDropdown extends Field { protected recomputeAria() { if (!this.fieldGroup_) return; // There's no element to set currently. + const isInFlyout = this.getSourceBlock()?.workspace?.isFlyout || false; const element = this.getFocusableElement(); - aria.setRole(element, aria.Role.COMBOBOX); - aria.setState(element, aria.State.HASPOPUP, aria.Role.LISTBOX); - aria.setState(element, aria.State.EXPANDED, !!this.menu_); - if (this.menu_) { - aria.setState(element, aria.State.CONTROLS, this.menu_.id); + if (!isInFlyout) { + aria.setRole(element, aria.Role.COMBOBOX); + aria.setState(element, aria.State.HASPOPUP, aria.Role.LISTBOX); + aria.setState(element, aria.State.EXPANDED, !!this.menu_); + if (this.menu_) { + aria.setState(element, aria.State.CONTROLS, this.menu_.id); + } else { + aria.clearState(element, aria.State.CONTROLS); + } + aria.setState(element, aria.State.LABEL, super.computeAriaLabel(true)); } else { - aria.clearState(element, aria.State.CONTROLS); + aria.setState(element, aria.State.HIDDEN, true); } - - aria.setState(element, aria.State.LABEL, super.computeAriaLabel(true)); } /** diff --git a/core/field_image.ts b/core/field_image.ts index b7aaf5e06bf..bfa19816eab 100644 --- a/core/field_image.ts +++ b/core/field_image.ts @@ -159,13 +159,14 @@ export class FieldImage extends Field { dom.addClass(this.fieldGroup_, 'blocklyImageField'); } + const isInFlyout = this.getSourceBlock()?.workspace?.isFlyout || false; const element = this.getFocusableElement(); - if (this.isClickable()) { + if (!isInFlyout && this.isClickable()) { this.imageElement.style.cursor = 'pointer'; aria.setRole(element, aria.Role.BUTTON); aria.setState(element, aria.State.LABEL, super.computeAriaLabel(true)); } else { - // The field isn't navigable unless it's clickable. + // The field isn't navigable unless it's clickable and outside the flyout. aria.setRole(element, aria.Role.PRESENTATION); } } diff --git a/core/field_input.ts b/core/field_input.ts index 7132d9ab16a..175e80ff55d 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -178,8 +178,6 @@ export abstract class FieldInput extends Field< dom.addClass(this.fieldGroup_, 'blocklyInputField'); } - const element = this.getFocusableElement(); - aria.setRole(element, aria.Role.BUTTON); this.recomputeAriaLabel(); } @@ -189,7 +187,13 @@ export abstract class FieldInput extends Field< protected recomputeAriaLabel() { if (!this.fieldGroup_) return; const element = this.getFocusableElement(); - aria.setState(element, aria.State.LABEL, super.computeAriaLabel()); + const isInFlyout = this.getSourceBlock()?.workspace?.isFlyout || false; + if (!isInFlyout) { + aria.setRole(element, aria.Role.BUTTON); + aria.setState(element, aria.State.LABEL, super.computeAriaLabel()); + } else { + aria.setState(element, aria.State.HIDDEN, true); + } } override isFullBlockField(): boolean { diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 8d4264f4608..d961f31b16e 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -708,7 +708,11 @@ export abstract class Flyout } } - return this.normalizeSeparators(contents); + // TODO: Optimize this. + return this.normalizeSeparators(contents).map((item) => { + // aria.setRole(item.getElement().getFocusableElement(), aria.Role.MENUITEM); + return item; + }); } /** diff --git a/core/flyout_button.ts b/core/flyout_button.ts index 74de275a459..9d8f1d6f95e 100644 --- a/core/flyout_button.ts +++ b/core/flyout_button.ts @@ -132,12 +132,13 @@ export class FlyoutButton this.svgContainerGroup, ); - aria.setRole(this.svgContainerGroup, aria.Role.TREEITEM); if (this.isFlyoutLabel) { + aria.setRole(this.svgContainerGroup, aria.Role.MENUITEM); aria.setRole(this.svgContentGroup, aria.Role.PRESENTATION); this.svgFocusableGroup = this.svgContainerGroup; } else { - aria.setRole(this.svgContentGroup, aria.Role.BUTTON); + aria.setRole(this.svgContainerGroup, aria.Role.PRESENTATION); + aria.setRole(this.svgContentGroup, aria.Role.MENUITEM); this.svgFocusableGroup = this.svgContentGroup; } this.svgFocusableGroup.id = this.id; @@ -183,9 +184,7 @@ export class FlyoutButton }, this.svgContentGroup, ); - if (!this.isFlyoutLabel) { - aria.setRole(svgText, aria.Role.PRESENTATION); - } + aria.setRole(svgText, aria.Role.PRESENTATION); let text = parsing.replaceMessageReferences(this.text); if (this.workspace.RTL) { // Force text to be RTL by adding an RLM. @@ -198,7 +197,15 @@ export class FlyoutButton .getThemeManager() .subscribe(this.svgText, 'flyoutForegroundColour', 'fill'); } - aria.setState(this.svgFocusableGroup, aria.State.LABEL, text); + if (this.isFlyoutLabel) { + aria.setState(this.svgFocusableGroup, aria.State.LABEL, text); + } else { + aria.setState( + this.svgFocusableGroup, + aria.State.LABEL, + `${text}, button`, + ); + } const fontSize = style.getComputedStyle(svgText, 'fontSize'); const fontWeight = style.getComputedStyle(svgText, 'fontWeight'); diff --git a/core/flyout_item.ts b/core/flyout_item.ts index 26be0ed12e2..f7e51877fb7 100644 --- a/core/flyout_item.ts +++ b/core/flyout_item.ts @@ -8,6 +8,11 @@ export class FlyoutItem { /** * Creates a new FlyoutItem. * + * Note that it's the responsibility of implementations to ensure that element + * is given the ARIA role MENUITEM and respects its expected constraints + * (which includes ensuring that no interactive elements are children of the + * item element--interactive elements themselves should be the MENUITEM). + * * @param element The element that will be displayed in the flyout. * @param type The type of element. Should correspond to the type of the * flyout inflater that created this object. diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index ffef106f579..f824cdb38f9 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -688,7 +688,7 @@ export class RenderedConnection /** See IFocusableNode.canBeFocused. */ canBeFocused(): boolean { - return this.findHighlightSvg() !== null; + return true; } private findHighlightSvg(): SVGPathElement | null { diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 1f16f24c6e0..ab24c103bb7 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -804,8 +804,8 @@ export class WorkspaceSvg this.svgBubbleCanvas_ = this.layerManager.getBubbleLayer(); if (this.isFlyout) { - // Use the block canvas as the primary tree parent for flyout blocks. - aria.setRole(this.svgBlockCanvas_, aria.Role.TREE); + // Use the block canvas as the primary menu for nesting. + aria.setRole(this.svgBlockCanvas_, aria.Role.MENU); aria.setState(this.svgBlockCanvas_, aria.State.LABEL, ariaLabel); } else { browserEvents.conditionalBind( diff --git a/tests/mocha/cursor_test.js b/tests/mocha/cursor_test.js index 6816cf8c1e7..2273ec4b381 100644 --- a/tests/mocha/cursor_test.js +++ b/tests/mocha/cursor_test.js @@ -342,7 +342,7 @@ suite('Cursor', function () { }); suite('one empty block', function () { setup(function () { - this.blockA = createRenderedBlock(this.workspace, 'empty_block'); + this.blockA = this.workspace.newBlock('empty_block'); }); teardown(function () { this.workspace.clear(); @@ -359,7 +359,7 @@ suite('Cursor', function () { suite('one stack block', function () { setup(function () { - this.blockA = createRenderedBlock(this.workspace, 'stack_block'); + this.blockA = this.workspace.newBlock('stack_block'); }); teardown(function () { this.workspace.clear(); @@ -376,7 +376,7 @@ suite('Cursor', function () { suite('one row block', function () { setup(function () { - this.blockA = createRenderedBlock(this.workspace, 'row_block'); + this.blockA = this.workspace.newBlock('row_block'); }); teardown(function () { this.workspace.clear(); @@ -392,7 +392,7 @@ suite('Cursor', function () { }); suite('one c-hat block', function () { setup(function () { - this.blockA = createRenderedBlock(this.workspace, 'c_hat_block'); + this.blockA = this.workspace.newBlock('c_hat_block'); }); teardown(function () { this.workspace.clear(); diff --git a/tests/mocha/navigation_test.js b/tests/mocha/navigation_test.js index 37972318d50..3a9292b9209 100644 --- a/tests/mocha/navigation_test.js +++ b/tests/mocha/navigation_test.js @@ -5,10 +5,10 @@ */ import {assert} from '../../node_modules/chai/index.js'; -import {createRenderedBlock} from './test_helpers/block_definitions.js'; import { sharedTestSetup, sharedTestTeardown, + workspaceTeardown, } from './test_helpers/setup_teardown.js'; suite('Navigation', function () { @@ -89,28 +89,13 @@ suite('Navigation', function () { ]); this.workspace = Blockly.inject('blocklyDiv', {}); this.navigator = this.workspace.getNavigator(); - const statementInput1 = createRenderedBlock( - this.workspace, - 'input_statement', - ); - const statementInput2 = createRenderedBlock( - this.workspace, - 'input_statement', - ); - const statementInput3 = createRenderedBlock( - this.workspace, - 'input_statement', - ); - const statementInput4 = createRenderedBlock( - this.workspace, - 'input_statement', - ); - const fieldWithOutput = createRenderedBlock(this.workspace, 'field_input'); - const doubleValueInput = createRenderedBlock( - this.workspace, - 'double_value_input', - ); - const valueInput = createRenderedBlock(this.workspace, 'value_input'); + const statementInput1 = this.workspace.newBlock('input_statement'); + const statementInput2 = this.workspace.newBlock('input_statement'); + const statementInput3 = this.workspace.newBlock('input_statement'); + const statementInput4 = this.workspace.newBlock('input_statement'); + const fieldWithOutput = this.workspace.newBlock('field_input'); + const doubleValueInput = this.workspace.newBlock('double_value_input'); + const valueInput = this.workspace.newBlock('value_input'); statementInput1.nextConnection.connect(statementInput2.previousConnection); statementInput1.inputList[0].connection.connect( @@ -370,25 +355,13 @@ suite('Navigation', function () { 'helpUrl': '', }, ]); - const noNextConnection = createRenderedBlock( - this.workspace, - 'top_connection', - ); - const fieldAndInputs = createRenderedBlock( - this.workspace, - 'fields_and_input', - ); - const twoFields = createRenderedBlock(this.workspace, 'two_fields'); - const fieldAndInputs2 = createRenderedBlock( - this.workspace, - 'fields_and_input2', - ); - const noPrevConnection = createRenderedBlock( - this.workspace, - 'start_block', - ); - const hiddenField = createRenderedBlock(this.workspace, 'hidden_field'); - const hiddenInput = createRenderedBlock(this.workspace, 'hidden_input'); + const noNextConnection = this.workspace.newBlock('top_connection'); + const fieldAndInputs = this.workspace.newBlock('fields_and_input'); + const twoFields = this.workspace.newBlock('two_fields'); + const fieldAndInputs2 = this.workspace.newBlock('fields_and_input2'); + const noPrevConnection = this.workspace.newBlock('start_block'); + const hiddenField = this.workspace.newBlock('hidden_field'); + const hiddenInput = this.workspace.newBlock('hidden_input'); this.blocks.noNextConnection = noNextConnection; this.blocks.fieldAndInputs = fieldAndInputs; this.blocks.twoFields = twoFields; @@ -400,47 +373,28 @@ suite('Navigation', function () { hiddenField.inputList[0].fieldRow[1].setVisible(false); hiddenInput.inputList[1].setVisible(false); - const dummyInput = createRenderedBlock(this.workspace, 'dummy_input'); - const dummyInputValue = createRenderedBlock( - this.workspace, - 'dummy_inputValue', - ); - const fieldWithOutput2 = createRenderedBlock( - this.workspace, - 'field_input', - ); + const dummyInput = this.workspace.newBlock('dummy_input'); + const dummyInputValue = this.workspace.newBlock('dummy_inputValue'); + const fieldWithOutput2 = this.workspace.newBlock('field_input'); this.blocks.dummyInput = dummyInput; this.blocks.dummyInputValue = dummyInputValue; this.blocks.fieldWithOutput2 = fieldWithOutput2; - const secondBlock = createRenderedBlock( - this.workspace, - 'input_statement', - ); - const outputNextBlock = createRenderedBlock( - this.workspace, - 'output_next', - ); + const secondBlock = this.workspace.newBlock('input_statement'); + const outputNextBlock = this.workspace.newBlock('output_next'); this.blocks.secondBlock = secondBlock; this.blocks.outputNextBlock = outputNextBlock; - const buttonBlock = createRenderedBlock( - this.workspace, - 'buttons', - 'button_block', - ); - const buttonInput1 = createRenderedBlock( - this.workspace, + const buttonBlock = this.workspace.newBlock('buttons', 'button_block'); + const buttonInput1 = this.workspace.newBlock( 'field_input', 'button_input1', ); - const buttonInput2 = createRenderedBlock( - this.workspace, + const buttonInput2 = this.workspace.newBlock( 'field_input', 'button_input2', ); - const buttonNext = createRenderedBlock( - this.workspace, + const buttonNext = this.workspace.newBlock( 'input_statement', 'button_next', ); @@ -466,6 +420,15 @@ suite('Navigation', function () { this.workspace.cleanUp(); }); suite('Next', function () { + setup(function () { + this.singleBlockWorkspace = new Blockly.Workspace(); + const singleBlock = this.singleBlockWorkspace.newBlock('two_fields'); + this.blocks.singleBlock = singleBlock; + }); + teardown(function () { + workspaceTeardown.call(this, this.singleBlockWorkspace); + }); + test('fromPreviousToBlock', function () { const prevConnection = this.blocks.statementInput1.previousConnection; const nextNode = this.navigator.getNextSibling(prevConnection); @@ -508,6 +471,8 @@ suite('Navigation', function () { }); test('fromFieldToNestedBlock', function () { const field = this.blocks.statementInput1.inputList[0].fieldRow[1]; + const inputConnection = + this.blocks.statementInput1.inputList[0].connection; const nextNode = this.navigator.getNextSibling(field); assert.equal(nextNode, this.blocks.fieldWithOutput); }); @@ -611,6 +576,7 @@ suite('Navigation', function () { const prevNode = this.navigator.getPreviousSibling( this.blocks.fieldWithOutput, ); + const outputConnection = this.blocks.fieldWithOutput.outputConnection; assert.equal(prevNode, [...this.blocks.statementInput1.getFields()][1]); }); test('fromNextToBlock', function () { @@ -651,12 +617,14 @@ suite('Navigation', function () { assert.isNull(prevNode); }); test('fromFieldToInput', function () { - const outputBlock = createRenderedBlock(this.workspace, 'field_input'); + const outputBlock = this.workspace.newBlock('field_input'); this.blocks.fieldAndInputs2.inputList[0].connection.connect( outputBlock.outputConnection, ); const field = this.blocks.fieldAndInputs2.inputList[1].fieldRow[0]; + const inputConnection = + this.blocks.fieldAndInputs2.inputList[0].connection; const prevNode = this.navigator.getPreviousSibling(field); assert.equal(prevNode, outputBlock); }); @@ -733,13 +701,18 @@ suite('Navigation', function () { }); suite('In', function () { + setup(function () { + this.emptyWorkspace = Blockly.inject(document.createElement('div'), {}); + }); + teardown(function () { + workspaceTeardown.call(this, this.emptyWorkspace); + }); + test('fromInputToOutput', function () { - // The first child is the connected block since the connection itself - // cannot be navigated to directly. const input = this.blocks.statementInput1.inputList[0]; const inNode = this.navigator.getFirstChild(input.connection); - const connectedBlock = this.blocks.fieldWithOutput; - assert.equal(inNode, connectedBlock); + const outputConnection = this.blocks.fieldWithOutput.outputConnection; + assert.equal(inNode, outputConnection); }); test('fromInputToNull', function () { const input = this.blocks.statementInput2.inputList[0]; diff --git a/tests/mocha/test_helpers/block_definitions.js b/tests/mocha/test_helpers/block_definitions.js index e5ca106d2c4..26507b29cb8 100644 --- a/tests/mocha/test_helpers/block_definitions.js +++ b/tests/mocha/test_helpers/block_definitions.js @@ -196,8 +196,8 @@ export function createTestBlock() { }; } -export function createRenderedBlock(workspaceSvg, type, opt_id) { - const block = workspaceSvg.newBlock(type, opt_id); +export function createRenderedBlock(workspaceSvg, type) { + const block = workspaceSvg.newBlock(type); block.initSvg(); block.render(); return block;