fix(overlays): no hiding of nested overlays having hideOnEsc configured

.changeset/rude-flies-reflect.md

@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+[overlays] no hiding of nested overlays having `hideOnEsc` configured

packages/ui/components/overlays/src/OverlayController.js

@@ -1,16 +1,17 @@
 import { overlays } from './singleton.js';
 import { containFocus } from './utils/contain-focus.js';
+import { deepContains } from './utils/deep-contains.js';
 import { overlayShadowDomStyle } from './overlayShadowDomStyle.js';
 import { _adoptStyleUtils } from './utils/adopt-styles.js';
 
 /**
- * @typedef {import('@lion/ui/types/overlays.js').OverlayConfig} OverlayConfig
+ * @typedef {'setup'|'init'|'teardown'|'before-show'|'show'|'hide'|'add'|'remove'} OverlayPhase
  * @typedef {import('@lion/ui/types/overlays.js').ViewportConfig} ViewportConfig
- * @typedef {import('@popperjs/core').createPopper} Popper
+ * @typedef {import('@lion/ui/types/overlays.js').OverlayConfig} OverlayConfig
  * @typedef {import('@popperjs/core').Options} PopperOptions
  * @typedef {import('@popperjs/core').Placement} Placement
+ * @typedef {import('@popperjs/core').createPopper} Popper
  * @typedef {{ createPopper: Popper }} PopperModule
- * @typedef {'setup'|'init'|'teardown'|'before-show'|'show'|'hide'|'add'|'remove'} OverlayPhase
  */
 
 /**
@@ -103,6 +104,8 @@ async function preloadPopper() {
   return /** @type {* & Promise<PopperModule>} */ (import('@popperjs/core/dist/esm/popper.js'));
 }
 
+const childDialogsClosedInEventLoopWeakmap = new WeakMap();
+
 /**
  * OverlayController is the fundament for every single type of overlay. With the right
  * configuration, it can be used to build (modal) dialogs, tooltips, dropdowns, popovers,
@@ -498,9 +501,6 @@ export class OverlayController extends EventTarget {
         '[OverlayController] .isTooltip only takes effect when .handlesAccessibility is enabled',
       );
     }
-    // if (newConfig.popperConfig.modifiers.arrow && !newConfig.contentWrapperNode) {
-    //   throw new Error('You need to provide a .contentWrapperNode when Popper arrow is enabled');
-    // }
   }
 
   /**
@@ -1146,10 +1146,39 @@ export class OverlayController extends EventTarget {
     ev.preventDefault();
   }
 
-  /** @private */
+  /**
-  __escKeyHandler(/** @type {KeyboardEvent} */ ev) {
+   * @param {KeyboardEvent} event
-    return ev.key === 'Escape' && this.hide();
+   * @returns {void}
+   */
+  __escKeyHandler(event) {
+    if (event.key !== 'Escape' || childDialogsClosedInEventLoopWeakmap.has(event)) return;
+
+    const hasPressedInside =
+      event.composedPath().includes(this.contentNode) ||
+      deepContains(this.contentNode, /** @type {HTMLElement|ShadowRoot} */ (event.target));
+    if (hasPressedInside) {
+      this.hide();
+      // We could do event.stopPropagation() here, but we don't want to hide info for
+      // the outside world about user interactions. Instead, we store the event in a WeakMap
+      // that will be garbage collected after the event loop.
+      childDialogsClosedInEventLoopWeakmap.set(event, this);
     }
+  }
+
+  /**
+   * @param {KeyboardEvent} event
+   * @returns {void}
+   */
+  #outsideEscKeyHandler = event => {
+    if (event.key !== 'Escape') return;
+
+    const hasPressedInside =
+      event.composedPath().includes(this.contentNode) ||
+      deepContains(this.contentNode, /** @type {HTMLElement|ShadowRoot} */ (event.target));
+    if (!hasPressedInside) {
+      this.hide();
+    }
+  };
 
   /**
    * @param {{ phase: OverlayPhase }} config
@@ -1175,11 +1204,9 @@ export class OverlayController extends EventTarget {
    */
   _handleHidesOnOutsideEsc({ phase }) {
     if (phase === 'show') {
-      this.__escKeyHandler = (/** @type {KeyboardEvent} */ ev) =>
+      document.addEventListener('keyup', this.#outsideEscKeyHandler);
-        ev.key === 'Escape' && this.hide();
-      document.addEventListener('keyup', this.__escKeyHandler);
     } else if (phase === 'hide') {
-      document.removeEventListener('keyup', this.__escKeyHandler);
+      document.removeEventListener('keyup', this.#outsideEscKeyHandler);
     }
   }
 

packages/ui/components/overlays/test-helpers/mimicClick.js

@@ -8,9 +8,9 @@ async function sleep(t = 0) {
 
 /**
  * @param {HTMLElement} el
- * @param {{isAsync?:boolean, releaseElement?: HTMLElement}} config
+ * @param {{isAsync?:boolean, releaseElement?: HTMLElement}} config releaseElement can be different when the mouse is dragged before release
  */
-export async function mimicClick(el, { isAsync, releaseElement } = { isAsync: false }) {
+export async function mimicClick(el, { isAsync = false, releaseElement } = {}) {
   const releaseEl = releaseElement || el;
   el.dispatchEvent(new MouseEvent('mousedown'));
   if (isAsync) {

packages/ui/components/overlays/test/OverlayController.test.js

@@ -1,28 +1,41 @@
 /* eslint-disable no-new */
-import {
-  aTimeout,
-  defineCE,
-  expect,
-  fixture,
-  html,
-  unsafeStatic,
-  fixtureSync,
-} from '@open-wc/testing';
-import sinon from 'sinon';
 import { OverlayController, overlays } from '@lion/ui/overlays.js';
 import { mimicClick } from '@lion/ui/overlays-test-helpers.js';
-import { keyCodes } from '../src/utils/key-codes.js';
+import sinon from 'sinon';
-import { simulateTab } from '../src/utils/simulate-tab.js';
+import {
-import { _adoptStyleUtils } from '../src/utils/adopt-styles.js';
+  unsafeStatic,
+  fixtureSync,
+  defineCE,
+  aTimeout,
+  fixture,
+  expect,
+  html,
+} from '@open-wc/testing';
+
 import { createShadowHost } from '../test-helpers/createShadowHost.js';
+import { _adoptStyleUtils } from '../src/utils/adopt-styles.js';
+import { simulateTab } from '../src/utils/simulate-tab.js';
+import { keyCodes } from '../src/utils/key-codes.js';
 
 /**
- * @typedef {import('../types/OverlayConfig.js').OverlayConfig} OverlayConfig
  * @typedef {import('../types/OverlayConfig.js').ViewportPlacement} ViewportPlacement
+ * @typedef {import('../types/OverlayConfig.js').OverlayConfig} OverlayConfig
  */
 
 const wrappingDialogNodeStyle = 'display: none; z-index: 9999; padding: 0px;';
 
+/**
+ * A small wrapper function that closely mimics an escape press from a user
+ * (prevents common mistakes like no bubbling or keydown)
+ * @param {Element|Document} element
+ */
+async function mimicEscapePress(element) {
+  element.dispatchEvent(
+    new KeyboardEvent('keyup', { key: 'Escape', bubbles: true, composed: true }),
+  );
+  await aTimeout(0);
+}
+
 /**
  * Make sure that all browsers serialize html in a similar way
  * (Firefox tends to output empty style attrs)
@@ -74,6 +87,50 @@ const withLocalTestConfig = () =>
     ),
   });
 
+/**
+ * @param {HTMLDivElement} parentContent
+ * @returns {Promise<{parentOverlay: OverlayController; childOverlay: OverlayController}>}
+ */
+async function createNestedEscControllers(parentContent) {
+  const childContent = /** @type {HTMLDivElement} */ (parentContent.querySelector('div[id]'));
+  // Assert valid fixure
+  const isValid =
+    (parentContent.id === 'parent-overlay--hidesOnEsc' ||
+      parentContent.id === 'parent-overlay--hidesOnOutsideEsc') &&
+    (childContent.id === 'child-overlay--hidesOnEsc' ||
+      childContent.id === 'child-overlay--hidesOnOutsideEsc');
+  if (!isValid) {
+    throw new Error('Provide a valid fixture');
+  }
+
+  if (parentContent.hasAttribute('data-convert-to-shadow-root')) {
+    const shadowRootParent = parentContent.attachShadow({ mode: 'open' });
+    shadowRootParent.appendChild(childContent);
+  }
+
+  const parentHasOutsideOnEsc = parentContent.id === 'parent-overlay--hidesOnOutsideEsc';
+  const childHasOutsideOnEsc = childContent.id === 'child-overlay--hidesOnOutsideEsc';
+
+  const parentConfig = parentHasOutsideOnEsc ? { hidesOnOutsideEsc: true } : { hidesOnEsc: true };
+  const childConfig = childHasOutsideOnEsc ? { hidesOnOutsideEsc: true } : { hidesOnEsc: true };
+
+  const parentOverlay = new OverlayController({
+    ...withGlobalTestConfig(),
+    contentNode: parentContent,
+    ...parentConfig,
+  });
+  const childOverlay = new OverlayController({
+    ...withGlobalTestConfig(),
+    contentNode: childContent,
+    ...childConfig,
+  });
+
+  await parentOverlay.show();
+  await childOverlay.show();
+
+  return { parentOverlay, childOverlay };
+}
+
 afterEach(() => {
   overlays.teardown();
 });
@@ -574,74 +631,221 @@ describe('OverlayController', () => {
     });
 
     describe('hidesOnEsc', () => {
-      it('hides when [escape] is pressed', async () => {
+      it('hides on [Escape] press', async () => {
         const ctrl = new OverlayController({
           ...withGlobalTestConfig(),
           hidesOnEsc: true,
         });
         await ctrl.show();
-        ctrl.contentNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
+        await mimicEscapePress(ctrl.contentNode);
-        await aTimeout(0);
         expect(ctrl.isShown).to.be.false;
       });
 
-      it("doesn't hide when [escape] is pressed and hidesOnEsc is set to false", async () => {
+      it('stays shown on [Escape] press with `.hidesOnEsc` set to false', async () => {
         const ctrl = new OverlayController({
           ...withGlobalTestConfig(),
           hidesOnEsc: false,
         });
         await ctrl.show();
-        ctrl.contentNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
+        await mimicEscapePress(ctrl.contentNode);
-        await aTimeout(0);
         expect(ctrl.isShown).to.be.true;
       });
 
-      it('stays shown when [escape] is pressed on outside element', async () => {
+      it('stays shown on [Escape] press on outside element', async () => {
         const ctrl = new OverlayController({
           ...withGlobalTestConfig(),
           hidesOnEsc: true,
         });
         await ctrl.show();
-        document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
+        await mimicEscapePress(document);
         expect(ctrl.isShown).to.be.true;
       });
 
-      it('does not hide when [escape] is pressed with modal <dialog> and "hidesOnEsc" is false', async () => {
+      it('stays shown on [Escape] press with modal <dialog> and `.hidesOnEsc` is false', async () => {
         const ctrl = new OverlayController({
           ...withGlobalTestConfig(),
           trapsKeyboardFocus: true,
           hidesOnEsc: false,
         });
         await ctrl.show();
-        ctrl.contentNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
+        await mimicEscapePress(ctrl.contentNode);
-        await aTimeout(0);
         expect(ctrl.isShown).to.be.true;
       });
+
+      it('parent stays shown on [Escape] press in a nested overlay', async () => {
+        const parentContent = /** @type {HTMLDivElement} */ (
+          await fixture(
+            html` <!-- -->
+              <div id="parent-overlay--hidesOnEsc">
+                <div id="child-overlay--hidesOnEsc">we press [Escape] here</div>
+              </div>`,
+          )
+        );
+        const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+        await mimicEscapePress(childOverlay.contentNode);
+
+        expect(parentOverlay.isShown).to.be.true;
+        expect(childOverlay.isShown).to.be.false;
+      });
     });
 
     describe('hidesOnOutsideEsc', () => {
-      it('hides when [escape] is pressed on outside element', async () => {
+      it('hides on [Escape] press on outside element', async () => {
         const ctrl = new OverlayController({
           ...withGlobalTestConfig(),
           hidesOnOutsideEsc: true,
         });
         await ctrl.show();
-        document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
+        await mimicEscapePress(document);
-        await aTimeout(0);
         expect(ctrl.isShown).to.be.false;
       });
 
-      it('stays shown when [escape] is pressed on inside element', async () => {
+      it('stays shown on [Escape] press on inside element', async () => {
         const ctrl = new OverlayController({
           ...withGlobalTestConfig(),
           hidesOnOutsideEsc: true,
         });
         await ctrl.show();
-        ctrl.contentNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
+        await mimicEscapePress(ctrl.contentNode);
         expect(ctrl.isShown).to.be.true;
       });
+
+      it('stays shown on [Escape] press in a nested overlay', async () => {
+        const parentContent = /** @type {HTMLDivElement} */ (
+          await fixture(
+            html` <!-- -->
+              <div id="parent-overlay--hidesOnOutsideEsc">
+                <div id="child-overlay--hidesOnOutsideEsc">we press [Escape] here</div>
+              </div>`,
+          )
+        );
+        const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+        mimicEscapePress(childOverlay.contentNode);
+
+        expect(parentOverlay.isShown).to.be.true;
+        expect(childOverlay.isShown).to.be.true;
+      });
     });
 
+    describe('Nested hidesOnEsc / hidesOnOutsideEsc', () => {
+      describe('Parent has hidesOnEsc and child has hidesOnOutsideEsc', () => {
+        it('on [Escape] press in child overlay: parent hides, child stays shown', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- -->
+                <div id="parent-overlay--hidesOnEsc">
+                  <div id="child-overlay--hidesOnOutsideEsc">we press [Escape] here</div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(childOverlay.contentNode);
+
+          expect(parentOverlay.isShown).to.be.false;
+          expect(childOverlay.isShown).to.be.true;
+        });
+
+        it('on [Escape] press outside overlays: parent stays shown, child hides', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- we press [Escape] here -->
+                <div id="parent-overlay--hidesOnEsc">
+                  <div id="child-overlay--hidesOnOutsideEsc"></div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(document);
+
+          expect(parentOverlay.isShown).to.be.true;
+          expect(childOverlay.isShown).to.be.false;
+        });
+
+        it('on [Escape] press in parent overlay: parent is hidden, child is hidden', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- -->
+                <div id="parent-overlay--hidesOnEsc">
+                  we press [Escape] here
+                  <div id="child-overlay--hidesOnOutsideEsc"></div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(parentContent);
+
+          expect(parentOverlay.isShown).to.be.false;
+          expect(childOverlay.isShown).to.be.false;
+        });
+      });
+      describe('Parent has hidesOnOutsideEsc and child has hidesOnEsc', () => {
+        it('on [Escape] press in child overlay: parent stays shown, child hides', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- -->
+                <div id="parent-overlay--hidesOnOutsideEsc">
+                  <div id="child-overlay--hidesOnEsc">we press [Escape] here</div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(childOverlay.contentNode);
+
+          expect(parentOverlay.isShown).to.be.true;
+          expect(childOverlay.isShown).to.be.false;
+        });
+
+        it('on [Escape] press outside overlays: parent hides, child stays shown', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- we press [Escape] here -->
+                <div id="parent-overlay--hidesOnOutsideEsc">
+                  <div id="child-overlay--hidesOnEsc"></div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(document);
+
+          expect(parentOverlay.isShown).to.be.false;
+          expect(childOverlay.isShown).to.be.true;
+        });
+
+        it('on [Escape] press in parent overlay: parent hides, child stays shown', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- we press [Escape] here -->
+                <div id="parent-overlay--hidesOnOutsideEsc">
+                  <div id="child-overlay--hidesOnEsc"></div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(document);
+
+          expect(parentOverlay.isShown).to.be.false;
+          expect(childOverlay.isShown).to.be.true;
+        });
+      });
+
+      describe('With shadow dom', () => {
+        it('on [Escape] press in child overlay in shadow root: parent hides, child stays shown', async () => {
+          const parentContent = /** @type {HTMLDivElement} */ (
+            await fixture(
+              html` <!-- -->
+                <div id="parent-overlay--hidesOnEsc" data-convert-to-shadow-root>
+                  <div id="child-overlay--hidesOnOutsideEsc">we press [Escape] here</div>
+                </div>`,
+            )
+          );
+          const { parentOverlay, childOverlay } = await createNestedEscControllers(parentContent);
+          await mimicEscapePress(childOverlay.contentNode);
+
+          expect(parentOverlay.isShown).to.be.false;
+          expect(childOverlay.isShown).to.be.true;
+        });
+      });
+    });
     describe('hidesOnOutsideClick', () => {
       it('hides on outside click', async () => {
         const contentNode = /** @type {HTMLElement} */ (await fixture('<div>Content</div>'));
@@ -684,9 +888,8 @@ describe('OverlayController', () => {
         expect(ctrl.isShown).to.be.true;
 
         // Don't hide on inside mousedown & outside mouseup
-        ctrl.contentNode.dispatchEvent(new MouseEvent('mousedown'));
+        await mimicClick(ctrl.contentNode, { releaseElement: document.body, isAsync: true });
-        await aTimeout(0);
+
-        document.body.dispatchEvent(new MouseEvent('mouseup'));
         await aTimeout(0);
         expect(ctrl.isShown).to.be.true;