feat(frontend): give a cached view a lifecycle and a keyboard owner

`index.ts` caches primary views and hides them with a class so
scrollTop survives navigation. Nothing else was told: `disconnectedCallback`
never fires for one, so everything written to clean up there never
cleans up. The worst case was not a leak — pressing `s` on Settings
skipped two albums out of the Autotag queue, and `a` on that same live
handler rewrites tags on disk.

- `utils/view-lifecycle.ts` is the missing half: `viewActivated` /
  `viewDeactivated`, with `listenWhileActive`, `intervalWhileActive`
  and `whileActive` torn down on the way out, and an off-screen view
  that does not render. `registerViewAware` gives a shared reactive
  controller the same treatment, because a controller cannot know
  whether its host is a cached view — `ContextMenuController` bound
  three document listeners in `hostConnected`, which for a cached host
  is "forever".
- `services/shortcut-scope.ts` publishes the ambient scope. Resolving
  scope from focus alone was not enough: this app is driven with the
  mouse, focus sits on `<body>`, and a focus-only rule would have made
  the panel keys work only after a click landed inside the panel.
- Global bindings yield to a focused control that owns the key —
  button, select, slider, checkbox, menu, grid row, or anything inside
  an open dialog — so the unmodified single-key bindings stop stealing
  Space and the arrows.
- `utils/roving-grid.ts` gives a card grid one tab stop moved with the
  arrows, since a card per tab stop makes a library-length tab
  sequence.
This commit is contained in:
2026-08-12 01:18:34 -04:00
parent 69ad558a44
commit 7acb197daf
7 changed files with 659 additions and 27 deletions
+45 -22
View File
@@ -53,39 +53,57 @@ export class AppSidebar extends LitElement {
padding: 16px;
}
/* The nav item is a real <button>: it was a bare <li @click>,
which is why tabbing through the whole app reached fourteen
controls and not one of them was navigation (H-5). */
li {
display: block;
}
li button {
display: flex;
width: 100%;
align-items: center;
gap: 10px;
border: none;
border-radius: 5px;
padding: 8px;
cursor: pointer;
background: none;
color: inherit;
font: inherit;
text-align: left;
transition: background-color 0.15s ease;
}
li wa-icon {
li button wa-icon {
font-size: var(--yj-icon-md);
flex-shrink: 0;
width: 20px;
text-align: center;
}
li:hover {
li button:hover {
background-color: var(--yj-bg-elevated, #343a40);
}
li.active {
li button:focus-visible {
outline: 2px solid var(--yj-accent, #ffd43b);
outline-offset: -2px;
}
li button.active {
background-color: var(--yj-bg-overlay, #495057);
}
li p {
li button p {
margin: 0;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
li.drag-hover {
li button.drag-hover {
background-color: var(
--yj-accent-bg-strong,
rgba(255, 212, 59, 0.15)
@@ -94,7 +112,7 @@ export class AppSidebar extends LitElement {
outline-offset: -1px;
}
li p {
li button p {
font-size: var(--yj-text-md);
}
@@ -103,16 +121,16 @@ export class AppSidebar extends LitElement {
padding: 8px;
}
:host(.collapsed) li {
:host(.collapsed) li button {
justify-content: center;
padding: 10px;
}
:host(.collapsed) li p {
:host(.collapsed) li button p {
display: none;
}
:host(.collapsed) li wa-icon {
:host(.collapsed) li button wa-icon {
font-size: var(--yj-icon-md);
}
`];
@@ -199,6 +217,7 @@ export class AppSidebar extends LitElement {
class="resize-handle ${this.isDragging ? 'dragging' : ''}"
@mousedown=${this.handleMouseDown}
></div>
<nav aria-label="Main">
<ul>
${this.navItems.map((item) => {
const classes = [
@@ -213,34 +232,38 @@ export class AppSidebar extends LitElement {
.join(' ');
return html`
<li
class=${classes}
data-testid="nav-${item.id}"
aria-current=${this.activeView === item.id
<li>
<button
type="button"
class=${classes}
data-testid="nav-${item.id}"
aria-current=${this.activeView === item.id
? 'page'
: 'false'}
@click=${() =>
@click=${() =>
this.navigate(item.id)}
@dragover=${(e: DragEvent) =>
@dragover=${(e: DragEvent) =>
this.onNavDragOver(
e,
item.id,
)}
@dragleave=${() =>
@dragleave=${() =>
this.onNavDragLeave(
item.id,
)}
@drop=${(e: DragEvent) =>
@drop=${(e: DragEvent) =>
this.onNavDrop(e)}
>
<wa-icon
name=${item.icon}
></wa-icon>
<p>${item.label}</p>
>
<wa-icon
name=${item.icon}
></wa-icon>
<p>${item.label}</p>
</button>
</li>
`;
})}
</ul>
</nav>
`;
}
@@ -11,6 +11,7 @@
* - Action dispatch to player/queue/nav stores
*/
import { shortcutsStore } from '@store/shortcuts-store';
import { ambientShortcutScope } from './shortcut-scope';
import { playerStore } from '@store/player-store';
import { queueStore } from '@store/queue-store';
import * as Player from '@go/player/Player';
@@ -131,6 +132,128 @@ type ShortcutScope =
| `panel:${string}`
| 'global';
/**
* Elements that own particular keys themselves.
*
* The global bindings are unmodified single keys (Space, arrows, letters)
* — see Decision 1 in plan 007 — so without this a focused button cannot
* be activated with Space and a `<select>` cannot be arrowed through,
* because the service `preventDefault()`s every match. Only text inputs
* were exempt, which is finding H-6.
*/
const ACTIVATION_KEYS = new Set(['Space', 'Enter']);
const ARROW_KEYS = new Set(['Up', 'Down', 'Left', 'Right', 'Home', 'End']);
const LIST_KEYS = new Set([...ARROW_KEYS, ...ACTIVATION_KEYS]);
const SLIDER_KEYS = new Set([...ARROW_KEYS, 'PageUp', 'PageDown']);
/** Roles/tags that consume a key, and which keys they consume. */
function keysOwnedBy(el: Element): ReadonlySet<string> | null {
const tag = el.tagName.toUpperCase();
const role = el.getAttribute('role')?.toLowerCase() ?? '';
if (tag === 'SELECT' || role === 'listbox' || role === 'combobox') {
return LIST_KEYS;
}
if (role === 'menu' || role === 'menuitem' || role === 'menubar') {
return LIST_KEYS;
}
// A grid row or a listbox option moves with the arrow keys, which is
// what makes a track list navigable without a mouse.
if (
role === 'row' ||
role === 'gridcell' ||
role === 'grid' ||
role === 'option' ||
role === 'treeitem'
) {
return ARROW_KEYS;
}
if (tag === 'INPUT') {
const type = (el as HTMLInputElement).type.toLowerCase();
if (type === 'range') return SLIDER_KEYS;
if (type === 'radio') return LIST_KEYS;
if (type === 'checkbox') return ACTIVATION_KEYS;
}
if (role === 'slider' || tag === 'WA-SLIDER') return SLIDER_KEYS;
if (
role === 'checkbox' ||
role === 'switch' ||
role === 'radio' ||
tag === 'WA-CHECKBOX' ||
tag === 'WA-SWITCH'
) {
return ACTIVATION_KEYS;
}
if (
tag === 'BUTTON' ||
tag === 'SUMMARY' ||
tag === 'WA-BUTTON' ||
role === 'button' ||
(tag === 'A' && el.hasAttribute('href'))
) {
return ACTIVATION_KEYS;
}
if (tag === 'WA-SELECT') return LIST_KEYS;
return null;
}
/** Whether an open dialog contains the focused element. A dialog owns
* the whole keyboard while it is up. */
function insideOpenDialog(el: Element | null): boolean {
let walker: Element | null = el;
while (walker) {
const role = walker.getAttribute?.('role')?.toLowerCase();
const tag = walker.tagName.toUpperCase();
if (
role === 'dialog' ||
role === 'alertdialog' ||
((tag === 'DIALOG' || tag === 'WA-DIALOG' || tag === 'WA-DRAWER') &&
walker.hasAttribute('open'))
) {
return true;
}
const parent =
walker.parentElement ??
((walker.getRootNode() as ShadowRoot).host ?? null);
if (parent === walker) break;
walker = parent;
}
return false;
}
/**
* Whether the focused control, rather than the app, should get this key.
*/
function focusedControlOwnsKey(
el: Element | null,
keyStr: string,
): boolean {
if (!el) return false;
// Modified combinations (Ctrl+F, Ctrl+A) are the app's; only the
// unmodified keys are ever contested.
if (keyStr.includes('+')) return false;
if (insideOpenDialog(el)) return true;
return keysOwnedBy(el)?.has(keyStr) ?? false;
}
/**
* Resolve the current shortcut scope based on the focused element.
*
@@ -159,7 +282,13 @@ function resolveScope(deepEl: Element | null): ShortcutScope {
walker = parent ?? null;
}
return 'global';
// Nothing focused inside a panel. Fall back to the panel the active
// view claimed, if any: this app is driven with the mouse, so focus
// usually sits on <body> and a focus-only rule would make panel
// bindings work only after a click landed inside the panel.
const ambient = ambientShortcutScope();
return ambient ? (`panel:${ambient}` as ShortcutScope) : 'global';
}
// ===================================================================
@@ -288,6 +417,23 @@ async function dispatch(action: string): Promise<void> {
);
break;
// Panel-specific: autotag review. The view listens for these
// while it is the view on screen, and for nothing while it is
// not — which is the whole of finding H-1.
case 'autotag.apply':
case 'autotag.skip':
case 'autotag.leave':
case 'autotag.paste':
case 'autotag.search':
case 'autotag.next':
case 'autotag.previous':
document.dispatchEvent(
new CustomEvent(
`shortcut:autotag-${action.slice('autotag.'.length)}`,
),
);
break;
default:
// Unknown action — silently ignore.
break;
@@ -335,6 +481,10 @@ class KeyboardShortcutService {
if (!action) return;
// A focused control that owns this key keeps it: Space activates
// the button you tabbed to, arrows move the select you opened.
if (focusedControlOwnsKey(deepEl, keyStr)) return;
// Found a match — prevent default and dispatch.
e.preventDefault();
void dispatch(action);
+45
View File
@@ -0,0 +1,45 @@
/**
* The ambient shortcut scope: which panel's bindings apply when nothing
* inside a panel has focus.
*
* `resolveScope` in the shortcut service walks up from the focused
* element looking for `data-shortcut-scope`, which is the right answer
* when something is focused — but this app is mostly driven with the
* mouse, so the usual state is that focus sits on `<body>` and the walk
* finds nothing. Without a fallback the panel bindings would only work
* after a click landed inside the panel, which is not the behaviour the
* Autotag page has today and not one worth regressing to.
*
* A claim is held by the *active view* (see `utils/view-lifecycle.ts`),
* so it is released the moment the view leaves the screen — which is
* what stops an off-screen view's keys from firing.
*/
/** Claims, innermost last. A stack rather than a single value so that
* releasing an outer claim out of order cannot resurrect it. */
const claims: Array<{ scope: string }> = [];
/**
* Claim `scope` as the ambient panel scope. Returns the release.
*/
export function claimShortcutScope(scope: string): () => void {
const claim = { scope };
claims.push(claim);
return () => {
const at = claims.indexOf(claim);
if (at >= 0) claims.splice(at, 1);
};
}
/** The innermost claimed scope, or null. */
export function ambientShortcutScope(): string | null {
return claims.length > 0 ? claims[claims.length - 1]!.scope : null;
}
/** Drop every claim. Test-only escape hatch. */
export function resetShortcutScopes(): void {
claims.length = 0;
}
+4 -2
View File
@@ -14,8 +14,10 @@ export interface ShortcutsState {
type Subscriber = () => void;
/** Panel scope prefixes for scope-aware lookups. */
const PANEL_PREFIXES = ['tracklist.'] as const;
/** Panel scope prefixes for scope-aware lookups. An action with one of
* these prefixes is only ever resolved inside its own panel, so it can
* reuse a key the global table also binds. */
const PANEL_PREFIXES = ['tracklist.', 'autotag.'] as const;
/**
* ShortcutsStore holds the current keyboard shortcut bindings
+30 -2
View File
@@ -5,6 +5,8 @@ import type {
} from 'lit';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import { registerViewAware } from './view-lifecycle';
/**
* Host interface for components using the ContextMenuController.
* The host must provide access to the popup elements (typically
@@ -96,7 +98,31 @@ export class ContextMenuController
// LIFECYCLE
// =================================================================
/** Whether the document listeners are currently installed. */
private listening = false;
hostConnected(): void {
// On a cached view, connection is not the right signal: it never
// un-happens, so these listeners would stay on the document for
// the life of the session and close a menu belonging to a page
// the user left. A lifecycle host drives attach/detach instead.
const managed = registerViewAware(this.host, {
onHostActivate: () => this.attach(),
onHostDeactivate: () => this.detach(),
});
if (!managed) this.attach();
}
hostDisconnected(): void {
this.detach();
this.clearSubmenuCloseTimer();
}
private attach(): void {
if (this.listening) return;
this.listening = true;
document.addEventListener(
'click',
this.closeHandler,
@@ -111,7 +137,10 @@ export class ContextMenuController
);
}
hostDisconnected(): void {
private detach(): void {
if (!this.listening) return;
this.listening = false;
document.removeEventListener(
'click',
this.closeHandler,
@@ -124,7 +153,6 @@ export class ContextMenuController
'mousedown',
this.mousedownCloseHandler,
);
this.clearSubmenuCloseTimer();
}
// =================================================================
+133
View File
@@ -0,0 +1,133 @@
/**
* A roving tab stop for a grid of cards.
*
* Every card in the albums, artists and genres grids is
* `role="button" tabindex="0"`, which is reachable but unusable: the tab
* sequence is as long as the library, so getting past the grid means
* pressing Tab a thousand times and getting *into* it lands on card one
* with no way to move but Tab. The convention for a grid is one tab
* stop that the arrow keys move — which is what this is.
*
* The column count is measured from the rendered cards rather than
* computed from the layout config, because all three grids are
* virtualized with a centring `justify` and the arithmetic would be a
* second description of a layout the DOM already knows.
*/
import type { ReactiveController, ReactiveControllerHost } from 'lit';
export interface RovingGridHost extends ReactiveControllerHost {
shadowRoot: ShadowRoot | null;
}
export interface RovingGridOptions {
/** CSS selector matching one card, inside the host's shadow root. */
cardSelector: string;
/** How many cards there are right now. */
count: () => number;
/** Bring an index into view before focusing it — the grids are
* virtualized, so a card outside the window does not exist yet. */
scrollToIndex?: (index: number) => void;
/** Called when the user activates the focused card. */
activate?: (index: number) => void;
}
export class RovingGridController implements ReactiveController {
private host: RovingGridHost;
private opts: RovingGridOptions;
/** The card holding the tab stop. */
private focusedIndex = 0;
constructor(host: RovingGridHost, opts: RovingGridOptions) {
this.host = host;
this.opts = opts;
host.addController(this);
}
hostConnected(): void {}
/** `tabindex` for the card at `index`. */
tabIndexFor(index: number): number {
return index === this.focusedIndex ? 0 : -1;
}
/** Remember where the user is, so tabbing back returns there. */
noteFocus(index: number): void {
if (index === this.focusedIndex) return;
this.focusedIndex = index;
this.host.requestUpdate();
}
/** Keydown handler for the scroll container. */
handleKeydown = (e: KeyboardEvent): void => {
const last = this.opts.count() - 1;
if (last < 0) return;
const columns = this.measureColumns();
let next = this.focusedIndex;
switch (e.key) {
case 'ArrowRight':
next = Math.min(this.focusedIndex + 1, last);
break;
case 'ArrowLeft':
next = Math.max(this.focusedIndex - 1, 0);
break;
case 'ArrowDown':
next = Math.min(this.focusedIndex + columns, last);
break;
case 'ArrowUp':
next = Math.max(this.focusedIndex - columns, 0);
break;
case 'Home':
next = 0;
break;
case 'End':
next = last;
break;
default:
return;
}
e.preventDefault();
e.stopPropagation();
this.focus(next);
};
/** Move the tab stop, scrolling and focusing the card. */
focus(index: number): void {
this.focusedIndex = index;
this.opts.scrollToIndex?.(index);
this.host.requestUpdate();
void this.host.updateComplete.then(() => {
const cards = this.cards();
cards
.find((card) => Number(card.dataset['index']) === index)
?.focus();
});
}
private cards(): HTMLElement[] {
return [
...(this.host.shadowRoot?.querySelectorAll<HTMLElement>(
this.opts.cardSelector,
) ?? []),
];
}
/** Cards sharing a top offset are one row. */
private measureColumns(): number {
const cards = this.cards();
if (cards.length === 0) return 1;
const top = cards[0]!.offsetTop;
const inRow = cards.filter((card) => card.offsetTop === top).length;
return Math.max(1, inRow);
}
}
+251
View File
@@ -0,0 +1,251 @@
/**
* View lifecycle for the cached primary views.
*
* `index.ts` keeps every primary view alive in the DOM and toggles a
* `.view-hidden` class, because that is what preserves `scrollTop`
* across navigation. The cost is that `disconnectedCallback` never
* fires, so a view that is off-screen keeps its document listeners, its
* intervals and its backend subscriptions — which is how a keypress on
* Settings ended up skipping albums out of the Autotag queue
* (`.planning/audits/2026-08-11-ui/hands-on.md`, H-1).
*
* The fix is not to unmount; it is to give the cache the half of the
* lifecycle it never had. A view is *activated* when it is the view on
* screen and *deactivated* when it is not, and everything that listens
* to the world hangs off that pair instead of off connection.
*
* Anything registered through `listenWhileActive`, `intervalWhileActive`
* or `whileActive` is torn down on deactivation, so the common case
* needs no `onViewDeactivate` at all.
*/
import type { LitElement } from 'lit';
import { claimShortcutScope } from '../services/shortcut-scope';
/** The half of the lifecycle `index.ts` drives. */
export interface ViewLifecycle {
/** Called when this view becomes the one on screen. */
viewActivated(): void;
/** Called when it stops being. */
viewDeactivated(): void;
/** Whether it is on screen right now. */
readonly viewActive: boolean;
}
/**
* A reactive controller that wants the view lifecycle rather than the
* connection lifecycle.
*
* A shared controller cannot know whether its host is a cached view, and
* `hostDisconnected` never fires for one — so a controller that binds
* document listeners (the context menu does) leaks them exactly the way
* the views themselves did. Registering here moves it onto the same
* pair of calls; a host that is not a cached view never calls them, so
* the controller keeps its connection-based behaviour there.
*/
export interface ViewAware {
onHostActivate(): void;
onHostDeactivate(): void;
}
interface ViewAwareRegistrar {
addViewAware(aware: ViewAware): void;
}
/**
* Register `aware` with `host` if the host takes part in the lifecycle.
* Returns whether it did, which is also the answer to "should I wait to
* be activated rather than attaching now?".
*/
export function registerViewAware(
host: unknown,
aware: ViewAware,
): boolean {
const registrar = host as Partial<ViewAwareRegistrar>;
if (typeof registrar.addViewAware !== 'function') return false;
registrar.addViewAware(aware);
return true;
}
/** Whether an element participates in the lifecycle. */
export function isViewLifecycle(
el: Element | null,
): el is Element & ViewLifecycle {
return (
!!el &&
typeof (el as Partial<ViewLifecycle>).viewActivated === 'function' &&
typeof (el as Partial<ViewLifecycle>).viewDeactivated === 'function'
);
}
/** Activate an element if it takes part in the lifecycle. */
export function activateView(el: Element | null): void {
if (isViewLifecycle(el)) el.viewActivated();
}
/** Deactivate an element if it takes part in the lifecycle. */
export function deactivateView(el: Element | null): void {
if (isViewLifecycle(el)) el.viewDeactivated();
}
type Constructor<T> = new (...args: any[]) => T;
/**
* Mixin implementing {@link ViewLifecycle}.
*
* Subclasses override `onViewActivate`/`onViewDeactivate` rather than
* `connectedCallback`/`disconnectedCallback` for anything outside their
* own subtree.
*
* Activation is driven by `index.ts`, with one exception: a view that is
* connected without being hidden was put on screen by whoever created it
* (every ephemeral detail view, and every nested use of a view element),
* so it activates itself. `index.ts` creates cached views hidden, which
* is what keeps that rule from firing for them.
*/
export function ViewLifecycleMixin<T extends Constructor<LitElement>>(
Base: T,
) {
abstract class ViewLifecycleElement extends Base implements ViewLifecycle {
/** Panel scope this view claims while it is on screen, if any —
* see `services/shortcut-scope.ts`. Subclasses set it. */
protected shortcutScope: string | null = null;
#active = false;
#disposers: Array<() => void> = [];
#missedUpdate = false;
#aware = new Set<ViewAware>();
get viewActive(): boolean {
return this.#active;
}
override connectedCallback(): void {
super.connectedCallback();
if (!this.classList.contains('view-hidden')) {
this.viewActivated();
}
}
override disconnectedCallback(): void {
this.viewDeactivated();
super.disconnectedCallback();
}
viewActivated(): void {
if (this.#active || !this.isConnected) return;
this.#active = true;
if (this.shortcutScope) {
this.dataset['shortcutScope'] = this.shortcutScope;
this.whileActive(claimShortcutScope(this.shortcutScope));
}
for (const aware of this.#aware) aware.onHostActivate();
this.onViewActivate();
if (this.#missedUpdate) {
this.#missedUpdate = false;
this.requestUpdate();
}
}
viewDeactivated(): void {
if (!this.#active) return;
this.#active = false;
const disposers = this.#disposers;
this.#disposers = [];
for (const dispose of disposers) dispose();
if (this.shortcutScope) delete this.dataset['shortcutScope'];
for (const aware of this.#aware) aware.onHostDeactivate();
this.onViewDeactivate();
}
/** Called by {@link registerViewAware}. */
addViewAware(aware: ViewAware): void {
this.#aware.add(aware);
if (this.#active) aware.onHostActivate();
}
/** Register a teardown to run on deactivation. */
protected whileActive(dispose: () => void): void {
if (this.#active) {
this.#disposers.push(dispose);
} else {
// Registered by something that ran after deactivation
// (an in-flight promise, say) — it has no owner, so run
// it now rather than leak it until the next activation.
dispose();
}
}
/** `addEventListener`, removed again on deactivation. */
protected listenWhileActive<E extends Event = Event>(
target: EventTarget,
type: string,
handler: (event: E) => void,
options?: boolean | AddEventListenerOptions,
): void {
const listener = handler as EventListener;
target.addEventListener(type, listener, options);
this.whileActive(() =>
target.removeEventListener(type, listener, options),
);
}
/** `setInterval`, cleared again on deactivation. */
protected intervalWhileActive(
handler: () => void,
ms: number,
): void {
const id = setInterval(handler, ms);
this.whileActive(() => clearInterval(id));
}
/**
* An off-screen view does not render.
*
* Store subscriptions held by shared reactive controllers keep
* calling `requestUpdate()` on every cached view — so a keystroke
* in the search box re-rendered eleven pages, ten of which are
* not on screen. The update is remembered and replayed on
* activation, so returning to a view still shows current state.
*/
protected override shouldUpdate(
changed: Map<PropertyKey, unknown>,
): boolean {
if (!this.#active && this.hasUpdated) {
this.#missedUpdate = true;
return false;
}
return super.shouldUpdate(changed);
}
/** Called when the view goes on screen. */
protected onViewActivate(): void {}
/** Called when it leaves. Registered teardowns have already
* run; override only for state that is not a disposer. */
protected onViewDeactivate(): void {}
}
return ViewLifecycleElement;
}