From 3806982030a6014d7094c5454f8bed480d466b7b Mon Sep 17 00:00:00 2001
From: Matt Hillsdon <matt.hillsdon@microbit.org>
Date: Mon, 31 Mar 2025 18:01:20 +0100
Subject: [PATCH] feat: allow for toolbox with multiple tab stops

- Switch to focusin/out for toolbox (maybe in future elsewhere)
- Drop the tab index fiddling as it's not practical to get it right in the face
  of more than one tab stop in the toolbox, especially if that toolbox has a
  DOM structure not known to the plugin.

  Given that we're reinstating the ability to shift-tab flyout->toolbox it
  makes sense to allow tab to the flyout. Shift-tab to the flyout remains not
  possible in the auto-hide case but that's not unreasonable.

This version can be made to work with MakeCode without further modifications.
---
 src/index.ts      | 39 +++++++++++++++++++++++++++------------
 src/navigation.ts | 12 ------------
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/src/index.ts b/src/index.ts
index b0060689..53e3019b 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -31,10 +31,10 @@ export class KeyboardNavigation {
   private blurListener: (e: Event) => void;
 
   /** Event handler run when the toolbox gains focus. */
-  private toolboxFocusListener: () => void;
+  private toolboxFocusInListener: (e: Event) => void;
 
   /** Event handler run when the toolbox loses focus. */
-  private toolboxBlurListener: (e: Event) => void;
+  private toolboxFocusOutListener: (e: Event) => void;
 
   /** Event handler run when the flyout gains focus. */
   private flyoutFocusListener: () => void;
@@ -99,10 +99,6 @@ export class KeyboardNavigation {
       flyoutElement,
       workspace.getParentSvg(),
     );
-    // Allow tab to the flyout only when there's no toolbox.
-    if (workspace.getToolbox() && flyoutElement) {
-      flyoutElement.tabIndex = -1;
-    }
 
     this.focusListener = (e: Event) => {
       if (e.currentTarget === this.workspace.getParentSvg()) {
@@ -143,10 +139,26 @@ export class KeyboardNavigation {
     workspace.getSvgGroup().addEventListener('blur', this.blurListener);
 
     const toolboxElement = getToolboxElement(workspace);
-    this.toolboxFocusListener = () => {
+    this.toolboxFocusInListener = (e: Event) => {
+      if (
+        (e.currentTarget as Element).contains(
+          (e as FocusEvent).relatedTarget as Node,
+        )
+      ) {
+        return;
+      }
+
       this.navigationController.handleFocusToolbox(workspace);
     };
-    this.toolboxBlurListener = (e: Event) => {
+    this.toolboxFocusOutListener = (e: Event) => {
+      if (
+        (e.currentTarget as Element).contains(
+          (e as FocusEvent).relatedTarget as Node,
+        )
+      ) {
+        return;
+      }
+
       this.navigationController.handleBlurToolbox(
         workspace,
         this.shouldCloseFlyoutOnBlur(
@@ -155,8 +167,8 @@ export class KeyboardNavigation {
         ),
       );
     };
-    toolboxElement?.addEventListener('focus', this.toolboxFocusListener);
-    toolboxElement?.addEventListener('blur', this.toolboxBlurListener);
+    toolboxElement?.addEventListener('focusin', this.toolboxFocusInListener);
+    toolboxElement?.addEventListener('focusout', this.toolboxFocusOutListener);
 
     this.flyoutFocusListener = () => {
       this.navigationController.handleFocusFlyout(workspace);
@@ -198,8 +210,11 @@ export class KeyboardNavigation {
       .removeEventListener('focus', this.focusListener);
 
     const toolboxElement = getToolboxElement(this.workspace);
-    toolboxElement?.removeEventListener('focus', this.toolboxFocusListener);
-    toolboxElement?.removeEventListener('blur', this.toolboxBlurListener);
+    toolboxElement?.removeEventListener('focusin', this.toolboxFocusInListener);
+    toolboxElement?.removeEventListener(
+      'focusout',
+      this.toolboxFocusOutListener,
+    );
 
     const flyoutElement = getFlyoutElement(this.workspace);
     flyoutElement?.removeEventListener('focus', this.flyoutFocusListener);
diff --git a/src/navigation.ts b/src/navigation.ts
index 5d97409a..a2643d69 100644
--- a/src/navigation.ts
+++ b/src/navigation.ts
@@ -503,12 +503,6 @@ export class Navigation {
     this.setState(workspace, Constants.STATE.FLYOUT);
     this.getFlyoutCursor(workspace)?.draw();
     this.resetFlyoutCursorPosition(workspace);
-
-    // Prevent shift-tab to the toolbox while the flyout has focus.
-    const toolboxElement = getToolboxElement(workspace);
-    if (toolboxElement) {
-      toolboxElement.tabIndex = -1;
-    }
   }
 
   /**
@@ -524,12 +518,6 @@ export class Navigation {
       workspace.hideChaff();
     }
     this.getFlyoutCursor(workspace)?.hide();
-
-    // Reinstate tab to toolbox.
-    const toolboxElement = getToolboxElement(workspace);
-    if (toolboxElement) {
-      toolboxElement.tabIndex = 0;
-    }
   }
 
   /**