Revert "fix: Auto-close widget divs on lost focus (RaspberryPiFoundation#9216)"

This reverts commit bea183d.

Author: microbit-robert

core/dropdowndiv.ts

@@ -346,8 +346,8 @@ function showPositionedByRect(
     secondaryX,
     secondaryY,
     manageEphemeralFocus,
-    opt_onHide,
     autoCloseOnLostFocus,
+    opt_onHide,
   );
 }
 
@@ -366,9 +366,9 @@ function showPositionedByRect(
  * @param primaryY Desired origin point y, in absolute px.
  * @param secondaryX Secondary/alternative origin point x, in absolute px.
  * @param secondaryY Secondary/alternative origin point y, in absolute px.
+ * @param opt_onHide Optional callback for when the drop-down is hidden.
  * @param manageEphemeralFocus Whether ephemeral focus should be managed
  *     according to the widget div's lifetime.
- * @param opt_onHide Optional callback for when the drop-down is hidden.
  * @param autoCloseOnLostFocus Whether the drop-down should automatically hide
  *     if it loses DOM focus for any reason.
  * @returns True if the menu rendered at the primary origin point.
@@ -382,8 +382,8 @@ export function show<T>(
   secondaryX: number,
   secondaryY: number,
   manageEphemeralFocus: boolean,
+  autoCloseOnLostFocus: boolean,
   opt_onHide?: () => void,
-  autoCloseOnLostFocus?: boolean,
 ): boolean {
   owner = newOwner as Field;
   onHide = opt_onHide || null;

core/focus_manager.ts

@@ -138,10 +138,10 @@ export class FocusManager {
           element instanceof Node &&
           ephemeralFocusElem.contains(element);
         if (hadFocus !== hasFocus) {
-          this.ephemerallyFocusedElementCurrentlyHasFocus = hasFocus;
           if (this.ephemeralDomFocusChangedCallback) {
             this.ephemeralDomFocusChangedCallback(hasFocus);
           }
+          this.ephemerallyFocusedElementCurrentlyHasFocus = hasFocus;
         }
       }
     };

core/widgetdiv.ts

@@ -99,16 +99,13 @@ export function createDom() {
  *     passed in here then callers should manage ephemeral focus directly
  *     otherwise focus may not properly restore when the widget closes. Defaults
  *     to true.
- * @param autoCloseOnLostFocus Whether the widget should automatically hide if
- *     it loses DOM focus for any reason.
  */
 export function show(
   newOwner: unknown,
   rtl: boolean,
   newDispose: () => void,
   workspace?: WorkspaceSvg | null,
   manageEphemeralFocus: boolean = true,
-  autoCloseOnLostFocus: boolean = true,
 ) {
   hide();
   owner = newOwner;
@@ -134,18 +131,7 @@ export function show(
     dom.addClass(div, themeClassName);
   }
   if (manageEphemeralFocus) {
-    const autoCloseCallback = autoCloseOnLostFocus
-      ? (hasFocus: boolean) => {
-          // If focus is ever lost, close the widget.
-          if (!hasFocus) {
-            hide();
-          }
-        }
-      : null;
-    returnEphemeralFocus = getFocusManager().takeEphemeralFocus(
-      div,
-      autoCloseCallback,
-    );
+    returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
   }
 }
 

tests/mocha/dropdowndiv_test.js

@@ -155,7 +155,7 @@ suite('DropDownDiv', function () {
     });
     test('Escape dismisses DropDownDiv', function () {
       let hidden = false;
-      Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, () => {
+      Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, false, () => {
         hidden = true;
       });
       assert.isFalse(hidden);
@@ -276,7 +276,7 @@ suite('DropDownDiv', function () {
       // Focus an element outside of the drop-down.
       document.getElementById('nonTreeElementForEphemeralFocus').focus();
 
-      // The drop-down should now be hidden since it lost focus.
+      // the drop-down should now be hidden since it lost focus.
       const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
       assert.strictEqual(dropDownDivElem.style.opacity, '0');
     });

tests/mocha/focus_manager_test.js

@@ -5624,6 +5624,21 @@ suite('FocusManager', function () {
   /* Ephemeral focus tests. */
 
   suite('takeEphemeralFocus()', function () {
+    setup(function () {
+      // Ensure ephemeral-specific elements are focusable.
+      document.getElementById('nonTreeElementForEphemeralFocus').tabIndex = -1;
+      document.getElementById('nonTreeGroupForEphemeralFocus').tabIndex = -1;
+    });
+    teardown(function () {
+      // Ensure ephemeral-specific elements have their tab indexes reset for a clean state.
+      document
+        .getElementById('nonTreeElementForEphemeralFocus')
+        .removeAttribute('tabindex');
+      document
+        .getElementById('nonTreeGroupForEphemeralFocus')
+        .removeAttribute('tabindex');
+    });
+
     test('with no focused node does not change states', function () {
       this.focusManager.registerTree(this.testFocusableTree2);
       this.focusManager.registerTree(this.testFocusableGroup2);
@@ -6055,7 +6070,7 @@ suite('FocusManager', function () {
       assert.isTrue(callback.thirdCall.calledWithExactly(true));
     });
 
-    test('with focus change callback set focus to non-ephemeral element with auto return finishes ephemeral does not restore to focused node', function () {
+    test('with focus change callback set focus to non-ephemeral element with auto return finishes ephemeral', function () {
       this.focusManager.registerTree(this.testFocusableTree2);
       this.focusManager.registerTree(this.testFocusableGroup2);
       this.focusManager.focusNode(this.testFocusableTree2Node1);
@@ -6075,24 +6090,21 @@ suite('FocusManager', function () {
       // Force focus away, triggering the callback's automatic returning logic.
       ephemeralElement2.focus();
 
-      // The original node should not be focused since the ephemeral element
-      // lost its own DOM focus while ephemeral focus was active. Instead, the
-      // newly active element should still hold focus.
+      // The original focused node should be restored.
+      const nodeElem = this.testFocusableTree2Node1.getFocusableElement();
       const activeElems = Array.from(
         document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
       );
-      const passiveElems = Array.from(
-        document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR),
+      assert.strictEqual(
+        this.focusManager.getFocusedNode(),
+        this.testFocusableTree2Node1,
       );
-      assert.isEmpty(activeElems);
-      assert.strictEqual(passiveElems.length, 1);
+      assert.strictEqual(activeElems.length, 1);
       assert.includesClass(
-        this.testFocusableTree2Node1.getFocusableElement().classList,
-        FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME,
+        nodeElem.classList,
+        FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME,
       );
-      assert.isNull(this.focusManager.getFocusedNode());
-      assert.strictEqual(document.activeElement, ephemeralElement2);
-      assert.isFalse(this.focusManager.ephemeralFocusTaken());
+      assert.strictEqual(document.activeElement, nodeElem);
     });
 
     test('with focus on non-ephemeral element ephemeral ended does not restore to focused node', function () {
@@ -6127,7 +6139,6 @@ suite('FocusManager', function () {
         this.testFocusableTree2Node1.getFocusableElement().classList,
         FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME,
       );
-      assert.isNull(this.focusManager.getFocusedNode());
       assert.strictEqual(document.activeElement, ephemeralElement2);
       assert.isFalse(this.focusManager.ephemeralFocusTaken());
     });

tests/mocha/index.html

@@ -142,7 +142,7 @@
           </text>
         </g>
       </g>
-      <g id="nonTreeGroupForEphemeralFocus" tabindex="-1"></g>
+      <g id="nonTreeGroupForEphemeralFocus"></g>
     </svg>
     <!-- Load mocha et al. before Blockly and the test modules so that
          we can safely import the test modules that make calls

tests/mocha/widget_div_test.js

@@ -444,71 +444,5 @@ suite('WidgetDiv', function () {
       assert.strictEqual(Blockly.getFocusManager().getFocusedNode(), block);
       assert.strictEqual(document.activeElement, blockFocusableElem);
     });
-
-    test('without auto close on lost focus lost focus does not hide widget div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      Blockly.WidgetDiv.show(field, false, () => {}, null, true, false);
-
-      // Focus an element outside of the widget.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // Even though the widget lost focus, it should still be visible.
-      const widgetDivElem = document.querySelector('.blocklyWidgetDiv');
-      assert.strictEqual(widgetDivElem.style.display, 'block');
-    });
-
-    test('with auto close on lost focus lost focus hides widget div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      Blockly.WidgetDiv.show(field, false, () => {}, null, true, true);
-
-      // Focus an element outside of the widget.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // The widget should now be hidden since it lost focus.
-      const widgetDivElem = document.querySelector('.blocklyWidgetDiv');
-      assert.strictEqual(widgetDivElem.style.display, 'none');
-    });
-
-    test('with auto close on lost focus lost focus with nested div hides widget div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      const nestedDiv = document.createElement('div');
-      nestedDiv.tabIndex = -1;
-      Blockly.WidgetDiv.getDiv().appendChild(nestedDiv);
-      Blockly.WidgetDiv.show(field, false, () => {}, null, true, true);
-      nestedDiv.focus(); // It's valid to focus this during ephemeral focus.
-
-      // Focus an element outside of the widget.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // The widget should now be hidden since it lost focus.
-      const widgetDivElem = document.querySelector('.blocklyWidgetDiv');
-      assert.strictEqual(widgetDivElem.style.display, 'none');
-    });
-
-    test('with auto close on lost focus lost focus with nested div does not restore DOM focus', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      const nestedDiv = document.createElement('div');
-      nestedDiv.tabIndex = -1;
-      Blockly.WidgetDiv.getDiv().appendChild(nestedDiv);
-      Blockly.WidgetDiv.show(field, false, () => {}, null, true, true);
-      nestedDiv.focus(); // It's valid to focus this during ephemeral focus.
-
-      // Focus an element outside of the widget.
-      const elem = document.getElementById('nonTreeElementForEphemeralFocus');
-      elem.focus();
-
-      // Auto hiding should not restore focus back to the block since ephemeral
-      // focus was lost before it was returned.
-      assert.isNull(Blockly.getFocusManager().getFocusedNode());
-      assert.strictEqual(document.activeElement, elem);
-    });
   });
 });