fix: allow popovers to close when interacting with non-nested overlays (#10322) (#10654)

Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>

Author: vaadin-bot

packages/overlay/src/vaadin-overlay-stack-mixin.js

@@ -13,6 +13,25 @@ const getAttachedInstances = () =>
     .filter((el) => el instanceof HTMLElement && el._hasOverlayStackMixin && !el.hasAttribute('closing'))
     .sort((a, b) => a.__zIndex - b.__zIndex || 0);
 
+/**
+ * Returns true if all the instances on top of the overlay are nested overlays.
+ * @private
+ */
+export const hasOnlyNestedOverlays = (overlay) => {
+  const instances = getAttachedInstances();
+  const next = instances[instances.indexOf(overlay) + 1];
+  if (!next) {
+    return true;
+  }
+
+  // Check if the overlay contains the owner element of another overlay
+  if (next.owner && !overlay._deepContains(next.owner)) {
+    return false;
+  }
+
+  return hasOnlyNestedOverlays(next);
+};
+
 /**
  * Returns all attached overlay instances excluding notification container,
  * which only needs to be in the stack for zIndex but not pointer-events.

packages/popover/src/vaadin-popover.js

@@ -17,7 +17,10 @@ import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
 import { OverlayClassMixin } from '@vaadin/component-base/src/overlay-class-mixin.js';
 import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
 import { generateUniqueId } from '@vaadin/component-base/src/unique-id-utils.js';
-import { isLastOverlay as isLastOverlayBase } from '@vaadin/overlay/src/vaadin-overlay-stack-mixin.js';
+import {
+  hasOnlyNestedOverlays,
+  isLastOverlay as isLastOverlayBase,
+} from '@vaadin/overlay/src/vaadin-overlay-stack-mixin.js';
 import { ThemePropertyMixin } from '@vaadin/vaadin-themable-mixin/vaadin-theme-property-mixin.js';
 import { PopoverPositionMixin } from './vaadin-popover-position-mixin.js';
 import { PopoverTargetMixin } from './vaadin-popover-target-mixin.js';
@@ -753,10 +756,14 @@ class Popover extends PopoverPositionMixin(
 
   /** @private */
   __onTargetFocusOut(event) {
-    // Do not close the popover on overlay focusout if it's not the last one.
+    // Do not close if there is a nested overlay that should be closed through some method first.
     // This covers the case when focus moves to the nested popover opened
     // without focusing parent popover overlay (e.g. using hover trigger).
-    if (this._overlayElement.opened && !isLastOverlay(this._overlayElement)) {
+    if (
+      this._overlayElement.opened &&
+      !isLastOverlay(this._overlayElement) &&
+      hasOnlyNestedOverlays(this._overlayElement)
+    ) {
       return;
     }
 
@@ -782,13 +789,16 @@ class Popover extends PopoverPositionMixin(
 
   /** @private */
   __onTargetMouseLeave(event) {
-    // Do not close the popover on target focusout if the overlay is not the last one.
-    // This happens e.g. when opening the nested popover that uses non-modal overlay.
-    if (this._overlayElement.opened && !isLastOverlay(this._overlayElement)) {
+    // Do not close if the pointer moves to the overlay
+    if (this._overlayElement.contains(event.relatedTarget)) {
       return;
     }
-
-    if (this._overlayElement.contains(event.relatedTarget)) {
+    // Do not close if there is a nested overlay that should be closed through some method first.
+    if (
+      this._overlayElement.opened &&
+      !isLastOverlay(this._overlayElement) &&
+      hasOnlyNestedOverlays(this._overlayElement)
+    ) {
       return;
     }
 
@@ -808,11 +818,11 @@ class Popover extends PopoverPositionMixin(
 
   /** @private */
   __onOverlayFocusOut(event) {
-    // Do not close the popover on overlay focusout if it's not the last one.
+    // Do not close if there is a nested overlay that should be closed through some method first.
     // This covers the following cases of nested overlay based components:
     // 1. Moving focus to the nested overlay (e.g. vaadin-select, vaadin-menu-bar)
     // 2. Closing not focused nested overlay on outside (e.g. vaadin-combo-box)
-    if (!isLastOverlay(this._overlayElement)) {
+    if (!isLastOverlay(this._overlayElement) && hasOnlyNestedOverlays(this._overlayElement)) {
       return;
     }
 
@@ -854,14 +864,12 @@ class Popover extends PopoverPositionMixin(
 
   /** @private */
   __onOverlayMouseLeave(event) {
-    // Do not close the popover on overlay focusout if it's not the last one.
-    // This happens when opening the nested component that uses "modal" overlay
-    // setting `pointer-events: none` on the body (combo-box, date-picker etc).
-    if (!isLastOverlay(this._overlayElement)) {
+    // Do not close if the pointer moves to the target
+    if (event.relatedTarget === this.target) {
       return;
     }
-
-    if (event.relatedTarget === this.target) {
+    // Do not close if there is a nested overlay that should be closed through some method first.
+    if (!isLastOverlay(this._overlayElement) && hasOnlyNestedOverlays(this._overlayElement)) {
       return;
     }
 

packages/popover/test/nested.test.js

@@ -6,7 +6,7 @@ import { Popover } from '../src/vaadin-popover.js';
 import { mouseenter, mouseleave } from './helpers.js';
 
 describe('nested popover', () => {
-  let popover, target, secondPopover, secondTarget;
+  let popover, target, nestedPopover, nestedTarget;
 
   before(() => {
     Popover.setDefaultFocusDelay(0);
@@ -23,11 +23,11 @@ describe('nested popover', () => {
         return;
       }
       root.innerHTML = `
-        <button id="second-target">Second target</button>
-        <vaadin-popover for="second-target"></vaadin-popover>
+        <button id="nested-target">Nested target</button>
+        <vaadin-popover for="nested-target"></vaadin-popover>
       `;
-      [secondTarget, secondPopover] = root.children;
-      secondPopover.renderer = (root2) => {
+      [nestedTarget, nestedPopover] = root.children;
+      nestedPopover.renderer = (root2) => {
         root2.textContent = 'Nested';
       };
     };
@@ -40,45 +40,45 @@ describe('nested popover', () => {
       target.click();
       await nextRender();
 
-      // Open the second popover
-      secondTarget.click();
+      // Open the nested popover
+      nestedTarget.click();
       await nextRender();
 
       // Expect both popovers to be opened
       expect(popover.opened).to.be.true;
-      expect(secondPopover.opened).to.be.true;
+      expect(nestedPopover.opened).to.be.true;
     });
 
     it('should close the topmost overlay on global Escape press', async () => {
       esc(document.body);
       await nextRender();
 
-      // Expect only the second popover to be closed
+      // Expect only the nested popover to be closed
       expect(popover.opened).to.be.true;
-      expect(secondPopover.opened).to.be.false;
+      expect(nestedPopover.opened).to.be.false;
 
       esc(document.body);
       await nextRender();
 
       // Expect both popovers to be closed
       expect(popover.opened).to.be.false;
-      expect(secondPopover.opened).to.be.false;
+      expect(nestedPopover.opened).to.be.false;
     });
 
     it('should close the topmost overlay on outside click', async () => {
       outsideClick();
       await nextRender();
 
-      // Expect only the second popover to be closed
+      // Expect only the nested popover to be closed
       expect(popover.opened).to.be.true;
-      expect(secondPopover.opened).to.be.false;
+      expect(nestedPopover.opened).to.be.false;
 
       outsideClick();
       await nextRender();
 
       // Expect both popovers to be closed
       expect(popover.opened).to.be.false;
-      expect(secondPopover.opened).to.be.false;
+      expect(nestedPopover.opened).to.be.false;
     });
   });
 
@@ -92,11 +92,11 @@ describe('nested popover', () => {
       target.focus();
       await nextRender();
 
-      secondPopover.modal = true;
-      await nextUpdate(secondPopover);
+      nestedPopover.modal = true;
+      await nextUpdate(nestedPopover);
 
       // Open programmatically so focus stays on target
-      secondPopover.opened = true;
+      nestedPopover.opened = true;
       await nextRender();
 
       expect(popover.opened).to.be.true;
@@ -106,11 +106,11 @@ describe('nested popover', () => {
       target.focus();
       await nextRender();
 
-      secondPopover.modal = true;
-      await nextUpdate(secondPopover);
+      nestedPopover.modal = true;
+      await nextUpdate(nestedPopover);
 
-      secondTarget.focus();
-      secondTarget.click();
+      nestedTarget.focus();
+      nestedTarget.click();
       await nextRender();
 
       expect(popover.opened).to.be.true;
@@ -120,8 +120,8 @@ describe('nested popover', () => {
       target.focus();
       await nextRender();
 
-      secondTarget.focus();
-      secondTarget.click();
+      nestedTarget.focus();
+      nestedTarget.click();
       await nextRender();
 
       outsideClick();
@@ -137,15 +137,15 @@ describe('nested popover', () => {
       await nextUpdate(popover);
     });
 
-    it('should not close when mouse leaves the target if popover is not the last one', async () => {
+    it('should not close when mouse leaves the target to nested popover', async () => {
       mouseenter(target);
       await nextRender();
 
-      mouseleave(target, secondTarget);
-      secondTarget.click();
+      mouseleave(target, nestedTarget);
+      nestedTarget.click();
       await nextRender();
 
-      mouseleave(secondTarget, target);
+      mouseleave(nestedTarget, target);
 
       mouseenter(target);
       mouseleave(target);
@@ -154,15 +154,15 @@ describe('nested popover', () => {
       expect(popover.opened).to.be.true;
     });
 
-    it('should not close when mouse leaves the overlay if popover is not the last one', async () => {
+    it('should not close when mouse leaves the overlay to nested popover', async () => {
       mouseenter(target);
       await nextRender();
 
-      mouseleave(target, secondTarget);
-      secondTarget.click();
+      mouseleave(target, nestedTarget);
+      nestedTarget.click();
       await nextRender();
 
-      mouseleave(secondTarget);
+      mouseleave(nestedTarget);
       await nextUpdate(popover);
 
       expect(popover.opened).to.be.true;
@@ -176,14 +176,113 @@ describe('nested popover', () => {
       await nextRender();
 
       // Open the second popover
-      secondTarget.click();
+      nestedTarget.click();
       await nextRender();
     });
 
     it('should bring to front nested overlay on parent overlay bringToFront()', () => {
-      const spy = sinon.spy(secondPopover._overlayElement, 'bringToFront');
+      const spy = sinon.spy(nestedPopover._overlayElement, 'bringToFront');
       popover._overlayElement.bringToFront();
       expect(spy).to.be.calledOnce;
     });
   });
 });
+
+describe('not nested popover', () => {
+  let popover, popoverContent, target, secondPopover, secondPopoverContent, secondTarget;
+
+  beforeEach(async () => {
+    popover = fixtureSync('<vaadin-popover></vaadin-popover>');
+    target = fixtureSync('<button>Target</button>');
+    popover.target = target;
+    popover.renderer = (root) => {
+      root.innerHTML = '<button>Popover content</button>';
+      popoverContent = root.firstElementChild;
+    };
+
+    secondPopover = fixtureSync('<vaadin-popover></vaadin-popover>');
+    secondTarget = fixtureSync('<button>Second Target</button>');
+    secondPopover.target = secondTarget;
+    secondPopover.trigger = [];
+    secondPopover.renderer = (root) => {
+      root.innerHTML = '<button>Second Popover content</button>';
+      secondPopoverContent = root.firstElementChild;
+    };
+    await nextRender();
+  });
+
+  describe('focus', () => {
+    beforeEach(() => {
+      popover.trigger = ['focus'];
+    });
+
+    it('should close the popover when focus moves from target to non-nested popover', async () => {
+      target.focus();
+      await nextRender();
+
+      // open second popover
+      secondPopover.opened = true;
+      await nextRender();
+
+      secondPopoverContent.focus();
+
+      expect(popover.opened).to.be.false;
+    });
+
+    it('should close when focus moves from the overlay to non-nested popover', async () => {
+      target.focus();
+      await nextRender();
+
+      popoverContent.focus();
+      await nextRender();
+      expect(popover.opened).to.be.true;
+
+      // open second popover
+      secondPopover.opened = true;
+      await nextRender();
+
+      secondPopoverContent.focus();
+      await nextRender();
+
+      expect(popover.opened).to.be.false;
+    });
+  });
+
+  describe('hover', () => {
+    beforeEach(() => {
+      popover.trigger = ['hover'];
+    });
+
+    it('should close the popover when mouse leaves target to non-nested popover', async () => {
+      mouseenter(target);
+      await nextRender();
+
+      // open second popover
+      secondPopover.opened = true;
+      await nextRender();
+
+      mouseleave(target, secondPopoverContent);
+      await nextUpdate(popover);
+
+      expect(popover.opened).to.be.false;
+    });
+
+    it('should close when mouse leaves the overlay to non-nested popover', async () => {
+      mouseenter(target);
+      await nextRender();
+
+      mouseenter(popoverContent);
+      await nextRender();
+      expect(popover.opened).to.be.true;
+
+      // open second popover
+      secondPopover.opened = true;
+      await nextRender();
+
+      mouseleave(popoverContent, secondPopoverContent);
+      await nextUpdate(popover);
+
+      expect(popover.opened).to.be.false;
+    });
+  });
+});