fix: Fix bug that prevented using keyboard shortcuts when the DropDownDiv is open. (#9085)

* fix: Fix bug that prevented using keyboard shortcuts when the DropDownDiv is open.

* chore: Remove obsolete comment.

* Refactor: Remove unreachable null check.

* chore: Add tests for handling Escape to dismiss the Widget/DropDownDivs.

* chore: Satisfy the linter.

* fix: Fix post-merge test failure.

Author: gonfunko

core/common.ts

@@ -8,11 +8,13 @@
 
 import type {Block} from './block.js';
 import {BlockDefinition, Blocks} from './blocks.js';
+import * as browserEvents from './browser_events.js';
 import type {Connection} from './connection.js';
 import {EventType} from './events/type.js';
 import * as eventUtils from './events/utils.js';
 import {getFocusManager} from './focus_manager.js';
 import {ISelectable, isSelectable} from './interfaces/i_selectable.js';
+import {ShortcutRegistry} from './shortcut_registry.js';
 import type {Workspace} from './workspace.js';
 import type {WorkspaceSvg} from './workspace_svg.js';
 
@@ -310,4 +312,29 @@ export function defineBlocks(blocks: {[key: string]: BlockDefinition}) {
   }
 }
 
+/**
+ * Handle a key-down on SVG drawing surface. Does nothing if the main workspace
+ * is not visible.
+ *
+ * @internal
+ * @param e Key down event.
+ */
+export function globalShortcutHandler(e: KeyboardEvent) {
+  const mainWorkspace = getMainWorkspace() as WorkspaceSvg;
+  if (!mainWorkspace) {
+    return;
+  }
+
+  if (
+    browserEvents.isTargetInput(e) ||
+    (mainWorkspace.rendered && !mainWorkspace.isVisible())
+  ) {
+    // When focused on an HTML text input widget, don't trap any keys.
+    // Ignore keypresses on rendered workspaces that have been explicitly
+    // hidden.
+    return;
+  }
+  ShortcutRegistry.registry.onKeyDown(mainWorkspace, e);
+}
+
 export const TEST_ONLY = {defineBlocksWithJsonArrayInternal};

core/dropdowndiv.ts

@@ -13,6 +13,7 @@
 // Former goog.module ID: Blockly.dropDownDiv
 
 import type {BlockSvg} from './block_svg.js';
+import * as browserEvents from './browser_events.js';
 import * as common from './common.js';
 import type {Field} from './field.js';
 import {ReturnEphemeralFocus, getFocusManager} from './focus_manager.js';
@@ -86,6 +87,9 @@ let positionToField: boolean | null = null;
 /** Callback to FocusManager to return ephemeral focus when the div closes. */
 let returnEphemeralFocus: ReturnEphemeralFocus | null = null;
 
+/** Identifier for shortcut keydown listener used to unbind it. */
+let keydownListener: browserEvents.Data | null = null;
+
 /**
  * Dropdown bounds info object used to encapsulate sizing information about a
  * bounding element (bounding box and width/height).
@@ -130,6 +134,13 @@ export function createDom() {
   content.className = 'blocklyDropDownContent';
   div.appendChild(content);
 
+  keydownListener = browserEvents.conditionalBind(
+    content,
+    'keydown',
+    null,
+    common.globalShortcutHandler,
+  );
+
   arrow = document.createElement('div');
   arrow.className = 'blocklyDropDownArrow';
   div.appendChild(arrow);
@@ -168,6 +179,10 @@ export function getContentDiv(): HTMLDivElement {
 
 /** Clear the content of the drop-down. */
 export function clearContent() {
+  if (keydownListener) {
+    browserEvents.unbind(keydownListener);
+    keydownListener = null;
+  }
   div.remove();
   createDom();
 }

core/inject.ts

@@ -15,7 +15,6 @@ import * as dropDownDiv from './dropdowndiv.js';
 import {Grid} from './grid.js';
 import {Options} from './options.js';
 import {ScrollbarPair} from './scrollbar_pair.js';
-import {ShortcutRegistry} from './shortcut_registry.js';
 import * as Tooltip from './tooltip.js';
 import * as Touch from './touch.js';
 import * as dom from './utils/dom.js';
@@ -72,17 +71,12 @@ export function inject(
     common.setMainWorkspace(workspace);
   });
 
-  browserEvents.conditionalBind(subContainer, 'keydown', null, onKeyDown);
   browserEvents.conditionalBind(
-    dropDownDiv.getContentDiv(),
+    subContainer,
     'keydown',
     null,
-    onKeyDown,
+    common.globalShortcutHandler,
   );
-  const widgetContainer = WidgetDiv.getDiv();
-  if (widgetContainer) {
-    browserEvents.conditionalBind(widgetContainer, 'keydown', null, onKeyDown);
-  }
 
   return workspace;
 }
@@ -292,32 +286,6 @@ function init(mainWorkspace: WorkspaceSvg) {
   }
 }
 
-/**
- * Handle a key-down on SVG drawing surface. Does nothing if the main workspace
- * is not visible.
- *
- * @param e Key down event.
- */
-// TODO (https://github.com/google/blockly/issues/1998) handle cases where there
-// are multiple workspaces and non-main workspaces are able to accept input.
-function onKeyDown(e: KeyboardEvent) {
-  const mainWorkspace = common.getMainWorkspace() as WorkspaceSvg;
-  if (!mainWorkspace) {
-    return;
-  }
-
-  if (
-    browserEvents.isTargetInput(e) ||
-    (mainWorkspace.rendered && !mainWorkspace.isVisible())
-  ) {
-    // When focused on an HTML text input widget, don't trap any keys.
-    // Ignore keypresses on rendered workspaces that have been explicitly
-    // hidden.
-    return;
-  }
-  ShortcutRegistry.registry.onKeyDown(mainWorkspace, e);
-}
-
 /**
  * Whether event handlers have been bound. Document event handlers will only
  * be bound once, even if Blockly is destroyed and reinjected.

core/widgetdiv.ts

@@ -6,6 +6,7 @@
 
 // Former goog.module ID: Blockly.WidgetDiv
 
+import * as browserEvents from './browser_events.js';
 import * as common from './common.js';
 import {Field} from './field.js';
 import {ReturnEphemeralFocus, getFocusManager} from './focus_manager.js';
@@ -66,15 +67,23 @@ export function testOnly_setDiv(newDiv: HTMLDivElement | null) {
 export function createDom() {
   const container = common.getParentContainer() || document.body;
 
-  if (document.querySelector('.' + containerClassName)) {
-    containerDiv = document.querySelector('.' + containerClassName);
+  const existingContainer = document.querySelector('div.' + containerClassName);
+  if (existingContainer) {
+    containerDiv = existingContainer as HTMLDivElement;
   } else {
-    containerDiv = document.createElement('div') as HTMLDivElement;
+    containerDiv = document.createElement('div');
     containerDiv.className = containerClassName;
     containerDiv.tabIndex = -1;
   }
 
-  container.appendChild(containerDiv!);
+  browserEvents.conditionalBind(
+    containerDiv,
+    'keydown',
+    null,
+    common.globalShortcutHandler,
+  );
+
+  container.appendChild(containerDiv);
 }
 
 /**

tests/mocha/dropdowndiv_test.js

@@ -136,6 +136,39 @@ suite('DropDownDiv', function () {
     });
   });
 
+  suite('Keyboard Shortcuts', function () {
+    setup(function () {
+      this.boundsStub = sinon
+        .stub(Blockly.DropDownDiv.TEST_ONLY, 'getBoundsInfo')
+        .returns({
+          left: 0,
+          right: 100,
+          top: 0,
+          bottom: 100,
+          width: 100,
+          height: 100,
+        });
+      this.workspace = Blockly.inject('blocklyDiv', {});
+    });
+    teardown(function () {
+      this.boundsStub.restore();
+    });
+    test('Escape dismisses DropDownDiv', function () {
+      let hidden = false;
+      Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, () => {
+        hidden = true;
+      });
+      assert.isFalse(hidden);
+      Blockly.DropDownDiv.getContentDiv().dispatchEvent(
+        new KeyboardEvent('keydown', {
+          key: 'Escape',
+          keyCode: 27, // example values.
+        }),
+      );
+      assert.isTrue(hidden);
+    });
+  });
+
   suite('show()', function () {
     test('without bounds set throws error', function () {
       const block = this.setUpBlockWithField();

tests/mocha/widget_div_test.js

@@ -287,6 +287,29 @@ suite('WidgetDiv', function () {
     });
   });
 
+  suite('Keyboard Shortcuts', function () {
+    test('Escape dismisses WidgetDiv', function () {
+      let hidden = false;
+      Blockly.WidgetDiv.show(
+        this,
+        false,
+        () => {
+          hidden = true;
+        },
+        this.workspace,
+        false,
+      );
+      assert.isFalse(hidden);
+      Blockly.WidgetDiv.getDiv().dispatchEvent(
+        new KeyboardEvent('keydown', {
+          key: 'Escape',
+          keyCode: 27, // example values.
+        }),
+      );
+      assert.isTrue(hidden);
+    });
+  });
+
   suite('show()', function () {
     test('shows nowhere', function () {
       const block = this.setUpBlockWithField();