Make flyout a menu (diff analysis)

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();
   }
 
   /**

core/field_checkbox.ts

@@ -122,13 +122,18 @@ export class FieldCheckbox extends Field<CheckboxBool> {
 
   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_() {

core/field_dropdown.ts

@@ -208,17 +208,21 @@ export class FieldDropdown extends Field<string> {
 
   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));
   }
 
   /**

core/field_image.ts

@@ -159,13 +159,14 @@ export class FieldImage extends Field<string> {
       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);
     }
   }

core/field_input.ts

@@ -178,8 +178,6 @@ export abstract class FieldInput<T extends InputTypes> 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<T extends InputTypes> 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 {

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;
+    });
   }
 
   /**

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');

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.

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 {

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(

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();