From e6bb0eb0e0082a082bc201a627da80dc966265d2 Mon Sep 17 00:00:00 2001 From: Matthijs <115901851+tieskuh@users.noreply.github.com> Date: Sun, 30 Aug 2026 23:04:40 +0200 Subject: [PATCH] fix: claim pointer focus without a visible focus ring (#2733) Since f240646 (#2639, first released in v8.0.0) the card claims focus on any `pointerdown` inside it, so that `key` triggers receive their keyboard events (`keyboard-state-manager.ts`). The claim is a script call, `element.focus({ preventScroll: true })`, and script-initiated focus comes with the browser's focus indicator: after a pointer press while focus was outside the card, the card matches `:focus-visible` and Chromium draws its default ring around the entire card (measured: `outline: auto 1px rgb(238, 238, 238)`, a bright line on a dark dashboard). The ring then persists until focus leaves the card, which users experience as a white border that appears intermittently when they click or tap the card. Ordinary dashboard cards are unaffected because they rely on the browser's native pointer focus, which shows no indicator. Isolated in the same browser, a plain `tabindex` element gains focus from a click without matching `:focus-visible`, while `focus()` from script does match it. v7 did not claim focus at all, so it never showed this. **Change:** pass the intent along with the claim: `element.focus({ preventScroll: true, focusVisible: false })`. This code path only runs for pointer interaction, where no indicator is wanted. Keyboard focus does not pass through it: tabbing to the card keeps its ring, and the `key` trigger support from #2639 is unchanged. Browsers without `FocusOptions.focusVisible` ignore the option and simply keep today's behaviour. (`focusVisible` is not yet in the bundled TypeScript DOM types, hence the small global augmentation.) **Verification:** - Unit test asserts the focus claim carries `focusVisible: false`. - Measured in Chromium 152 on a live dashboard: before, a pointer press on the card leaves it `:focus-visible` with the UA default ring; after, the same press focuses the card without one, and reaching the card with Tab still shows the ring. In the same browser, `focus({ focusVisible: false })` verifiably suppresses `:focus-visible` where a plain `focus()` sets it. - `yarn run test`, `yarn run test:browser` (chromium and firefox), `yarn run lint` and `yarn run typecheck` pass. The webkit browser run fails one focus test in this local environment, identically on unmodified `main`, so it is unrelated to this change. --------- Co-authored-by: dermotduffy --- src/card-controller/keyboard-state-manager.ts | 8 +++-- .../notification-popup-controller.ts | 2 +- src/declarations.d.ts | 6 ++++ .../keyboard-state-manager.browser.test.ts | 26 +++++++++++++++ .../keyboard-state-manager.test.ts | 4 +-- ...ification-popup-controller.browser.test.ts | 32 ++++++++++++++++--- .../notification-popup-controller.test.ts | 16 ++++++++++ 7 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/card-controller/keyboard-state-manager.ts b/src/card-controller/keyboard-state-manager.ts index b9863924..ff539a94 100644 --- a/src/card-controller/keyboard-state-manager.ts +++ b/src/card-controller/keyboard-state-manager.ts @@ -142,8 +142,12 @@ export class KeyboardStateManager { return; } - // Taking focus must not scroll the dashboard to bring the card into view. - element.focus({ preventScroll: true }); + // Taking focus must not scroll the dashboard to bring the card into view, + // nor summon the browser's focus ring: script-initiated focus counts as + // keyboard-like and would draw the ring around the entire card on a plain + // pointer press. Tabbing to the card does not pass through here, so + // keyboard users keep their ring. + element.focus({ preventScroll: true, focusVisible: false }); }; private _handleBlur = (ev: FocusEvent): void => { diff --git a/src/components-lib/notification/notification-popup-controller.ts b/src/components-lib/notification/notification-popup-controller.ts index b7dc010c..4bf03b47 100644 --- a/src/components-lib/notification/notification-popup-controller.ts +++ b/src/components-lib/notification/notification-popup-controller.ts @@ -55,7 +55,7 @@ export class NotificationPopupController implements ReactiveController { this._elementFocusedBeforePopup instanceof HTMLElement && document.activeElement === document.body ) { - this._elementFocusedBeforePopup.focus(); + this._elementFocusedBeforePopup.focus({ focusVisible: false }); } this._elementFocusedBeforePopup = null; } diff --git a/src/declarations.d.ts b/src/declarations.d.ts index 5ed73d12..f488ece9 100644 --- a/src/declarations.d.ts +++ b/src/declarations.d.ts @@ -21,3 +21,9 @@ declare module 'action' { // eslint-disable-next-line @typescript-eslint/no-empty-object-type interface ActionContext {} } + +// The `focusVisible` option is part of the focus specification, but is absent +// from the bundled TypeScript DOM types. +interface FocusOptions { + focusVisible?: boolean; +} diff --git a/tests/card-controller/keyboard-state-manager.browser.test.ts b/tests/card-controller/keyboard-state-manager.browser.test.ts index 754bc0be..8844d26f 100644 --- a/tests/card-controller/keyboard-state-manager.browser.test.ts +++ b/tests/card-controller/keyboard-state-manager.browser.test.ts @@ -14,6 +14,7 @@ import { getFocusedElement, holdKey, pressKey, + pressTab, releaseKey, } from '../browser/dom'; import { @@ -232,6 +233,31 @@ describe('KeyboardStateManager', () => { await card.console.waitForMessage(KEY_MESSAGE); }); + it('should not draw a focus indicator when it takes focus', async () => { + const card = await mountCard(); + + await clickMedia(card); + + expect(getFocusedElement()).toBe(card.card); + + // Focus taken by script counts as keyboard-driven, and the browser rings + // the whole card for it: a bright border around a card the user only + // pressed. + expect(card.card.matches(':focus-visible')).toBe(false); + }); + + it('should draw a focus indicator when it is reached with the keyboard', async () => { + const card = await mountCard(); + + await pressTab(); + + expect(getFocusedElement()).toBe(card.card); + + // The card is in the tab order, and a user who arrives on it that way needs + // to be able to see where they are. + expect(card.card.matches(':focus-visible')).toBe(true); + }); + it('should not scroll the page when it takes focus', async () => { // Well below the window, so the card is out of sight until the page is // scrolled to it. diff --git a/tests/card-controller/keyboard-state-manager.test.ts b/tests/card-controller/keyboard-state-manager.test.ts index 4213416f..0168d123 100644 --- a/tests/card-controller/keyboard-state-manager.test.ts +++ b/tests/card-controller/keyboard-state-manager.test.ts @@ -243,13 +243,13 @@ describe('KeyboardStateManager', () => { }); }); - it('should take focus on pointerdown', () => { + it('should take focus on pointerdown without a visible focus ring', () => { const { element } = createManager(); const focus = vi.spyOn(element, 'focus'); element.dispatchEvent(new Event('pointerdown')); - expect(focus).toHaveBeenCalledWith({ preventScroll: true }); + expect(focus).toHaveBeenCalledWith({ preventScroll: true, focusVisible: false }); }); it('should not take focus on pointerdown when focus is already within the card', () => { diff --git a/tests/components-lib/notification/notification-popup-controller.browser.test.ts b/tests/components-lib/notification/notification-popup-controller.browser.test.ts index 46e96c75..e1ff341f 100644 --- a/tests/components-lib/notification/notification-popup-controller.browser.test.ts +++ b/tests/components-lib/notification/notification-popup-controller.browser.test.ts @@ -57,6 +57,14 @@ const showNotification = async (card: MountedCard): Promise => { return await card.waitForSelector('.notification'); }; +const dismissNotification = async (card: MountedCard): Promise => { + await pressKey('Escape'); + await card.waitForRender( + () => (deepQuery(card.card, '.notification') ? null : true), + 'the notification to be removed', + ); +}; + describe('NotificationPopupController', () => { it('should keep the notification open when its own text is pressed', async () => { const card = await mount(); @@ -132,15 +140,29 @@ describe('NotificationPopupController', () => { 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', - ); + await dismissNotification(card); expect(getFocusedElement()).toBe(elsewhere); }); + it('should return focus without a visible focus ring', async () => { + const card = await mount(); + await card.console.waitForMessage(CARD_INITIALIZED_MESSAGE); + + const elsewhere = document.createElement('button'); + document.body.appendChild(elsewhere); + + // Focused as a pointer press leaves it: with no ring, which is the state + // the return of focus must not change. + elsewhere.focus({ focusVisible: false }); + + await showNotification(card); + await dismissNotification(card); + + expect(getFocusedElement()).toBe(elsewhere); + expect(elsewhere.matches(':focus-visible')).toBe(false); + }); + it('should activate a notification control from the keyboard', async () => { const card = await mount({ ...NOTIFICATION, diff --git a/tests/components-lib/notification/notification-popup-controller.test.ts b/tests/components-lib/notification/notification-popup-controller.test.ts index de3d04e3..607fb9b0 100644 --- a/tests/components-lib/notification/notification-popup-controller.test.ts +++ b/tests/components-lib/notification/notification-popup-controller.test.ts @@ -133,6 +133,22 @@ describe('NotificationPopupController', () => { expect(document.activeElement).toBe(before); }); + it('should return focus without a visible focus ring', () => { + const before = document.createElement('button'); + document.body.appendChild(before); + before.focus(); + const focus = vi.spyOn(before, 'focus'); + + const popup = createFocusablePopup(); + const { controller } = create(() => popup); + controller.hostUpdated(); + popup.remove(); + + controller.hostDisconnected(); + + expect(focus).toHaveBeenCalledWith({ focusVisible: false }); + }); + it('should leave focus alone when something else has taken it', () => { const before = document.createElement('button'); document.body.appendChild(before);