refactor: do not propagate overlay events through shadow roots (#9855)

* refactor: do not propagate overlay events through the DOM

* fix test

* improve integration test

* fix rte visual test

* allow open and close events to propagate to document

---------

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

Author: sissbruecker

packages/context-menu/test/helpers.js

@@ -7,7 +7,16 @@ export function activateItem(target, event = isTouch ? 'click' : 'mouseover') {
 }
 
 export async function openMenu(target, event = isTouch ? 'click' : 'mouseover') {
-  const menu = target.closest('vaadin-context-menu');
+  // Try to find a submenu first
+  let menu;
+  const overlayContent = target.closest('[slot="overlay"]');
+  if (overlayContent) {
+    menu = overlayContent.parentElement.querySelector('[slot="submenu"]');
+  }
+  // If no submenu is found, use the closest context menu
+  if (!menu) {
+    menu = target.closest('vaadin-context-menu');
+  }
   // Disable logic that delays opening submenu
   menu.__openListenerActive = true;
   activateItem(target, event);

packages/multi-select-combo-box/test/selecting-items.test.js

@@ -163,7 +163,7 @@ describe('selecting items', () => {
       await sendMouse({ type: 'click', position: [400, 400] });
 
       await sendKeys({ press: 'ArrowDown' });
-      await oneEvent(comboBox, 'vaadin-overlay-open');
+      await oneEvent(comboBox._overlayElement, 'vaadin-overlay-open');
 
       const item = getFirstItem(comboBox);
       expect(item.hasAttribute('focused')).to.be.false;

packages/notification/test/notification.test.js

@@ -136,34 +136,6 @@ describe('vaadin-notification', () => {
       expect(hidePopoverSpy.calledBefore(showPopoverSpy)).to.be.true;
     });
 
-    it('should cancel vaadin-overlay-close events when the source event occurred within the container', (done) => {
-      listenOnce(document, 'click', (clickEvent) => {
-        const overlayCloseEvent = new CustomEvent('vaadin-overlay-close', {
-          cancelable: true,
-          detail: { sourceEvent: clickEvent },
-        });
-        document.dispatchEvent(overlayCloseEvent);
-
-        expect(overlayCloseEvent.defaultPrevented).to.be.true;
-        done();
-      });
-      notification._card.click();
-    });
-
-    it('should not cancel vaadin-overlay-close events when the source event occurred outside of the container', (done) => {
-      listenOnce(document, 'click', (clickEvent) => {
-        const overlayCloseEvent = new CustomEvent('vaadin-overlay-close', {
-          cancelable: true,
-          detail: { sourceEvent: clickEvent },
-        });
-        document.dispatchEvent(overlayCloseEvent);
-
-        expect(overlayCloseEvent.defaultPrevented).to.be.false;
-        done();
-      });
-      document.body.click();
-    });
-
     (isIOS ? describe : describe.skip)('iOS incorrect viewport height workaround', () => {
       let container;
 

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

@@ -178,13 +178,17 @@ export const OverlayMixin = (superClass) =>
      * @param {Event=} sourceEvent
      */
     close(sourceEvent) {
-      const evt = new CustomEvent('vaadin-overlay-close', {
+      // Dispatch the event on the overlay. Not using composed, as propagating the event through shadow roots could have
+      // side effects when nesting overlays
+      const event = new CustomEvent('vaadin-overlay-close', {
         bubbles: true,
         cancelable: true,
         detail: { sourceEvent },
       });
-      this.dispatchEvent(evt);
-      if (!evt.defaultPrevented) {
+      this.dispatchEvent(event);
+      // To allow listening for the event globally, also dispatch it on the document body
+      document.body.dispatchEvent(event);
+      if (!event.defaultPrevented) {
         this.opened = false;
       }
     }
@@ -311,7 +315,12 @@ export const OverlayMixin = (superClass) =>
           setTimeout(() => {
             this._trapFocus();
 
-            this.dispatchEvent(new CustomEvent('vaadin-overlay-open', { bubbles: true, composed: true }));
+            // Dispatch the event on the overlay. Not using composed, as propagating the event through shadow roots
+            // could have side effects when nesting overlays
+            const event = new CustomEvent('vaadin-overlay-open', { bubbles: true });
+            this.dispatchEvent(event);
+            // To allow listening for the event globally, also dispatch it on the document body
+            document.body.dispatchEvent(event);
           });
         });
 
@@ -493,7 +502,6 @@ export const OverlayMixin = (superClass) =>
       }
 
       const evt = new CustomEvent('vaadin-overlay-outside-click', {
-        bubbles: true,
         cancelable: true,
         detail: { sourceEvent: event },
       });
@@ -520,7 +528,6 @@ export const OverlayMixin = (superClass) =>
 
       if (event.key === 'Escape') {
         const evt = new CustomEvent('vaadin-overlay-escape-press', {
-          bubbles: true,
           cancelable: true,
           detail: { sourceEvent: event },
         });

packages/overlay/test/basic.test.js

@@ -35,6 +35,34 @@ describe('vaadin-overlay', () => {
       await aTimeout(0);
       expect(spy).to.not.be.called;
     });
+
+    it('should not propagate through shadow roots', async () => {
+      overlay.opened = true;
+      await nextFrame();
+      await aTimeout(0);
+
+      expect(spy.firstCall.args[0].composed).to.be.false;
+    });
+
+    describe('global', () => {
+      let globalSpy;
+
+      beforeEach(() => {
+        globalSpy = sinon.spy();
+        document.addEventListener('vaadin-overlay-open', globalSpy);
+      });
+
+      afterEach(() => {
+        document.removeEventListener('vaadin-overlay-open', globalSpy);
+      });
+
+      it('should fire a global event on the document body when opened', async () => {
+        overlay.opened = true;
+        await nextFrame();
+        await aTimeout(0);
+        expect(globalSpy).to.be.called;
+      });
+    });
   });
 
   describe('popover', () => {

packages/overlay/test/interactions.test.js

@@ -1,6 +1,7 @@
 import { expect } from '@vaadin/chai-plugins';
 import { resetMouse, sendMouseToElement } from '@vaadin/test-runner-commands';
 import {
+  aTimeout,
   click,
   enterKeyDown,
   escKeyDown,
@@ -153,16 +154,42 @@ describe('interactions', () => {
     });
 
     describe('vaadin-overlay-close event', () => {
-      it('should prevent closing the overlay if the event was prevented', (done) => {
-        overlay.addEventListener('vaadin-overlay-close', (e) => {
-          e.preventDefault();
-
-          setTimeout(() => {
-            expect(overlay.opened).to.be.true;
-            done();
-          }, 1);
-        });
+      const preventDefaultListener = (e) => {
+        e.preventDefault();
+      };
+
+      it('should not propagate through shadow roots', () => {
+        const spy = sinon.spy();
+        overlay.addEventListener('vaadin-overlay-close', spy);
+
         click(parent);
+
+        expect(spy.firstCall.args[0].composed).to.be.false;
+      });
+
+      it('should prevent closing the overlay if the event was prevented', async () => {
+        overlay.addEventListener('vaadin-overlay-close', preventDefaultListener);
+        click(parent);
+        await aTimeout(1);
+
+        expect(overlay.opened).to.be.true;
+      });
+
+      describe('global', () => {
+        beforeEach(() => {
+          document.addEventListener('vaadin-overlay-close', preventDefaultListener);
+        });
+
+        afterEach(() => {
+          document.removeEventListener('vaadin-overlay-close', preventDefaultListener);
+        });
+
+        it('should prevent closing the overlay if the global event was prevented', async () => {
+          click(parent);
+          await aTimeout(1);
+
+          expect(overlay.opened).to.be.true;
+        });
       });
     });
 

packages/rich-text-editor/src/vaadin-rich-text-editor-popup.js

@@ -70,6 +70,7 @@ class RichTextEditorPopup extends PolylitMixin(LitElement) {
   render() {
     return html`
       <vaadin-rich-text-editor-popup-overlay
+        id="overlay"
         .owner="${this}"
         .opened="${this.opened}"
         .positionTarget="${this.target}"

packages/rich-text-editor/test/visual/base/rich-text-editor.test.js

@@ -103,7 +103,7 @@ describe('rich-text-editor', () => {
     it('color popup', async () => {
       element.style.minHeight = '200px';
       element.shadowRoot.querySelector('[part~="toolbar-button-color"]').click();
-      await oneEvent(element, 'vaadin-overlay-open');
+      await oneEvent(element.querySelector('[slot="color-popup"]').$.overlay, 'vaadin-overlay-open');
       return visualDiff(div, 'color-popup');
     });
   });

test/integration/notification-dialog.test.js

@@ -0,0 +1,66 @@
+import { expect } from '@vaadin/chai-plugins';
+import { sendKeys } from '@vaadin/test-runner-commands';
+import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
+import './not-animated-styles.js';
+import '@vaadin/dialog/src/vaadin-dialog.js';
+import '@vaadin/notification/src/vaadin-notification.js';
+
+describe('notification and dialog', () => {
+  let dialog, dialogInput, notification, notificationInput;
+
+  beforeEach(async () => {
+    const container = fixtureSync(`
+      <div>
+        <vaadin-dialog></vaadin-dialog>
+        <vaadin-notification></vaadin-notification>
+      </div>
+    `);
+    dialog = container.querySelector('vaadin-dialog');
+    dialog.renderer = (root) => {
+      if (root.firstElementChild) {
+        return;
+      }
+      dialogInput = document.createElement('input');
+      root.append(dialogInput);
+    };
+    dialog.opened = true;
+
+    notification = container.querySelector('vaadin-notification');
+    notification.renderer = (root) => {
+      if (root.firstElementChild) {
+        return;
+      }
+      notificationInput = document.createElement('input');
+      root.append(notificationInput);
+    };
+    notification.opened = true;
+
+    await nextRender();
+  });
+
+  it('should prevent the dialog from closing when pressing Escape in the notification', async () => {
+    notificationInput.focus();
+    await sendKeys({ press: 'Escape' });
+
+    expect(dialog.opened).to.be.true;
+  });
+
+  it('should allow the dialog to close when pressing Escape in the dialog', async () => {
+    dialogInput.focus();
+    await sendKeys({ press: 'Escape' });
+
+    expect(dialog.opened).to.be.false;
+  });
+
+  it('should prevent the dialog from closing when clicking in the notification', () => {
+    notificationInput.click();
+
+    expect(dialog.opened).to.be.true;
+  });
+
+  it('should allow the dialog to close when clicking outside', () => {
+    document.body.click();
+
+    expect(dialog.opened).to.be.false;
+  });
+});