fix: Restore keyboard focus for key triggers (#2634) (#2639)

- Closes #2634
This commit is contained in:
Dermot Duffy
2026-07-30 20:52:54 -07:00
committed by GitHub
parent a736f3dfaf
commit f24064689a
8 changed files with 192 additions and 7 deletions
@@ -98,6 +98,10 @@ export class CardElementManager {
this._element.toggleAttribute('panel', isCardInPanel(this._element));
this._element.toggleAttribute('casted', isBeingCasted());
// The card must be focusable in its own right, so that it can receive
// keyboard events and be reached by tabbing.
this._element.setAttribute('tabindex', '0');
this._api.getFullscreenManager().connect();
this._element.addEventListener(
+34 -1
View File
@@ -1,5 +1,6 @@
import { isEqual } from 'lodash-es';
import { isFocusWithin } from '../utils/focus';
import type { CardKeyboardStateAPI, KeysState } from './types';
export class KeyboardStateManager {
@@ -15,6 +16,12 @@ export class KeyboardStateManager {
element.addEventListener('keydown', this._handleKeydown);
element.addEventListener('keyup', this._handleKeyup);
element.addEventListener('blur', this._handleBlur);
// Must capture, since elements within the card stop pointer events propagating
// (e.g. the zoom controller during a pan).
element.addEventListener('pointerdown', this._handlePointerdown, {
capture: true,
});
}
public uninitialize(): void {
@@ -22,6 +29,9 @@ export class KeyboardStateManager {
element.removeEventListener('keydown', this._handleKeydown);
element.removeEventListener('keyup', this._handleKeyup);
element.removeEventListener('blur', this._handleBlur);
element.removeEventListener('pointerdown', this._handlePointerdown, {
capture: true,
});
// Clear state on disconnect. Without listeners the card cannot know
// whether a key was released while detached, and stale "down" state
@@ -54,7 +64,30 @@ export class KeyboardStateManager {
}
};
private _handleBlur = (): void => {
// Keys are only received when the card or something within it has focus, so
// focus is claimed on interaction. The card itself is focused rather than a
// child, as a child may be removed by the next render and take focus with it.
private _handlePointerdown = (): void => {
const element = this._api.getCardElementManager().getElement();
// Focus already inside the card is left where it is, as taking it would blur
// whatever the user is interacting with (e.g. a text field being typed in).
if (isFocusWithin(element)) {
return;
}
// Taking focus must not scroll the dashboard to bring the card into view.
element.focus({ preventScroll: true });
};
private _handleBlur = (ev: FocusEvent): void => {
// 'relatedTarget' would be the card element due to event retargeting --
// focus gained by another element within the card will be reported as to
// the card itself at this level.
if (ev.relatedTarget === this._api.getCardElementManager().getElement()) {
return;
}
if (Object.keys(this._state).length) {
// State is emptied if the element loses focus.
this._state = {};
-1
View File
@@ -454,7 +454,6 @@ class AdvancedCameraCard extends LitElement {
@advanced-camera-card:media:pause=${
() => this.requestUpdate() /* Refresh play/pause menu button */
}
@advanced-camera-card:focus=${() => this.focus()}
@advanced-camera-card:notification:dismiss=${() =>
this._controller.getNotificationManager().reset()}
>
@@ -73,11 +73,6 @@ export class ZoomController {
// handler in the viewer).
if (!this._allowClick) {
ev.stopPropagation();
// Even though the click is stopped,the card still needs to gain focus so
// that keyboard shortcuts will work immediately after the card is clicked
// upon.
fireAdvancedCameraCardEvent(this._element, 'focus');
}
this._allowClick = true;
};
+14
View File
@@ -0,0 +1,14 @@
/**
* Determine whether focus currently rests on an element, or on any of its
* descendants (including those inside nested shadow roots).
*/
export const isFocusWithin = (element: Element): boolean => {
const root = element.getRootNode();
// Focus is reported per tree, with a shadow host standing in for whatever is
// focused inside it, so the element's own tree is the one that will name it.
const active =
root instanceof Document || root instanceof ShadowRoot ? root.activeElement : null;
return !!active && element.contains(active);
};
@@ -123,6 +123,7 @@ describe('CardElementManager', () => {
expect(element.getAttribute('panel')).toBeNull();
expect(element.getAttribute('casted')).toBeNull();
expect(element.getAttribute('tabindex')).toBe('0');
expect(api.getFullscreenManager().connect).toHaveBeenCalled();
expect(addEventListener).toHaveBeenCalledWith(
@@ -173,6 +174,7 @@ describe('CardElementManager', () => {
const element = createCardHTMLElement();
element.setAttribute('panel', '');
element.setAttribute('casted', '');
element.setAttribute('tabindex', '0');
const removeEventListener = vi.fn();
element.removeEventListener = removeEventListener;
@@ -190,6 +192,7 @@ describe('CardElementManager', () => {
expect(element.getAttribute('panel')).toBeNull();
expect(element.getAttribute('casted')).toBeNull();
expect(element.getAttribute('tabindex')).toBeNull();
expect(api.getMediaLoadedInfoManager().clear).toHaveBeenCalled();
expect(api.getFullscreenManager().disconnect).toHaveBeenCalled();
@@ -72,6 +72,73 @@ describe('KeyboardStateManager', () => {
});
});
it('should not clear state when focus moves within the card', () => {
const api = createCardAPI();
const element = createLitElement();
vi.mocked(api.getCardElementManager().getElement).mockReturnValue(element);
const manager = new KeyboardStateManager(api);
manager.initialize();
element.dispatchEvent(new KeyboardEvent('keydown', { key: 'a' }));
element.dispatchEvent(new FocusEvent('blur', { relatedTarget: element }));
expect(api.getConditionStateManager().setState).toHaveBeenCalledTimes(1);
expect(api.getConditionStateManager().setState).toHaveBeenLastCalledWith({
keys: {
a: { state: 'down', ctrl: false, alt: false, meta: false, shift: false },
},
});
});
it('should take focus on pointerdown', () => {
const api = createCardAPI();
const element = createLitElement();
vi.mocked(api.getCardElementManager().getElement).mockReturnValue(element);
const focus = vi.spyOn(element, 'focus');
const manager = new KeyboardStateManager(api);
manager.initialize();
element.dispatchEvent(new Event('pointerdown'));
expect(focus).toHaveBeenCalledWith({ preventScroll: true });
});
it('should not take focus on pointerdown when focus is already within the card', () => {
const api = createCardAPI();
const element = createLitElement();
vi.mocked(api.getCardElementManager().getElement).mockReturnValue(element);
document.body.append(element);
const child = document.createElement('div');
child.setAttribute('tabindex', '0');
element.attachShadow({ mode: 'open' }).appendChild(child);
child.focus();
const focus = vi.spyOn(element, 'focus');
const manager = new KeyboardStateManager(api);
manager.initialize();
element.dispatchEvent(new Event('pointerdown'));
expect(focus).not.toHaveBeenCalled();
element.remove();
});
it('should not take focus on pointerdown after uninitialization', () => {
const api = createCardAPI();
const element = createLitElement();
vi.mocked(api.getCardElementManager().getElement).mockReturnValue(element);
const focus = vi.spyOn(element, 'focus');
const manager = new KeyboardStateManager(api);
manager.initialize();
manager.uninitialize();
element.dispatchEvent(new Event('pointerdown'));
expect(focus).not.toHaveBeenCalled();
});
it('should not act after uninitialization', () => {
const api = createCardAPI();
const element = createLitElement();
+70
View File
@@ -0,0 +1,70 @@
import { afterEach, describe, expect, it } from 'vitest';
import { isFocusWithin } from '../../src/utils/focus';
// @vitest-environment jsdom
describe('isFocusWithin', () => {
const createFocusableElement = (parent: Node): HTMLElement => {
const element = document.createElement('div');
element.setAttribute('tabindex', '0');
parent.appendChild(element);
return element;
};
afterEach(() => {
document.body.replaceChildren();
});
it('should return false without focus', () => {
const element = createFocusableElement(document.body);
expect(isFocusWithin(element)).toBeFalsy();
});
it('should return false when focus is elsewhere', () => {
const element = createFocusableElement(document.body);
createFocusableElement(document.body).focus();
expect(isFocusWithin(element)).toBeFalsy();
});
it('should return true when the element itself has focus', () => {
const element = createFocusableElement(document.body);
element.focus();
expect(isFocusWithin(element)).toBeTruthy();
});
it('should return true when a child has focus', () => {
const element = createFocusableElement(document.body);
createFocusableElement(element).focus();
expect(isFocusWithin(element)).toBeTruthy();
});
it('should return true when a child within nested shadow roots has focus', () => {
const element = createFocusableElement(document.body);
const outerShadow = element.attachShadow({ mode: 'open' });
const inner = createFocusableElement(outerShadow);
const innerShadow = inner.attachShadow({ mode: 'open' });
createFocusableElement(innerShadow).focus();
expect(isFocusWithin(element)).toBeTruthy();
});
it('should return true when the element is itself within a shadow root', () => {
const host = createFocusableElement(document.body);
const element = createFocusableElement(host.attachShadow({ mode: 'open' }));
createFocusableElement(element.attachShadow({ mode: 'open' })).focus();
expect(isFocusWithin(element)).toBeTruthy();
});
it('should return false when the element is not attached to a document', () => {
const element = document.createElement('div');
expect(isFocusWithin(element)).toBeFalsy();
});
});