Revert "fix: Auto close drop-down divs on lost focus (#9175)" (#9204)

This reverts commit 4c78c1d4a3 / PR #9175.

core/dropdowndiv.ts

@@ -213,8 +213,6 @@ export function setColour(backgroundColour: string, borderColour: string) {
  *     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 drop-down should automatically hide
- *     if it loses DOM focus for any reason.
  * @returns True if the menu rendered below block; false if above.
  */
 export function showPositionedByBlock<T>(
@@ -223,13 +221,11 @@ export function showPositionedByBlock<T>(
   opt_onHide?: () => void,
   opt_secondaryYOffset?: number,
   manageEphemeralFocus: boolean = true,
-  autoCloseOnLostFocus: boolean = true,
 ): boolean {
   return showPositionedByRect(
     getScaledBboxOfBlock(block),
     field as Field,
     manageEphemeralFocus,
-    autoCloseOnLostFocus,
     opt_onHide,
     opt_secondaryYOffset,
   );
@@ -249,8 +245,6 @@ export function showPositionedByBlock<T>(
  *     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 drop-down should automatically hide
- *     if it loses DOM focus for any reason.
  * @returns True if the menu rendered below block; false if above.
  */
 export function showPositionedByField<T>(
@@ -258,14 +252,12 @@ export function showPositionedByField<T>(
   opt_onHide?: () => void,
   opt_secondaryYOffset?: number,
   manageEphemeralFocus: boolean = true,
-  autoCloseOnLostFocus: boolean = true,
 ): boolean {
   positionToField = true;
   return showPositionedByRect(
     getScaledBboxOfField(field as Field),
     field as Field,
     manageEphemeralFocus,
-    autoCloseOnLostFocus,
     opt_onHide,
     opt_secondaryYOffset,
   );
@@ -310,15 +302,12 @@ function getScaledBboxOfField(field: Field): Rect {
  *     according to the drop-down div's lifetime. Note that if a false value is
  *     passed in here then callers should manage ephemeral focus directly
  *     otherwise focus may not properly restore when the widget closes.
- * @param autoCloseOnLostFocus Whether the drop-down should automatically hide
- *     if it loses DOM focus for any reason.
  * @returns True if the menu rendered below block; false if above.
  */
 function showPositionedByRect(
   bBox: Rect,
   field: Field,
   manageEphemeralFocus: boolean,
-  autoCloseOnLostFocus: boolean,
   opt_onHide?: () => void,
   opt_secondaryYOffset?: number,
 ): boolean {
@@ -346,7 +335,6 @@ function showPositionedByRect(
     secondaryX,
     secondaryY,
     manageEphemeralFocus,
-    autoCloseOnLostFocus,
     opt_onHide,
   );
 }
@@ -369,8 +357,6 @@ function showPositionedByRect(
  * @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 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.
  * @internal
  */
@@ -382,7 +368,6 @@ export function show<T>(
   secondaryX: number,
   secondaryY: number,
   manageEphemeralFocus: boolean,
-  autoCloseOnLostFocus: boolean,
   opt_onHide?: () => void,
 ): boolean {
   owner = newOwner as Field;
@@ -409,18 +394,7 @@ export function show<T>(
   // Ephemeral focus must happen after the div is fully visible in order to
   // ensure that it properly receives focus.
   if (manageEphemeralFocus) {
-    const autoCloseCallback = autoCloseOnLostFocus
+    returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
-      ? (hasFocus: boolean) => {
-          // If focus is ever lost, close the drop-down.
-          if (!hasFocus) {
-            hide();
-          }
-        }
-      : null;
-    returnEphemeralFocus = getFocusManager().takeEphemeralFocus(
-      div,
-      autoCloseCallback,
-    );
   }
 
   return atOrigin;
@@ -719,6 +693,7 @@ export function hideWithoutAnimation() {
     onHide();
     onHide = null;
   }
+  clearContent();
   owner = null;
 
   (common.getMainWorkspace() as WorkspaceSvg).markFocused();
@@ -727,13 +702,6 @@ export function hideWithoutAnimation() {
     returnEphemeralFocus();
     returnEphemeralFocus = null;
   }
-
-  // Content must be cleared after returning ephemeral focus since otherwise it
-  // may force focus changes which could desynchronize the focus manager and
-  // make it think the user directed focus away from the drop-down div (which
-  // will then notify it to not restore focus back to any previously focused
-  // node).
-  clearContent();
 }
 
 /**

core/focus_manager.ts

@@ -17,14 +17,6 @@ import {FocusableTreeTraverser} from './utils/focusable_tree_traverser.js';
  */
 export type ReturnEphemeralFocus = () => void;
 
-/**
- * Type declaration for an optional callback to observe when an element with
- * ephemeral focus has its DOM focus changed before ephemeral focus is returned.
- *
- * See FocusManager.takeEphemeralFocus for more details.
- */
-export type EphemeralFocusChangedInDom = (hasDomFocus: boolean) => void;
-
 /**
  * Represents an IFocusableTree that has been registered for focus management in
  * FocusManager.
@@ -86,10 +78,7 @@ export class FocusManager {
   private previouslyFocusedNode: IFocusableNode | null = null;
   private registeredTrees: Array<TreeRegistration> = [];
 
-  private ephemerallyFocusedElement: HTMLElement | SVGElement | null = null;
+  private currentlyHoldsEphemeralFocus: boolean = false;
-  private ephemeralDomFocusChangedCallback: EphemeralFocusChangedInDom | null =
-    null;
-  private ephemerallyFocusedElementCurrentlyHasFocus: boolean = false;
   private lockFocusStateChanges: boolean = false;
   private recentlyLostAllFocus: boolean = false;
   private isUpdatingFocusedNode: boolean = false;
@@ -129,21 +118,6 @@ export class FocusManager {
       } else {
         this.defocusCurrentFocusedNode();
       }
-
-      const ephemeralFocusElem = this.ephemerallyFocusedElement;
-      if (ephemeralFocusElem) {
-        const hadFocus = this.ephemerallyFocusedElementCurrentlyHasFocus;
-        const hasFocus =
-          !!element &&
-          element instanceof Node &&
-          ephemeralFocusElem.contains(element);
-        if (hadFocus !== hasFocus) {
-          if (this.ephemeralDomFocusChangedCallback) {
-            this.ephemeralDomFocusChangedCallback(hasFocus);
-          }
-          this.ephemerallyFocusedElementCurrentlyHasFocus = hasFocus;
-        }
-      }
     };
 
     // Register root document focus listeners for tracking when focus leaves all
@@ -339,7 +313,7 @@ export class FocusManager {
    */
   focusNode(focusableNode: IFocusableNode): void {
     this.ensureManagerIsUnlocked();
-    const mustRestoreUpdatingNode = !this.ephemerallyFocusedElement;
+    const mustRestoreUpdatingNode = !this.currentlyHoldsEphemeralFocus;
     if (mustRestoreUpdatingNode) {
       // Disable state syncing from DOM events since possible calls to focus()
       // below will loop a call back to focusNode().
@@ -421,7 +395,7 @@ export class FocusManager {
       this.removeHighlight(nextTreeRoot);
     }
 
-    if (!this.ephemerallyFocusedElement) {
+    if (!this.currentlyHoldsEphemeralFocus) {
       // Only change the actively focused node if ephemeral state isn't held.
       this.activelyFocusNode(nodeToFocus, prevTree ?? null);
     }
@@ -449,50 +423,24 @@ export class FocusManager {
    * the returned lambda is called. Additionally, only 1 ephemeral focus context
    * can be active at any given time (attempting to activate more than one
    * simultaneously will result in an error being thrown).
-   *
-   * Important details regarding the onFocusChangedInDom callback:
-   * - This method will be called initially with a value of 'true' indicating
-   *   that the ephemeral element has been focused, so callers can rely on that,
-   *   if needed, for initialization logic.
-   * - It's safe to end ephemeral focus in this callback (and is encouraged for
-   *   callers that wish to automatically end ephemeral focus when the user
-   *   directs focus outside of the element).
-   * - The element AND all of its descendants are tracked for focus. That means
-   *   the callback will ONLY be called with a value of 'false' if focus
-   *   completely leaves the DOM tree for the provided focusable element.
-   * - It's invalid to return focus on the very first call to the callback,
-   *   however this is expected to be impossible, anyway, since this method
-   *   won't return until after the first call to the callback (thus there will
-   *   be no means to return ephemeral focus).
-   *
-   * @param focusableElement The element that should be focused until returned.
-   * @param onFocusChangedInDom An optional callback which will be notified
-   *     whenever the provided element's focus changes before ephemeral focus is
-   *     returned. See the details above for specifics.
-   * @returns A ReturnEphemeralFocus that must be called when ephemeral focus
-   *     should end.
    */
   takeEphemeralFocus(
     focusableElement: HTMLElement | SVGElement,
-    onFocusChangedInDom: EphemeralFocusChangedInDom | null = null,
   ): ReturnEphemeralFocus {
     this.ensureManagerIsUnlocked();
-    if (this.ephemerallyFocusedElement) {
+    if (this.currentlyHoldsEphemeralFocus) {
       throw Error(
         `Attempted to take ephemeral focus when it's already held, ` +
           `with new element: ${focusableElement}.`,
       );
     }
-    this.ephemerallyFocusedElement = focusableElement;
+    this.currentlyHoldsEphemeralFocus = true;
-    this.ephemeralDomFocusChangedCallback = onFocusChangedInDom;
 
     if (this.focusedNode) {
       this.passivelyFocusNode(this.focusedNode, null);
     }
     focusableElement.focus();
-    this.ephemerallyFocusedElementCurrentlyHasFocus = true;
 
-    const focusedNodeAtStart = this.focusedNode;
     let hasFinishedEphemeralFocus = false;
     return () => {
       if (hasFinishedEphemeralFocus) {
@@ -502,22 +450,9 @@ export class FocusManager {
         );
       }
       hasFinishedEphemeralFocus = true;
-      this.ephemerallyFocusedElement = null;
+      this.currentlyHoldsEphemeralFocus = false;
-      this.ephemeralDomFocusChangedCallback = null;
 
-      const hadEphemeralFocusAtEnd =
+      if (this.focusedNode) {
-        this.ephemerallyFocusedElementCurrentlyHasFocus;
-      this.ephemerallyFocusedElementCurrentlyHasFocus = false;
-
-      // If the user forced away DOM focus during ephemeral focus, then
-      // determine whether focus should be restored back to a focusable node
-      // after ephemeral focus ends. Generally it shouldn't be, but in some
-      // cases (such as the user focusing an actual focusable node) it then
-      // should be.
-      const hasNewFocusedNode = focusedNodeAtStart !== this.focusedNode;
-      const shouldRestoreToNode = hasNewFocusedNode || hadEphemeralFocusAtEnd;
-
-      if (this.focusedNode && shouldRestoreToNode) {
         this.activelyFocusNode(this.focusedNode, null);
 
         // Even though focus was restored, check if it's lost again. It's
@@ -535,11 +470,6 @@ export class FocusManager {
             this.focusNode(capturedNode);
           }
         }, 0);
-      } else {
-        // If the ephemeral element lost focus then do not force it back since
-        // that likely will override the user's own attempt to move focus away
-        // from the ephemeral experience.
-        this.defocusCurrentFocusedNode();
       }
     };
   }
@@ -548,7 +478,7 @@ export class FocusManager {
    * @returns whether something is currently holding ephemeral focus
    */
   ephemeralFocusTaken(): boolean {
-    return !!this.ephemerallyFocusedElement;
+    return this.currentlyHoldsEphemeralFocus;
   }
 
   /**
@@ -586,7 +516,7 @@ export class FocusManager {
     // The current node will likely be defocused while ephemeral focus is held,
     // but internal manager state shouldn't change since the node should be
     // restored upon exiting ephemeral focus mode.
-    if (this.focusedNode && !this.ephemerallyFocusedElement) {
+    if (this.focusedNode && !this.currentlyHoldsEphemeralFocus) {
       this.passivelyFocusNode(this.focusedNode, null);
       this.updateFocusedNode(null);
     }

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, false, () => {
+      Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, () => {
         hidden = true;
       });
       assert.isFalse(hidden);
@@ -252,34 +252,6 @@ suite('DropDownDiv', function () {
       assert.strictEqual(Blockly.getFocusManager().getFocusedNode(), block);
       assert.strictEqual(document.activeElement, dropDownDivElem);
     });
-
-    test('without auto close on lost focus lost focus does not hide drop-down div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      Blockly.DropDownDiv.showPositionedByField(field, null, null, true, false);
-
-      // Focus an element outside of the drop-down.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // Even though the drop-down lost focus, it should still be visible.
-      const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
-      assert.strictEqual(dropDownDivElem.style.opacity, '1');
-    });
-
-    test('with auto close on lost focus lost focus hides drop-down div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      Blockly.DropDownDiv.showPositionedByField(field, null, null, true, true);
-
-      // Focus an element outside of the drop-down.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // the drop-down should now be hidden since it lost focus.
-      const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
-      assert.strictEqual(dropDownDivElem.style.opacity, '0');
-    });
   });
 
   suite('showPositionedByBlock()', function () {
@@ -353,48 +325,6 @@ suite('DropDownDiv', function () {
       assert.strictEqual(Blockly.getFocusManager().getFocusedNode(), block);
       assert.strictEqual(document.activeElement, dropDownDivElem);
     });
-
-    test('without auto close on lost focus lost focus does not hide drop-down div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      Blockly.DropDownDiv.showPositionedByBlock(
-        field,
-        block,
-        null,
-        null,
-        true,
-        false,
-      );
-
-      // Focus an element outside of the drop-down.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // Even though the drop-down lost focus, it should still be visible.
-      const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
-      assert.strictEqual(dropDownDivElem.style.opacity, '1');
-    });
-
-    test('with auto close on lost focus lost focus hides drop-down div', function () {
-      const block = this.setUpBlockWithField();
-      const field = Array.from(block.getFields())[0];
-      Blockly.getFocusManager().focusNode(block);
-      Blockly.DropDownDiv.showPositionedByBlock(
-        field,
-        block,
-        null,
-        null,
-        true,
-        true,
-      );
-
-      // Focus an element outside of the drop-down.
-      document.getElementById('nonTreeElementForEphemeralFocus').focus();
-
-      // the drop-down should now be hidden since it lost focus.
-      const dropDownDivElem = document.querySelector('.blocklyDropDownDiv');
-      assert.strictEqual(dropDownDivElem.style.opacity, '0');
-    });
   });
 
   suite('hideWithoutAnimation()', function () {

tests/mocha/focus_manager_test.js

@@ -5975,172 +5975,5 @@ suite('FocusManager', function () {
       );
       assert.strictEqual(document.activeElement, nodeElem);
     });
-
-    test('with focus change callback initially calls focus change callback with initial state', function () {
-      const callback = sinon.fake();
-      this.focusManager.registerTree(this.testFocusableTree2);
-      this.focusManager.registerTree(this.testFocusableGroup2);
-      const ephemeralElement = document.getElementById(
-        'nonTreeElementForEphemeralFocus',
-      );
-
-      this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
-
-      assert.strictEqual(callback.callCount, 1);
-      assert.isTrue(callback.firstCall.calledWithExactly(true));
-    });
-
-    test('with focus change callback finishes ephemeral does not calls focus change callback again', function () {
-      const callback = sinon.fake();
-      this.focusManager.registerTree(this.testFocusableTree2);
-      this.focusManager.registerTree(this.testFocusableGroup2);
-      const ephemeralElement = document.getElementById(
-        'nonTreeElementForEphemeralFocus',
-      );
-      const finishFocusCallback = this.focusManager.takeEphemeralFocus(
-        ephemeralElement,
-        callback,
-      );
-      callback.resetHistory();
-
-      finishFocusCallback();
-
-      assert.isFalse(callback.called);
-    });
-
-    test('with focus change callback set focus to ephemeral child does not call focus change callback again', function () {
-      const callback = sinon.fake();
-      this.focusManager.registerTree(this.testFocusableTree2);
-      this.focusManager.registerTree(this.testFocusableGroup2);
-      const ephemeralElement = document.getElementById(
-        'nonTreeElementForEphemeralFocus',
-      );
-      const ephemeralElementChild = document.getElementById(
-        'nonTreeElementForEphemeralFocus.child1',
-      );
-      this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
-      callback.resetHistory();
-
-      ephemeralElementChild.focus();
-
-      // Focusing a child element shouldn't invoke the callback since the
-      // ephemeral element's tree still holds focus.
-      assert.isFalse(callback.called);
-    });
-
-    test('with focus change callback set focus to non-ephemeral element calls focus change callback', function () {
-      const callback = sinon.fake();
-      this.focusManager.registerTree(this.testFocusableTree2);
-      this.focusManager.registerTree(this.testFocusableGroup2);
-      const ephemeralElement = document.getElementById(
-        'nonTreeElementForEphemeralFocus',
-      );
-      const ephemeralElement2 = document.getElementById(
-        'nonTreeElementForEphemeralFocus2',
-      );
-      this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
-
-      ephemeralElement2.focus();
-
-      // There should be a second call that indicates focus was lost.
-      assert.strictEqual(callback.callCount, 2);
-      assert.isTrue(callback.secondCall.calledWithExactly(false));
-    });
-
-    test('with focus change callback set focus to non-ephemeral element then back calls focus change callback again', function () {
-      const callback = sinon.fake();
-      this.focusManager.registerTree(this.testFocusableTree2);
-      this.focusManager.registerTree(this.testFocusableGroup2);
-      const ephemeralElement = document.getElementById(
-        'nonTreeElementForEphemeralFocus',
-      );
-      const ephemeralElementChild = document.getElementById(
-        'nonTreeElementForEphemeralFocus.child1',
-      );
-      const ephemeralElement2 = document.getElementById(
-        'nonTreeElementForEphemeralFocus2',
-      );
-      this.focusManager.takeEphemeralFocus(ephemeralElement, callback);
-      ephemeralElement2.focus();
-
-      ephemeralElementChild.focus();
-
-      // The latest call should be returning focus.
-      assert.strictEqual(callback.callCount, 3);
-      assert.isTrue(callback.thirdCall.calledWithExactly(true));
-    });
-
-    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);
-      const ephemeralElement = document.getElementById(
-        'nonTreeGroupForEphemeralFocus',
-      );
-      const ephemeralElement2 = document.getElementById(
-        'nonTreeElementForEphemeralFocus2',
-      );
-      const finishFocusCallback = this.focusManager.takeEphemeralFocus(
-        ephemeralElement,
-        (hasFocus) => {
-          if (!hasFocus) finishFocusCallback();
-        },
-      );
-
-      // Force focus away, triggering the callback's automatic returning logic.
-      ephemeralElement2.focus();
-
-      // The original focused node should be restored.
-      const nodeElem = this.testFocusableTree2Node1.getFocusableElement();
-      const activeElems = Array.from(
-        document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
-      );
-      assert.strictEqual(
-        this.focusManager.getFocusedNode(),
-        this.testFocusableTree2Node1,
-      );
-      assert.strictEqual(activeElems.length, 1);
-      assert.includesClass(
-        nodeElem.classList,
-        FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME,
-      );
-      assert.strictEqual(document.activeElement, nodeElem);
-    });
-
-    test('with focus on non-ephemeral element ephemeral ended does not restore to focused node', function () {
-      this.focusManager.registerTree(this.testFocusableTree2);
-      this.focusManager.registerTree(this.testFocusableGroup2);
-      this.focusManager.focusNode(this.testFocusableTree2Node1);
-      const ephemeralElement = document.getElementById(
-        'nonTreeGroupForEphemeralFocus',
-      );
-      const ephemeralElement2 = document.getElementById(
-        'nonTreeElementForEphemeralFocus2',
-      );
-      const finishFocusCallback =
-        this.focusManager.takeEphemeralFocus(ephemeralElement);
-      // Force focus away, triggering the callback's automatic returning logic.
-      ephemeralElement2.focus();
-
-      finishFocusCallback();
-
-      // 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.
-      const activeElems = Array.from(
-        document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
-      );
-      const passiveElems = Array.from(
-        document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR),
-      );
-      assert.isEmpty(activeElems);
-      assert.strictEqual(passiveElems.length, 1);
-      assert.includesClass(
-        this.testFocusableTree2Node1.getFocusableElement().classList,
-        FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME,
-      );
-      assert.strictEqual(document.activeElement, ephemeralElement2);
-      assert.isFalse(this.focusManager.ephemeralFocusTaken());
-    });
   });
 });

tests/mocha/index.html

@@ -94,13 +94,7 @@
       </div>
     </div>
     <div id="testUnfocusableElement">Unfocusable element</div>
-    <div id="nonTreeElementForEphemeralFocus" tabindex="-1">
+    <div id="nonTreeElementForEphemeralFocus" />
-      <div
-        id="nonTreeElementForEphemeralFocus.child1"
-        tabindex="-1"
-        style="margin-left: 1em"></div>
-    </div>
-    <div id="nonTreeElementForEphemeralFocus2" tabindex="-1"></div>
     <svg width="250" height="250">
       <g id="testFocusableGroup1">
         <g id="testFocusableGroup1.node1">