From 965f2b4c9d7d56717b1cd2ff636e217cb9bf59c6 Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Sat, 22 Aug 2026 21:29:04 -0700 Subject: [PATCH] fix: Keep the notification popup open while its text is selected (#2706) --- .../notification-popup-controller.ts | 29 ++- .../notification/common-rendering.ts | 2 + src/components/notification/popup.ts | 10 +- src/localize/languages/en.json | 1 + src/scss/notification-popup.scss | 2 + tests/browser/dom.ts | 19 ++ .../card-element-manager.browser.test.ts | 13 +- ...ification-popup-controller.browser.test.ts | 167 ++++++++++++++++++ .../notification-popup-controller.test.ts | 92 +++++++++- .../notification/block.browser.test.ts | 41 +++++ 10 files changed, 360 insertions(+), 16 deletions(-) create mode 100644 tests/components-lib/notification/notification-popup-controller.browser.test.ts create mode 100644 tests/components/notification/block.browser.test.ts diff --git a/src/components-lib/notification/notification-popup-controller.ts b/src/components-lib/notification/notification-popup-controller.ts index 78b2734a..b7dc010c 100644 --- a/src/components-lib/notification/notification-popup-controller.ts +++ b/src/components-lib/notification/notification-popup-controller.ts @@ -4,11 +4,13 @@ import { hasPopOutAnimationEnded } from '../../utils/animation.js'; import { dispatchDismissNotificationEvent } from '../../utils/notification.js'; // Manages the popup notification's modal interaction: dismiss on outside -// interaction or Escape, and emit the dismiss event once the pop-out animation -// finishes. +// interaction or Escape, hold focus while it is shown, and emit the dismiss +// event once the pop-out animation finishes. export class NotificationPopupController implements ReactiveController { private _host: ReactiveControllerHost & HTMLElement; private _getNotificationElement: () => HTMLElement | null; + private _elementFocusedBeforePopup: Element | null = null; + private _hasTakenFocus = false; constructor( host: ReactiveControllerHost & HTMLElement, @@ -20,6 +22,8 @@ export class NotificationPopupController implements ReactiveController { } public hostConnected(): void { + this._elementFocusedBeforePopup = document.activeElement; + window.addEventListener('click', this._handleOutsideInteraction); window.addEventListener('focusin', this._handleOutsideInteraction); @@ -29,10 +33,31 @@ export class NotificationPopupController implements ReactiveController { window.addEventListener('keydown', this._handleKeyDown, { capture: true }); } + public hostUpdated(): void { + const notification = this._getNotificationElement(); + + if (notification && !this._hasTakenFocus) { + this._hasTakenFocus = true; + notification.focus(); + } + } + public hostDisconnected(): void { window.removeEventListener('click', this._handleOutsideInteraction); window.removeEventListener('focusin', this._handleOutsideInteraction); window.removeEventListener('keydown', this._handleKeyDown, { capture: true }); + + this._hasTakenFocus = false; + + // Focus returns to whatever held it before the popup appeared, unless + // something else has taken focus since. + if ( + this._elementFocusedBeforePopup instanceof HTMLElement && + document.activeElement === document.body + ) { + this._elementFocusedBeforePopup.focus(); + } + this._elementFocusedBeforePopup = null; } public dismiss = (): void => { diff --git a/src/components/notification/common-rendering.ts b/src/components/notification/common-rendering.ts index f85fa54a..11d32cb9 100644 --- a/src/components/notification/common-rendering.ts +++ b/src/components/notification/common-rendering.ts @@ -50,6 +50,8 @@ export function renderControl( return html`
@@ -70,11 +71,16 @@ export class AdvancedCameraCardNotification extends LitElement { )}
` : ''} -
+
+
${heading ? renderDetail(heading, 'heading') : ''} ${renderNotificationBody(this.notification, context)} diff --git a/src/localize/languages/en.json b/src/localize/languages/en.json index 4386195c..98a3d28b 100644 --- a/src/localize/languages/en.json +++ b/src/localize/languages/en.json @@ -6,6 +6,7 @@ "common": { "advanced_camera_card": "Advanced Camera Card", "advanced_camera_card_description": "An Advanced Camera Card", + "close": "Close", "event": "Event", "folder": "Folder", "in_progress": "In progress...", diff --git a/src/scss/notification-popup.scss b/src/scss/notification-popup.scss index b8c3ce8d..e52dd6db 100644 --- a/src/scss/notification-popup.scss +++ b/src/scss/notification-popup.scss @@ -113,6 +113,8 @@ display: flex; align-items: center; justify-content: center; + padding: 0; + border: none; border-radius: var(--advanced-camera-card-button-border-radius); background: color-mix( diff --git a/tests/browser/dom.ts b/tests/browser/dom.ts index 9fe04f23..58aa1fd6 100644 --- a/tests/browser/dom.ts +++ b/tests/browser/dom.ts @@ -71,6 +71,25 @@ export const releaseKey = async (key: string): Promise => export const pressTab = async (): Promise => await userEvent.tab(); +/** + * Press Tab until the page reaches the state the caller is waiting for, + * reporting whether it got there. `maximumPresses` is a runaway guard rather + * than a count: a page that never reaches that state fails the caller instead + * of tabbing forever. + */ +export const tabUntil = async ( + isReached: () => boolean, + maximumPresses: number, +): Promise => { + for (let press = 0; press < maximumPresses; ++press) { + await pressTab(); + if (isReached()) { + return true; + } + } + return false; +}; + /** * Click an element with a real pointer, which is the only kind that carries the * browser's own behaviour: the press moves focus, and an element that stops the diff --git a/tests/card-controller/card-element-manager.browser.test.ts b/tests/card-controller/card-element-manager.browser.test.ts index 3628732d..73556091 100644 --- a/tests/card-controller/card-element-manager.browser.test.ts +++ b/tests/card-controller/card-element-manager.browser.test.ts @@ -8,6 +8,7 @@ import { getFocusedElement, pressKey, pressTab, + tabUntil, } from '../browser/dom'; import { MountedCardFactory, type MountedCard } from '../browser/mounted-card'; import { @@ -52,17 +53,7 @@ const mountCard = async (): Promise => { const tabPastCard = async (card: MountedCard): Promise => { const bound = deepQueryAll(card.card, '*').length; - // Tabbing starts from the top of the page, so the first press is the one - // that reaches the card. - await pressTab(); - - for ( - let press = 0; - press < bound && card.card.contains(document.activeElement); - press++ - ) { - await pressTab(); - } + await tabUntil(() => !card.card.contains(document.activeElement), bound); }; /** diff --git a/tests/components-lib/notification/notification-popup-controller.browser.test.ts b/tests/components-lib/notification/notification-popup-controller.browser.test.ts new file mode 100644 index 00000000..46e96c75 --- /dev/null +++ b/tests/components-lib/notification/notification-popup-controller.browser.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it } from 'vitest'; + +import type { Notification } from '../../../src/config/schema/actions/types'; +import { createLogAction } from '../../../src/utils/action'; +import { + clickElement, + deepQuery, + getFocusedElement, + pressKey, + pressTab, +} from '../../browser/dom'; +import { MountedCardFactory, type MountedCard } from '../../browser/mounted-card'; +import { + CARD_INITIALIZED_MESSAGE, + createGenericCameraHASS, + createInitializedAutomation, + createStillImageCardConfig, +} from '../../browser/test-utils'; + +const TRIGGER_ENTITY = 'input_boolean.notify'; + +const BODY_TEXT = 'This camera does not support two-way audio.'; + +const CONTROL_TAPPED_MESSAGE = /control tapped/; + +const NOTIFICATION: Notification = { + heading: { text: 'Two-way audio unavailable' }, + body: { text: BODY_TEXT }, +}; + +const mount = async ( + notification: Notification = NOTIFICATION, +): Promise => { + const hass = createGenericCameraHASS({ entities: { [TRIGGER_ENTITY]: 'off' } }); + return await MountedCardFactory.createFromSource( + createStillImageCardConfig({ + automations: [ + createInitializedAutomation(), + { + triggers: [{ trigger: 'state', entity: TRIGGER_ENTITY, to: 'on' }], + actions: [ + { + action: 'fire-dom-event', + advanced_camera_card_action: 'notification', + notification, + }, + ], + }, + ], + }), + hass, + ); +}; + +const showNotification = async (card: MountedCard): Promise => { + card.setEntityState(TRIGGER_ENTITY, 'on'); + return await card.waitForSelector('.notification'); +}; + +describe('NotificationPopupController', () => { + it('should keep the notification open when its own text is pressed', async () => { + const card = await mount(); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + const elsewhere = document.createElement('button'); + document.body.appendChild(elsewhere); + elsewhere.focus(); + + const notification = await showNotification(card); + + const body = deepQuery(card.card, '.detail.body span'); + expect(body?.textContent).toBe(BODY_TEXT); + if (!body) { + return; + } + + await clickElement(body); + + expect(notification.classList.contains('exiting')).toBe(false); + }); + + it('should dismiss the notification when the page outside it is pressed', async () => { + const card = await mount(); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + const outside = document.createElement('button'); + outside.textContent = 'outside'; + document.body.appendChild(outside); + + const notification = await showNotification(card); + expect(notification.classList.contains('exiting')).toBe(false); + + await clickElement(outside); + + expect(notification.classList.contains('exiting')).toBe(true); + }); + + it('should take focus when notification is opened', async () => { + const card = await mount(); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + const elsewhere = document.createElement('button'); + document.body.appendChild(elsewhere); + elsewhere.focus(); + + const notification = await showNotification(card); + + expect(getFocusedElement()).toBe(notification); + }); + + it('should move focus to its close control on Tab', async () => { + const card = await mount(); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + await showNotification(card); + const close = deepQuery(card.card, 'button.close'); + expect(close).toBeTruthy(); + + await pressTab(); + + expect(getFocusedElement()).toBe(close); + }); + + it('should return focus to where it was once dismissed', async () => { + const card = await mount(); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + const elsewhere = document.createElement('button'); + document.body.appendChild(elsewhere); + elsewhere.focus(); + + await showNotification(card); + expect(getFocusedElement()).not.toBe(elsewhere); + + await pressKey('Escape'); + await card.waitForRender( + () => (deepQuery(card.card, '.notification') ? null : true), + 'the notification to be removed', + ); + + expect(getFocusedElement()).toBe(elsewhere); + }); + + it('should activate a notification control from the keyboard', async () => { + const card = await mount({ + ...NOTIFICATION, + controls: [ + { + icon: 'mdi:refresh', + tooltip: 'Retry', + dismiss: true, + actions: { tap_action: createLogAction(CONTROL_TAPPED_MESSAGE.source) }, + }, + ], + }); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + await showNotification(card); + + await pressTab(); + expect(getFocusedElement()).toBe(deepQuery(card.card, '.notification .control')); + + await pressKey('Enter'); + + await card.console.waitForMessage(CONTROL_TAPPED_MESSAGE); + }); +}); diff --git a/tests/components-lib/notification/notification-popup-controller.test.ts b/tests/components-lib/notification/notification-popup-controller.test.ts index 6ce301f3..de3d04e3 100644 --- a/tests/components-lib/notification/notification-popup-controller.test.ts +++ b/tests/components-lib/notification/notification-popup-controller.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, onTestFinished, vi } from 'vitest'; +import { assert, describe, expect, it, onTestFinished, vi } from 'vitest'; import { NotificationPopupController } from '../../../src/components-lib/notification/notification-popup-controller'; import { POP_OUT_ANIMATION_NAME } from '../../../src/utils/animation'; @@ -79,6 +79,96 @@ describe('NotificationPopupController', () => { }); }); + describe('focus', () => { + // The notification element must be in the document and focusable for the + // controller to be able to move focus to it. + const createFocusablePopup = (): HTMLElement => { + const popup = document.createElement('div'); + popup.setAttribute('tabindex', '-1'); + document.body.appendChild(popup); + return popup; + }; + + it('should take focus when the notification appears', () => { + const popup = createFocusablePopup(); + const { controller } = create(() => popup); + + controller.hostUpdated(); + + expect(document.activeElement).toBe(popup); + }); + + it('should leave focus alone on later updates', () => { + const popup = createFocusablePopup(); + const { controller } = create(() => popup); + controller.hostUpdated(); + + const control = document.createElement('button'); + document.body.appendChild(control); + control.focus(); + + controller.hostUpdated(); + + expect(document.activeElement).toBe(control); + }); + + it('should do nothing when there is no notification element', () => { + const { controller } = create(() => null); + + expect(() => controller.hostUpdated()).not.toThrow(); + }); + + it('should return focus to the element that had it', () => { + const before = document.createElement('button'); + document.body.appendChild(before); + before.focus(); + + const popup = createFocusablePopup(); + const { controller } = create(() => popup); + controller.hostUpdated(); + popup.remove(); + + controller.hostDisconnected(); + + expect(document.activeElement).toBe(before); + }); + + it('should leave focus alone when something else has taken it', () => { + const before = document.createElement('button'); + document.body.appendChild(before); + before.focus(); + + const { controller } = create(); + + const elsewhere = document.createElement('button'); + document.body.appendChild(elsewhere); + elsewhere.focus(); + + controller.hostDisconnected(); + + expect(document.activeElement).toBe(elsewhere); + }); + + it('should do nothing when nothing had focus', () => { + const activeElement = Object.getOwnPropertyDescriptor( + Document.prototype, + 'activeElement', + ); + assert(activeElement); + Object.defineProperty(document, 'activeElement', { + configurable: true, + get: () => null, + }); + onTestFinished(() => { + Object.defineProperty(document, 'activeElement', activeElement); + }); + + const { controller } = create(); + + expect(() => controller.hostDisconnected()).not.toThrow(); + }); + }); + describe('keydown', () => { it('should dismiss and consume the Escape key', () => { const { popup } = create(); diff --git a/tests/components/notification/block.browser.test.ts b/tests/components/notification/block.browser.test.ts new file mode 100644 index 00000000..6fe2ae1c --- /dev/null +++ b/tests/components/notification/block.browser.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest'; + +import type { IssueTriggerEventData } from '../../../src/card-controller/issues/types'; +import { fireAdvancedCameraCardEvent } from '../../../src/utils/fire-advanced-camera-card-event'; +import { getShadowRootHost } from '../../../src/utils/shadow-root'; +import { deepQueryAll, getFocusedElement, tabUntil } from '../../browser/dom'; +import { MountedCardFactory } from '../../browser/mounted-card'; +import { + createGenericCameraHASS, + createStillImageCardConfig, +} from '../../browser/test-utils'; + +const BLOCK_ELEMENT = 'advanced-camera-card-notification-block'; +const MAXIMUM_TAB_PRESSES = 15; + +describe('AdvancedCameraCardNotificationBlock', () => { + it('should let the keyboard reach the retry control on an issue', async () => { + const card = await MountedCardFactory.createFromSource( + createStillImageCardConfig(), + createGenericCameraHASS(), + ); + const views = await card.waitForSelector('advanced-camera-card-views'); + + fireAdvancedCameraCardEvent(views, 'issue:trigger', { + key: 'initialization', + error: new Error('Initialization failed'), + }); + + const control = await card.waitForRender( + () => + deepQueryAll(card.card, '.control').find( + (element) => getShadowRootHost(element)?.localName === BLOCK_ELEMENT, + ) ?? null, + 'a control on the issue block', + ); + + expect( + await tabUntil(() => getFocusedElement() === control, MAXIMUM_TAB_PRESSES), + ).toBe(true); + }); +});