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

This reverts commit 7ad18f717a.

core/dropdowndiv.ts

@@ -213,6 +213,8 @@ 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>(
@@ -221,11 +223,13 @@ 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,
   );
@@ -245,6 +249,8 @@ 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>(
@@ -252,12 +258,14 @@ 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,
   );
@@ -302,12 +310,15 @@ 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 {
@@ -335,6 +346,7 @@ function showPositionedByRect(
     secondaryX,
     secondaryY,
     manageEphemeralFocus,
+    autoCloseOnLostFocus,
     opt_onHide,
   );
 }
@@ -357,6 +369,8 @@ 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
  */
@@ -368,6 +382,7 @@ export function show<T>(
   secondaryX: number,
   secondaryY: number,
   manageEphemeralFocus: boolean,
+  autoCloseOnLostFocus: boolean,
   opt_onHide?: () => void,
 ): boolean {
   owner = newOwner as Field;
@@ -394,7 +409,18 @@ 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) {
-    returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div);
+    const autoCloseCallback = autoCloseOnLostFocus
+      ? (hasFocus: boolean) => {
+          // If focus is ever lost, close the drop-down.
+          if (!hasFocus) {
+            hide();
+          }
+        }
+      : null;
+    returnEphemeralFocus = getFocusManager().takeEphemeralFocus(
+      div,
+      autoCloseCallback,
+    );
   }
 
   return atOrigin;
@@ -693,7 +719,6 @@ export function hideWithoutAnimation() {
     onHide();
     onHide = null;
   }
-  clearContent();
   owner = null;
 
   (common.getMainWorkspace() as WorkspaceSvg).markFocused();
@@ -702,6 +727,13 @@ 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,6 +17,14 @@ 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.
@@ -78,7 +86,10 @@ export class FocusManager {
   private previouslyFocusedNode: IFocusableNode | null = null;
   private registeredTrees: Array<TreeRegistration> = [];
 
-  private currentlyHoldsEphemeralFocus: boolean = false;
+  private ephemerallyFocusedElement: HTMLElement | SVGElement | null = null;
+  private ephemeralDomFocusChangedCallback: EphemeralFocusChangedInDom | null =
+    null;
+  private ephemerallyFocusedElementCurrentlyHasFocus: boolean = false;
   private lockFocusStateChanges: boolean = false;
   private recentlyLostAllFocus: boolean = false;
   private isUpdatingFocusedNode: boolean = false;
@@ -118,6 +129,21 @@ 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
@@ -313,7 +339,7 @@ export class FocusManager {
    */
   focusNode(focusableNode: IFocusableNode): void {
     this.ensureManagerIsUnlocked();
-    const mustRestoreUpdatingNode = !this.currentlyHoldsEphemeralFocus;
+    const mustRestoreUpdatingNode = !this.ephemerallyFocusedElement;
     if (mustRestoreUpdatingNode) {
       // Disable state syncing from DOM events since possible calls to focus()
       // below will loop a call back to focusNode().
@@ -395,7 +421,7 @@ export class FocusManager {
       this.removeHighlight(nextTreeRoot);
     }
 
-    if (!this.currentlyHoldsEphemeralFocus) {
+    if (!this.ephemerallyFocusedElement) {
       // Only change the actively focused node if ephemeral state isn't held.
       this.activelyFocusNode(nodeToFocus, prevTree ?? null);
     }
@@ -423,24 +449,50 @@ 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.currentlyHoldsEphemeralFocus) {
+    if (this.ephemerallyFocusedElement) {
       throw Error(
         `Attempted to take ephemeral focus when it's already held, ` +
           `with new element: ${focusableElement}.`,
       );
     }
-    this.currentlyHoldsEphemeralFocus = true;
+    this.ephemerallyFocusedElement = focusableElement;
+    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) {
@@ -450,9 +502,22 @@ export class FocusManager {
         );
       }
       hasFinishedEphemeralFocus = true;
-      this.currentlyHoldsEphemeralFocus = false;
+      this.ephemerallyFocusedElement = null;
+      this.ephemeralDomFocusChangedCallback = null;
 
-      if (this.focusedNode) {
+      const hadEphemeralFocusAtEnd =
+        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
@@ -470,6 +535,11 @@ 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();
       }
     };
   }
@@ -478,7 +548,7 @@ export class FocusManager {
    * @returns whether something is currently holding ephemeral focus
    */
   ephemeralFocusTaken(): boolean {
-    return this.currentlyHoldsEphemeralFocus;
+    return !!this.ephemerallyFocusedElement;
   }
 
   /**
@@ -516,7 +586,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.currentlyHoldsEphemeralFocus) {
+    if (this.focusedNode && !this.ephemerallyFocusedElement) {
       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, () => {
+      Blockly.DropDownDiv.show(this, false, 0, 0, 0, 0, false, false, () => {
         hidden = true;
       });
       assert.isFalse(hidden);
@@ -252,6 +252,34 @@ 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 () {
@@ -325,6 +353,48 @@ 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,5 +5975,172 @@ 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,7 +94,13 @@
       </div>
     </div>
     <div id="testUnfocusableElement">Unfocusable element</div>
-    <div id="nonTreeElementForEphemeralFocus" />
+    <div id="nonTreeElementForEphemeralFocus" tabindex="-1">
+      <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">