From 0aebf3c48473d7cd727be1d4f875b0ca196f317e Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Sat, 22 Aug 2026 08:36:42 -0700 Subject: [PATCH] fix: Keep the notification popup visible / centered (#2702) - Closes: #2693 --- src/components-lib/notification/action.ts | 8 +- .../notification-popup-viewport-controller.ts | 69 +++++++ src/components/notification/popup.ts | 8 + src/ha/panel.ts | 9 +- src/scss/notification-popup.scss | 13 +- src/utils/shadow-root.ts | 8 + .../notification/action.test.ts | 37 ++++ ...fication-popup-viewport-controller.test.ts | 187 ++++++++++++++++++ .../notification/popup.browser.test.ts | 138 +++++++++++++ tests/utils/shadow-root.test.ts | 30 +++ 10 files changed, 498 insertions(+), 9 deletions(-) create mode 100644 src/components-lib/notification/notification-popup-viewport-controller.ts create mode 100644 src/utils/shadow-root.ts create mode 100644 tests/components-lib/notification/notification-popup-viewport-controller.test.ts create mode 100644 tests/components/notification/popup.browser.test.ts create mode 100644 tests/utils/shadow-root.test.ts diff --git a/src/components-lib/notification/action.ts b/src/components-lib/notification/action.ts index 142b3c9f..0836dbdf 100644 --- a/src/components-lib/notification/action.ts +++ b/src/components-lib/notification/action.ts @@ -6,6 +6,8 @@ import { } from '../../utils/action.js'; import { arrayify } from '../../utils/basic.js'; +const DISMISSING_INTERACTIONS = ['tap', 'hold', 'double_tap']; + export function handleControlAction( ev: CustomEvent<{ action: string }>, control: NotificationControl, @@ -17,7 +19,11 @@ export function handleControlAction( if (action) { dispatchActionExecutionRequest(host, { actions: arrayify(action) }); } - if (onDismiss && control.dismiss !== false) { + if ( + onDismiss && + control.dismiss !== false && + DISMISSING_INTERACTIONS.includes(ev.detail.action) + ) { onDismiss(); } } diff --git a/src/components-lib/notification/notification-popup-viewport-controller.ts b/src/components-lib/notification/notification-popup-viewport-controller.ts new file mode 100644 index 00000000..3773cad0 --- /dev/null +++ b/src/components-lib/notification/notification-popup-viewport-controller.ts @@ -0,0 +1,69 @@ +import type { ReactiveController, ReactiveControllerHost } from 'lit'; + +import { getShadowRootHost } from '../../utils/shadow-root.js'; + +const INSET_TOP_PROPERTY = '--notification-popup-inset-top'; +const INSET_BOTTOM_PROPERTY = '--notification-popup-inset-bottom'; + +// Ensure the popup stays within the visible part of the card. +export class NotificationPopupViewportController implements ReactiveController { + private _host: ReactiveControllerHost & HTMLElement; + private _resizeObserver = new ResizeObserver(() => this._update()); + + constructor(host: ReactiveControllerHost & HTMLElement) { + this._host = host; + host.addController(this); + } + + public hostConnected(): void { + const container = this._getContainer(); + if (container) { + this._resizeObserver.observe(container); + } + + // Scroll events do not bubble, so the listener uses the capture phase to + // observe scrolling in any element between the window and the container. A + // Home Assistant dashboard scrolls an element within the page rather than + // the window itself. + window.addEventListener('scroll', this._update, { capture: true, passive: true }); + window.addEventListener('resize', this._update); + + this._update(); + } + + public hostDisconnected(): void { + this._resizeObserver.disconnect(); + window.removeEventListener('scroll', this._update, { capture: true }); + window.removeEventListener('resize', this._update); + } + + private _getContainer(): Element | null { + return getShadowRootHost(this._host); + } + + private _update = (): void => { + const container = this._getContainer(); + if (!container) { + return; + } + + const containerBox = container.getBoundingClientRect(); + const visibleTop = Math.max(containerBox.top, 0); + const visibleBottom = Math.min(containerBox.bottom, window.innerHeight); + + if (visibleBottom <= visibleTop) { + // No inset can bring the popup on screen when the container itself is + // off screen, so the controller keeps the previous inset. + return; + } + + this._host.style.setProperty( + INSET_TOP_PROPERTY, + `${visibleTop - containerBox.top}px`, + ); + this._host.style.setProperty( + INSET_BOTTOM_PROPERTY, + `${containerBox.bottom - visibleBottom}px`, + ); + }; +} diff --git a/src/components/notification/popup.ts b/src/components/notification/popup.ts index 516a64f5..a6cc5812 100644 --- a/src/components/notification/popup.ts +++ b/src/components/notification/popup.ts @@ -11,6 +11,7 @@ import { createRef, ref, type Ref } from 'lit/directives/ref.js'; import { handleControlAction } from '../../components-lib/notification/action.js'; import { NotificationContextController } from '../../components-lib/notification/notification-context-controller.js'; import { NotificationPopupController } from '../../components-lib/notification/notification-popup-controller.js'; +import { NotificationPopupViewportController } from '../../components-lib/notification/notification-popup-viewport-controller.js'; import type { Notification } from '../../config/schema/actions/types.js'; import { localize } from '../../localize/localize.js'; import notificationPopupStyle from '../../scss/notification-popup.scss?inline'; @@ -32,6 +33,13 @@ export class AdvancedCameraCardNotification extends LitElement { ); private _contextController = new NotificationContextController(this); + constructor() { + super(); + + // Controller automatically registers itself with this element. + new NotificationPopupViewportController(this); + } + protected render(): TemplateResult | void { if (!this.notification) { return; diff --git a/src/ha/panel.ts b/src/ha/panel.ts index 40664001..2820ae06 100644 --- a/src/ha/panel.ts +++ b/src/ha/panel.ts @@ -1,11 +1,8 @@ +import { getShadowRootHost } from '../utils/shadow-root'; + /** * Determine if a card is in panel mode. */ export const isCardInPanel = (card: HTMLElement): boolean => { - const parent = card.getRootNode(); - return !!( - parent && - parent instanceof ShadowRoot && - parent.host.tagName === 'HUI-PANEL-VIEW' - ); + return getShadowRootHost(card)?.tagName === 'HUI-PANEL-VIEW'; }; diff --git a/src/scss/notification-popup.scss b/src/scss/notification-popup.scss index 5950568e..b8c3ce8d 100644 --- a/src/scss/notification-popup.scss +++ b/src/scss/notification-popup.scss @@ -4,10 +4,13 @@ :host { position: absolute; - inset: 0; + + inset: var(--notification-popup-inset-top, 0) 0 + var(--notification-popup-inset-bottom, 0) 0; + display: flex; justify-content: center; - align-items: flex-end; + align-items: center; padding: 24px; pointer-events: none; z-index: $z-index-notification; @@ -28,6 +31,12 @@ width: min(92%, 400px); max-height: calc(100% - 48px); + + // min-height has priority over max-height: with the 40px vertical padding it + // holds the border box at 48px (matched to equal control's 8px offset + 32px + // + 8px). + min-height: 8px; + padding: 20px 24px; padding-right: 48px; overflow-y: auto; diff --git a/src/utils/shadow-root.ts b/src/utils/shadow-root.ts new file mode 100644 index 00000000..b5bcf1d9 --- /dev/null +++ b/src/utils/shadow-root.ts @@ -0,0 +1,8 @@ +/** + * Get the host of the shadow root an element lives in, or `null` when the + * element is not in a shadow tree (in the light DOM, or not connected). + */ +export const getShadowRootHost = (element: Node): Element | null => { + const root = element.getRootNode(); + return root instanceof ShadowRoot ? root.host : null; +}; diff --git a/tests/components-lib/notification/action.test.ts b/tests/components-lib/notification/action.test.ts index afbe791e..ec8c7f0e 100644 --- a/tests/components-lib/notification/action.test.ts +++ b/tests/components-lib/notification/action.test.ts @@ -82,6 +82,43 @@ describe('handleControlAction', () => { expect(onDismiss).not.toHaveBeenCalled(); }); + it.each([['hold'], ['double_tap']])( + 'should dismiss the notification on a %s', + (interaction: string) => { + vi.mocked(getActionConfigGivenAction).mockReturnValue(null); + + const ev = new CustomEvent('action', { detail: { action: interaction } }); + const host = document.createElement('div'); + const onDismiss = vi.fn(); + + handleControlAction(ev, createControl(), host, onDismiss); + + expect(onDismiss).toHaveBeenCalled(); + }, + ); + + it.each([['start_tap'], ['end_tap']])( + 'should dispatch a %s action without dismissing', + (interaction: string) => { + const action = { action: 'navigate' as const, navigation_path: '/foo' }; + vi.mocked(getActionConfigGivenAction).mockReturnValue(action); + + const ev = new CustomEvent('action', { detail: { action: interaction } }); + const control = createControl({ + actions: { [`${interaction}_action`]: action }, + }); + const host = document.createElement('div'); + const onDismiss = vi.fn(); + + handleControlAction(ev, control, host, onDismiss); + + expect(dispatchActionExecutionRequest).toHaveBeenCalledWith(host, { + actions: [action], + }); + expect(onDismiss).not.toHaveBeenCalled(); + }, + ); + it('should not call onDismiss when no onDismiss is provided', () => { vi.mocked(getActionConfigGivenAction).mockReturnValue(null); diff --git a/tests/components-lib/notification/notification-popup-viewport-controller.test.ts b/tests/components-lib/notification/notification-popup-viewport-controller.test.ts new file mode 100644 index 00000000..71554375 --- /dev/null +++ b/tests/components-lib/notification/notification-popup-viewport-controller.test.ts @@ -0,0 +1,187 @@ +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + onTestFinished, + vi, +} from 'vitest'; + +import { NotificationPopupViewportController } from '../../../src/components-lib/notification/notification-popup-viewport-controller'; +import { + callResizeHandler, + createLitElement, + getResizeObserver, + ResizeObserverMock, +} from '../../test-utils'; + +// @vitest-environment jsdom +describe('NotificationPopupViewportController', () => { + beforeAll(() => { + vi.stubGlobal('ResizeObserver', ResizeObserverMock); + }); + + afterAll(() => { + vi.unstubAllGlobals(); + }); + + beforeEach(() => { + vi.clearAllMocks(); + document.body.replaceChildren(); + }); + + const create = (options?: { containerTop?: number; containerHeight?: number }) => { + const container = document.createElement('div'); + document.body.appendChild(container); + + const containerTop = options?.containerTop ?? 0; + const containerHeight = options?.containerHeight ?? 100; + const setContainerBox = (top: number, height: number): void => { + container.getBoundingClientRect = vi.fn().mockReturnValue({ + top: top, + bottom: top + height, + }); + }; + setContainerBox(containerTop, containerHeight); + + const host = createLitElement(); + container.attachShadow({ mode: 'open' }).appendChild(host); + + const controller = new NotificationPopupViewportController(host); + + // Cleanup (disconnecting the window listeners) is registered per test, so + // leaked listeners cannot bleed into later tests. + onTestFinished(() => controller.hostDisconnected()); + + return { container, host, controller, setContainerBox }; + }; + + const getInsets = (host: HTMLElement): { top: string; bottom: string } => ({ + top: host.style.getPropertyValue('--notification-popup-inset-top'), + bottom: host.style.getPropertyValue('--notification-popup-inset-bottom'), + }); + + it('should add itself to the host', () => { + const { host, controller } = create(); + expect(host.addController).toHaveBeenCalledWith(controller); + }); + + it('should not inset a container that fits within the viewport', () => { + const { host, controller } = create({ containerTop: 0, containerHeight: 100 }); + controller.hostConnected(); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '0px' }); + }); + + it('should inset the part of the container below the bottom of the viewport', () => { + const { host, controller } = create({ + containerTop: 0, + containerHeight: window.innerHeight + 200, + }); + controller.hostConnected(); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '200px' }); + }); + + it('should inset the part of the container above the top of the viewport', () => { + const { host, controller } = create({ + containerTop: -100, + containerHeight: window.innerHeight + 300, + }); + controller.hostConnected(); + + expect(getInsets(host)).toEqual({ top: '100px', bottom: '200px' }); + }); + + it('should keep the last inset when the container is entirely out of view', () => { + const { host, controller, setContainerBox } = create({ + containerTop: 0, + containerHeight: window.innerHeight + 200, + }); + controller.hostConnected(); + + setContainerBox(window.innerHeight, 100); + document.body.dispatchEvent(new Event('scroll')); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '200px' }); + }); + + it('should do nothing when the popup is not within a shadow root', () => { + const host = createLitElement(); + document.body.appendChild(host); + + const controller = new NotificationPopupViewportController(host); + controller.hostConnected(); + onTestFinished(() => controller.hostDisconnected()); + + expect(getResizeObserver()?.observe).not.toHaveBeenCalled(); + expect(getInsets(host)).toEqual({ top: '', bottom: '' }); + }); + + describe('while connected', () => { + it('should recalculate the inset on scroll', () => { + const { host, controller, setContainerBox } = create(); + controller.hostConnected(); + + setContainerBox(0, window.innerHeight + 50); + document.body.dispatchEvent(new Event('scroll')); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '50px' }); + }); + + it('should recalculate the inset on a viewport resize', () => { + const { host, controller, setContainerBox } = create(); + controller.hostConnected(); + + setContainerBox(0, window.innerHeight + 50); + window.dispatchEvent(new Event('resize')); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '50px' }); + }); + + it('should recalculate the inset on a container resize', () => { + const { container, host, controller, setContainerBox } = create(); + controller.hostConnected(); + expect(getResizeObserver()?.observe).toHaveBeenCalledWith(container); + + setContainerBox(0, window.innerHeight + 50); + callResizeHandler(); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '50px' }); + }); + }); + + describe('once disconnected', () => { + it('should not recalculate the inset on scroll', () => { + const { host, controller, setContainerBox } = create(); + controller.hostConnected(); + controller.hostDisconnected(); + + setContainerBox(0, window.innerHeight + 50); + document.body.dispatchEvent(new Event('scroll')); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '0px' }); + }); + + it('should not recalculate the inset on a viewport resize', () => { + const { host, controller, setContainerBox } = create(); + controller.hostConnected(); + controller.hostDisconnected(); + + setContainerBox(0, window.innerHeight + 50); + window.dispatchEvent(new Event('resize')); + + expect(getInsets(host)).toEqual({ top: '0px', bottom: '0px' }); + }); + + it('should stop observing container resizes', () => { + const { controller } = create(); + controller.hostConnected(); + controller.hostDisconnected(); + + expect(getResizeObserver()?.disconnect).toHaveBeenCalled(); + }); + }); +}); diff --git a/tests/components/notification/popup.browser.test.ts b/tests/components/notification/popup.browser.test.ts new file mode 100644 index 00000000..d3eee952 --- /dev/null +++ b/tests/components/notification/popup.browser.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest'; + +import { deepQuery } from '../../browser/dom'; +import { MountedCardFactory, type MountedCard } from '../../browser/mounted-card'; +import { + createGenericCameraHASS, + createStillImageCardConfig, +} from '../../browser/test-utils'; + +const generateNotification = (card: MountedCard): void => { + card.card.dispatchEvent( + new CustomEvent('ll-custom', { + bubbles: true, + composed: true, + detail: { + action: 'fire-dom-event', + advanced_camera_card_action: 'notification', + notification: { + heading: { text: 'Heading', icon: 'mdi:information' }, + }, + }, + }), + ); +}; + +const isWithinViewport = (rect: DOMRect): boolean => + rect.height > 0 && rect.top >= 0 && rect.bottom <= window.innerHeight; + +// The `.notification` box inside the popup, but only once it is within the +// viewport, so a test can wait for the placement it asserts on. +const findVisibleNotification = (card: MountedCard): Element | null => { + const notification = deepQuery(card.card, 'advanced-camera-card-notification'); + const box = notification ? deepQuery(notification, '.notification') : null; + return box && isWithinViewport(box.getBoundingClientRect()) ? box : null; +}; + +describe('AdvancedCameraCardNotification', () => { + it('should place the popup within the viewport on a card taller than it', async () => { + const card = await MountedCardFactory.createFromSource( + createStillImageCardConfig({ + dimensions: { height: `${window.innerHeight * 3}px` }, + }), + createGenericCameraHASS(), + ); + + generateNotification(card); + + const box = await card.waitForRender( + () => findVisibleNotification(card), + 'notification within the viewport', + ); + + expect(card.card.getBoundingClientRect().bottom).toBeGreaterThan(window.innerHeight); + expect(isWithinViewport(box.getBoundingClientRect())).toBe(true); + }); + + it('should center the popup within the visible part of the card', async () => { + const card = await MountedCardFactory.createFromSource( + createStillImageCardConfig({ + dimensions: { height: `${window.innerHeight * 3}px` }, + }), + createGenericCameraHASS(), + ); + + generateNotification(card); + + const box = await card.waitForRender( + () => findVisibleNotification(card), + 'notification within the viewport', + ); + + // The pop-in animation translates the popup while it runs; measure only + // once every animation on the popup has finished. + await Promise.all(box.getAnimations().map((animation) => animation.finished)); + + // The visible band runs from the card's top edge to the viewport bottom, + // and the popup's center should sit at the band's center. + const bandTop = card.card.getBoundingClientRect().top; + const bandCenter = (bandTop + window.innerHeight) / 2; + const rect = box.getBoundingClientRect(); + const popupCenter = (rect.top + rect.bottom) / 2; + expect(Math.abs(popupCenter - bandCenter)).toBeLessThanOrEqual(2); + }); + + it('should not shrink the popup below its close control', async () => { + const card = await MountedCardFactory.createFromSource( + createStillImageCardConfig({ + dimensions: { height: `${window.innerHeight}px` }, + }), + createGenericCameraHASS(), + // Leave a sliver of the card on screen. + { position: { top: `${window.innerHeight}px` } }, + ); + + generateNotification(card); + + // Bring 20 pixels of the card's top edge into view at the bottom of the + // viewport: far less room than the popup's minimum height. + window.scrollTo(0, 20); + + await card.waitForRender(() => { + const notification = deepQuery(card.card, 'advanced-camera-card-notification'); + const box = notification ? deepQuery(notification, '.notification') : null; + const rect = box?.getBoundingClientRect(); + return rect && + rect.height >= 48 && + rect.top < window.innerHeight && + rect.bottom > 0 + ? box + : null; + }, 'a popup no smaller than its close control'); + }); + + it('should keep the popup within the viewport while scrolling', async () => { + const card = await MountedCardFactory.createFromSource( + createStillImageCardConfig({ + dimensions: { height: `${window.innerHeight * 3}px` }, + }), + createGenericCameraHASS(), + ); + + generateNotification(card); + await card.waitForRender( + () => findVisibleNotification(card), + 'notification within the viewport', + ); + + // Scroll the middle of the card into view, putting both its top and bottom + // edges outside the viewport. + window.scrollTo(0, window.innerHeight); + + const box = await card.waitForRender( + () => findVisibleNotification(card), + 'notification within the viewport after scrolling', + ); + expect(isWithinViewport(box.getBoundingClientRect())).toBe(true); + }); +}); diff --git a/tests/utils/shadow-root.test.ts b/tests/utils/shadow-root.test.ts new file mode 100644 index 00000000..a6bf1469 --- /dev/null +++ b/tests/utils/shadow-root.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; + +import { getShadowRootHost } from '../../src/utils/shadow-root'; + +// @vitest-environment jsdom +describe('getShadowRootHost', () => { + it('should return the host of the shadow root the element lives in', () => { + const host = document.createElement('div'); + host.attachShadow({ mode: 'open' }); + + const element = document.createElement('span'); + host.shadowRoot?.append(element); + + expect(getShadowRootHost(element)).toBe(host); + }); + + it('should return null for an element in the light DOM', () => { + const element = document.createElement('span'); + document.body.append(element); + try { + expect(getShadowRootHost(element)).toBeNull(); + } finally { + element.remove(); + } + }); + + it('should return null for an element that is not connected', () => { + expect(getShadowRootHost(document.createElement('span'))).toBeNull(); + }); +});