From 54eeb85d8947d5dfe487d2b1ce810eb34c2c4a4f Mon Sep 17 00:00:00 2001
From: Maribeth Bottorff <maribethb@google.com>
Date: Wed, 8 May 2024 11:44:00 -0700
Subject: [PATCH] fix!: add getContents to IFlyout (#8064)

---
 core/flyout_base.ts           | 11 +------
 core/interfaces/i_flyout.ts   | 11 +++++++
 core/keyboard_nav/ast_node.ts | 60 ++++++++++++++++++++++++++++-------
 3 files changed, 61 insertions(+), 21 deletions(-)

diff --git a/core/flyout_base.ts b/core/flyout_base.ts
index 58b748433..6d8760b7f 100644
--- a/core/flyout_base.ts
+++ b/core/flyout_base.ts
@@ -566,16 +566,7 @@ export abstract class Flyout
    * @param contents - The array of items for the flyout.
    */
   setContents(contents: FlyoutItem[]): void {
-    const blocksAndButtons = contents.map((item) => {
-      if (item.type === 'block' && item.block) {
-        return item.block as BlockSvg;
-      }
-      if (item.type === 'button' && item.button) {
-        return item.button as FlyoutButton;
-      }
-    });
-
-    this.contents = blocksAndButtons as FlyoutItem[];
+    this.contents = contents;
   }
   /**
    * Update the display property of the flyout based whether it thinks it should
diff --git a/core/interfaces/i_flyout.ts b/core/interfaces/i_flyout.ts
index e73845dc7..84067f755 100644
--- a/core/interfaces/i_flyout.ts
+++ b/core/interfaces/i_flyout.ts
@@ -12,6 +12,7 @@ import type {Coordinate} from '../utils/coordinate.js';
 import type {FlyoutDefinition} from '../utils/toolbox.js';
 import type {Svg} from '../utils/svg.js';
 import type {IRegistrable} from './i_registrable.js';
+import {FlyoutItem} from '../flyout_base.js';
 
 /**
  * Interface for a flyout.
@@ -117,6 +118,16 @@ export interface IFlyout extends IRegistrable {
    */
   show(flyoutDef: FlyoutDefinition | string): void;
 
+  /**
+   * Returns the list of flyout items currently present in the flyout.
+   * The `show` method parses the flyout definition into a list of actual
+   * flyout items. This method should return those concrete items, which
+   * may be used for e.g. keyboard navigation.
+   *
+   * @returns List of flyout items.
+   */
+  getContents(): FlyoutItem[];
+
   /**
    * Create a copy of this block on the workspace.
    *
diff --git a/core/keyboard_nav/ast_node.ts b/core/keyboard_nav/ast_node.ts
index fb20539be..7985ac6dc 100644
--- a/core/keyboard_nav/ast_node.ts
+++ b/core/keyboard_nav/ast_node.ts
@@ -12,7 +12,7 @@
  */
 // Former goog.module ID: Blockly.ASTNode
 
-import type {Block} from '../block.js';
+import {Block} from '../block.js';
 import type {Connection} from '../connection.js';
 import {ConnectionType} from '../connection_type.js';
 import type {Field} from '../field.js';
@@ -23,7 +23,7 @@ import {Coordinate} from '../utils/coordinate.js';
 import type {Workspace} from '../workspace.js';
 import {FlyoutButton} from '../flyout_button.js';
 import {WorkspaceSvg} from '../workspace_svg.js';
-import {Flyout} from '../flyout_base.js';
+import {FlyoutItem} from '../flyout_base.js';
 
 /**
  * Class for an AST node.
@@ -337,21 +337,59 @@ export class ASTNode {
         return null;
     }
 
-    const flyout = targetWorkspace.getFlyout() as Flyout;
-    const flyoutContents = flyout.getContents() as (Block | FlyoutButton)[];
+    const flyout = targetWorkspace.getFlyout();
+    if (!flyout) return null;
+
+    const nextItem = this.findNextLocationInFlyout(
+      flyout.getContents(),
+      location,
+      forward,
+    );
+    if (!nextItem) return null;
+
+    if (nextItem.type === 'button' && nextItem.button) {
+      return ASTNode.createButtonNode(nextItem.button);
+    } else if (nextItem.type === 'block' && nextItem.block) {
+      return ASTNode.createStackNode(nextItem.block);
+    }
+
+    return null;
+  }
+
+  /**
+   * Finds the next (or previous if navigating backward) item in the flyout that should be navigated to.
+   *
+   * @param flyoutContents Contents of the current flyout.
+   * @param currentLocation Current ASTNode location.
+   * @param forward True if we're navigating forward, else false.
+   * @returns The next (or previous) FlyoutItem, or null if there is none.
+   */
+  private findNextLocationInFlyout(
+    flyoutContents: FlyoutItem[],
+    currentLocation: IASTNodeLocation,
+    forward: boolean,
+  ): FlyoutItem | null {
+    const currentIndex = flyoutContents.findIndex((item: FlyoutItem) => {
+      if (currentLocation instanceof Block && item.block === currentLocation) {
+        return true;
+      }
+      if (
+        currentLocation instanceof FlyoutButton &&
+        item.button === currentLocation
+      ) {
+        return true;
+      }
+      return false;
+    });
+
+    if (currentIndex < 0) return null;
 
-    const currentIndex = flyoutContents.indexOf(location);
     const resultIndex = forward ? currentIndex + 1 : currentIndex - 1;
     if (resultIndex === -1 || resultIndex === flyoutContents.length) {
       return null;
     }
 
-    const newLocation = flyoutContents[resultIndex];
-    if (newLocation instanceof FlyoutButton) {
-      return ASTNode.createButtonNode(newLocation);
-    } else {
-      return ASTNode.createStackNode(newLocation);
-    }
+    return flyoutContents[resultIndex];
   }
 
   /**