refactor!: update notification container to use native popover (#9819)

sissbruecker · web-flow · commit c173d67c9e62 · 2025-07-29T11:47:12.000+03:00
diff --git a/packages/notification/package.json b/packages/notification/package.json
@@ -39,7 +39,6 @@
     "@open-wc/dedupe-mixin": "^1.3.0",
     "@vaadin/component-base": "25.0.0-alpha8",
     "@vaadin/lit-renderer": "25.0.0-alpha8",
-    "@vaadin/overlay": "25.0.0-alpha8",
     "@vaadin/vaadin-lumo-styles": "25.0.0-alpha8",
     "@vaadin/vaadin-themable-mixin": "25.0.0-alpha8",
     "lit": "^3.0.0"
diff --git a/packages/notification/src/styles/vaadin-notification-container-core-styles.js b/packages/notification/src/styles/vaadin-notification-container-core-styles.js
@@ -8,13 +8,20 @@ import { css } from 'lit';
 export const notificationContainerStyles = css`
   :host {
     position: fixed;
-    z-index: 1000;
     inset: 0;
     box-sizing: border-box;
     display: flex;
     flex-direction: column;
     align-items: stretch;
     pointer-events: none;
+
+    /* Override native [popover] user agent styles */
+    width: auto;
+    height: auto;
+    border: none;
+    padding: 0;
+    background-color: transparent;
+    overflow: visible;
   }
 
   [region-group] {
diff --git a/packages/notification/src/vaadin-notification-mixin.d.ts b/packages/notification/src/vaadin-notification-mixin.d.ts
@@ -5,7 +5,6 @@
  */
 import type { Constructor } from '@open-wc/dedupe-mixin';
 import type { OverlayClassMixinClass } from '@vaadin/component-base/src/overlay-class-mixin.js';
-import type { OverlayStackMixinClass } from '@vaadin/overlay/src/vaadin-overlay-stack-mixin.js';
 import type { ThemePropertyMixinClass } from '@vaadin/vaadin-themable-mixin/vaadin-theme-property-mixin.js';
 import type { Notification } from './vaadin-notification.js';
 
@@ -27,7 +26,7 @@ export type NotificationRenderer = (root: HTMLElement, notification: Notificatio
  */
 export declare function NotificationContainerMixin<T extends Constructor<HTMLElement>>(
   base: T,
-): Constructor<NotificationContainerMixinClass> & Constructor<OverlayStackMixinClass> & T;
+): Constructor<NotificationContainerMixinClass> & T;
 
 export declare class NotificationContainerMixinClass {
   /**
diff --git a/packages/notification/src/vaadin-notification-mixin.js b/packages/notification/src/vaadin-notification-mixin.js
@@ -7,17 +7,15 @@ import { render } from 'lit';
 import { isTemplateResult } from 'lit/directive-helpers.js';
 import { isIOS } from '@vaadin/component-base/src/browser-utils.js';
 import { OverlayClassMixin } from '@vaadin/component-base/src/overlay-class-mixin.js';
-import { OverlayStackMixin } from '@vaadin/overlay/src/vaadin-overlay-stack-mixin.js';
 import { ThemePropertyMixin } from '@vaadin/vaadin-themable-mixin/vaadin-theme-property-mixin.js';
 
 /**
  * A mixin providing common notification container functionality.
  *
  * @polymerMixin
- * @mixes OverlayStackMixin
  */
 export const NotificationContainerMixin = (superClass) =>
-  class extends OverlayStackMixin(superClass) {
+  class extends superClass {
     static get properties() {
       return {
         /**
@@ -42,19 +40,36 @@ export const NotificationContainerMixin = (superClass) =>
       }
     }
 
+    /** @protected */
+    firstUpdated(props) {
+      super.firstUpdated(props);
+
+      this.popover = 'manual';
+    }
+
+    /**
+     * Move the notification container to the top of the stack.
+     */
+    bringToFront() {
+      if (this.matches(':popover-open')) {
+        this.hidePopover();
+        this.showPopover();
+      }
+    }
+
     /** @private */
     _openedChanged(opened) {
       if (opened) {
         document.body.appendChild(this);
-        this._appendAttachedInstance();
+        this.showPopover();
         document.addEventListener('vaadin-overlay-close', this._boundVaadinOverlayClose);
         if (this._boundIosResizeListener) {
           this._detectIosNavbar();
           window.addEventListener('resize', this._boundIosResizeListener);
         }
       } else {
         document.body.removeChild(this);
-        this._removeAttachedInstance();
+        this.hidePopover();
         document.removeEventListener('vaadin-overlay-close', this._boundVaadinOverlayClose);
         if (this._boundIosResizeListener) {
           window.removeEventListener('resize', this._boundIosResizeListener);
@@ -360,7 +375,11 @@ export const NotificationMixin = (superClass) =>
         return;
       }
 
-      this._container.bringToFront();
+      // Only call bringToFront if the container already has a child / was already opened.
+      // Otherwise, just setting `opened` on the container will make it the topmost overlay.
+      if (this._container.firstElementChild) {
+        this._container.bringToFront();
+      }
 
       this._card.slot = this.position;
       if (this._container.firstElementChild && /top/u.test(this.position)) {
diff --git a/packages/notification/test/multiple.test.js b/packages/notification/test/multiple.test.js
@@ -128,7 +128,7 @@ describe('multiple notification', () => {
   });
 
   describe('styles', () => {
-    it('content should ignore pointer events', () => {
+    it('container should ignore pointer events', () => {
       expect(getComputedStyle(container)['pointer-events']).to.equal('none');
     });
 
diff --git a/packages/notification/test/notification.test.js b/packages/notification/test/notification.test.js
@@ -4,28 +4,30 @@ import sinon from 'sinon';
 import '../src/vaadin-notification.js';
 
 describe('vaadin-notification', () => {
-  let notification;
+  let notification, container, showPopoverSpy, hidePopoverSpy;
 
   beforeEach(async () => {
     notification = fixtureSync(`
       <vaadin-notification duration="20"></vaadin-notification>
     `);
+    container = notification._container;
+    showPopoverSpy = sinon.spy(container, 'showPopover');
+    hidePopoverSpy = sinon.spy(container, 'hidePopover');
+
     await nextFrame();
 
     notification.renderer = (root) => {
       root.innerHTML = `Your work has been <strong>saved</strong>`;
     };
 
-    // Force sync card attaching and removal instead of waiting for the animation
-    sinon.stub(notification, '_animatedAppendNotificationCard').callsFake(() => notification._appendNotificationCard());
-    sinon.stub(notification, '_animatedRemoveNotificationCard').callsFake(() => notification._removeNotificationCard());
-
     notification.open();
   });
 
   afterEach(() => {
-    // Close to stop all pending timers.
-    notification.close();
+    // Close all notifications to stop all pending timers.
+    document.querySelectorAll('vaadin-notification').forEach((n) => {
+      n.close();
+    });
     // Delete singleton reference, so as it's created in next test
     delete notification.constructor._container;
   });
@@ -98,6 +100,42 @@ describe('vaadin-notification', () => {
       expect(isVisible(document.body.querySelector('vaadin-notification-container'))).to.be.true;
     });
 
+    it('should open as native popover', () => {
+      expect(container.popover).to.equal('manual');
+      expect(container.opened).to.be.true;
+      expect(showPopoverSpy.calledOnce).to.be.true;
+    });
+
+    it('should close native popover when notifications close', () => {
+      notification.close();
+
+      expect(container.opened).to.be.false;
+      expect(hidePopoverSpy.calledOnce).to.be.true;
+    });
+
+    it('should reopen native popover when notifications open', () => {
+      notification.close();
+      notification._removeNotificationCard();
+      notification.open();
+
+      expect(container.opened).to.be.true;
+      expect(showPopoverSpy.calledTwice).to.be.true;
+    });
+
+    it('should bring native popover to top of overlay stack when another notification opens', () => {
+      showPopoverSpy.resetHistory();
+
+      const anotherNotification = fixtureSync(`
+      <vaadin-notification duration="20"></vaadin-notification>
+    `);
+      anotherNotification.open();
+
+      // Bringing to front involves hiding and showing the popover again
+      expect(hidePopoverSpy.calledOnce).to.be.true;
+      expect(showPopoverSpy.calledOnce).to.be.true;
+      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', {
@@ -163,13 +201,6 @@ describe('vaadin-notification', () => {
         expect(container._detectIosNavbar.called).to.be.true;
         container._detectIosNavbar.restore();
       });
-
-      it('should bring to front on notification open ', () => {
-        notification.opened = false;
-        const spy = sinon.spy(container, 'bringToFront');
-        notification.opened = true;
-        expect(spy).to.be.calledOnce;
-      });
     });
   });
 
diff --git a/packages/overlay/src/vaadin-overlay-stack-mixin.js b/packages/overlay/src/vaadin-overlay-stack-mixin.js
@@ -16,13 +16,6 @@ const getAttachedInstances = () =>
     (el) => el instanceof HTMLElement && el._hasOverlayStackMixin && !el.hasAttribute('closing'),
   );
 
-/**
- * Returns all attached overlay instances excluding notification container,
- * which only needs to be in the stack for zIndex but not pointer-events.
- * @private
- */
-const getOverlayInstances = () => getAttachedInstances().filter((el) => el.$.overlay);
-
 /**
  * Returns true if all the instances on top of the overlay are nested overlays.
  * @private
@@ -49,7 +42,7 @@ const hasOnlyNestedOverlays = (overlay) => {
  * @protected
  */
 export const isLastOverlay = (overlay, filter = (_overlay) => true) => {
-  const filteredOverlays = getOverlayInstances().filter(filter);
+  const filteredOverlays = getAttachedInstances().filter(filter);
   return overlay === filteredOverlays.pop();
 };
 
@@ -128,7 +121,7 @@ export const OverlayStackMixin = (superClass) =>
       }
 
       // Disable pointer events in other attached overlays
-      getOverlayInstances().forEach((el) => {
+      getAttachedInstances().forEach((el) => {
         if (el !== this) {
           el.$.overlay.style.pointerEvents = 'none';
         }
@@ -144,7 +137,7 @@ export const OverlayStackMixin = (superClass) =>
       }
 
       // Restore pointer events in the previous overlay(s)
-      const instances = getOverlayInstances();
+      const instances = getAttachedInstances();
 
       let el;
       // Use instances.pop() to ensure the reverse order
diff --git a/packages/vaadin-lumo-styles/src/components/notification-container.css b/packages/vaadin-lumo-styles/src/components/notification-container.css
@@ -6,13 +6,20 @@
 @media lumo_components_notification-container {
   :host {
     position: fixed;
-    z-index: 1000;
     inset: 0;
     box-sizing: border-box;
     display: flex;
     flex-direction: column;
     align-items: stretch;
     pointer-events: none;
+
+    /* Override native [popover] user agent styles */
+    width: auto;
+    height: auto;
+    border: none;
+    padding: 0;
+    background-color: transparent;
+    overflow: visible;
   }
 
   [region-group] {
diff --git a/test/integration/notification-overlay.test.js b/test/integration/notification-overlay.test.js
