fix: Keep the notification popup open while its text is selected (#2706)

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 => {

src/components/notification/common-rendering.ts

@@ -50,6 +50,8 @@ export function renderControl(
   return html`
     <div
       class="${classMap(classes)}"
+      role="button"
+      tabindex="0"
       title=${control.tooltip ?? ''}
       .actionHandler=${actionHandler({
         hasHold: hasAction(control.actions?.hold_action),

src/components/notification/popup.ts

@@ -53,6 +53,7 @@ export class AdvancedCameraCardNotification extends LitElement {
       <div class="backdrop" @click=${this._popupController.dismiss}></div>
       <div
         class="notification"
+        tabindex="-1"
         ${ref(this._refNotification)}
         @animationend=${this._popupController.handleAnimationEnd}
       >
@@ -70,11 +71,16 @@ export class AdvancedCameraCardNotification extends LitElement {
               )}
             </div>`
           : ''}
-        <div class="close" @click=${this._popupController.dismiss}>
+        <button
+          class="close"
+          title=${localize('common.close')}
+          aria-label=${localize('common.close')}
+          @click=${this._popupController.dismiss}
+        >
           <advanced-camera-card-icon
             .icon=${{ icon: 'mdi:close' }}
           ></advanced-camera-card-icon>
-        </div>
+        </button>
         <div class="details">
           ${heading ? renderDetail(heading, 'heading') : ''}
           ${renderNotificationBody(this.notification, context)}

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...",

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(

tests/browser/dom.ts

@@ -71,6 +71,25 @@ export const releaseKey = async (key: string): Promise<void> =>
 
 export const pressTab = async (): Promise<void> => 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<boolean> => {
+  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

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<MountedCard> => {
 const tabPastCard = async (card: MountedCard): Promise<void> => {
   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);
 };
 
 /**

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<MountedCard> => {
+  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<HTMLElement> => {
+  card.setEntityState(TRIGGER_ENTITY, 'on');
+  return await card.waitForSelector<HTMLElement>('.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<HTMLElement>(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<HTMLElement>(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);
+  });
+});

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();

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<IssueTriggerEventData>(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);
+  });
+});