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

Author: sissbruecker

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

@@ -17,7 +17,7 @@ const getAttachedInstances = () => [...attachedInstances].filter((el) => !el.has
  * Returns true if all the instances on top of the overlay are nested overlays.
  * @private
  */
-const hasOnlyNestedOverlays = (overlay) => {
+export const hasOnlyNestedOverlays = (overlay) => {
   const instances = getAttachedInstances();
   const next = instances[instances.indexOf(overlay) + 1];
   if (!next) {

packages/popover/src/vaadin-popover.js

@@ -16,7 +16,10 @@ import { defineCustomElement } from '@vaadin/component-base/src/define.js';
 import { ElementMixin } from '@vaadin/component-base/src/element-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';
@@ -823,10 +826,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;
     }
 
@@ -852,13 +859,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.contains(event.relatedTarget)) {
       return;
     }
-
-    if (this.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;
     }
 
@@ -878,11 +888,11 @@ class Popover extends PopoverPositionMixin(
 
   /** @private */
   __onFocusOut(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;
     }
 
@@ -924,14 +934,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

@@ -5,7 +5,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);
@@ -22,11 +22,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';
       };
     };
@@ -39,45 +39,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;
     });
   });
 
@@ -91,11 +91,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;
@@ -105,11 +105,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;
@@ -119,8 +119,8 @@ describe('nested popover', () => {
       target.focus();
       await nextRender();
 
-      secondTarget.focus();
-      secondTarget.click();
+      nestedTarget.focus();
+      nestedTarget.click();
       await nextRender();
 
       outsideClick();
@@ -136,15 +136,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);
@@ -153,18 +153,117 @@ 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;
     });
   });
 });
+
+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;
+    });
+  });
+});