feat(shortcuts): tell the key story once, and give the arrows back
Decision 1 keeps the unmodified single-key bindings, and Settings was the only place they were written down — three of the four categories of them, because config-page listed the categories by hand, so the autotag keys were written down nowhere at all. `?` now opens an overlay from anywhere the app owns the keyboard, and both surfaces read one table (services/shortcut-meta.ts, moved out of config-page's private static). The other half is the same explanation from the other side. Phase 1 gave the arrow keys to the grid, correctly — but all six of them, and no list in this app moves horizontally: track-list's own handler and utils/roving-rows both take Up/Down/Home/End and ignore Left/Right. So seeking stopped working from a focused row and nothing gained the keys. Reproduced in the running app: two ArrowRights on a focused track row, zero Player.Seek calls, against one per press from the body. A shifted character no longer reports Shift, so the binding is `?` and not `Shift+?` — the character already carries the shift, and a layout where it does not is a layout where "Shift+?" is wrong anyway.
This commit is contained in:
@@ -29,8 +29,9 @@ func DefaultBindings() map[string]string {
|
||||
"nav.searchAlt": "Ctrl+F",
|
||||
"nav.queue": "Q",
|
||||
|
||||
// App actions (Global scope, Ctrl modifier)
|
||||
// App actions
|
||||
"app.selectAll": "Ctrl+A",
|
||||
"app.shortcuts": "?",
|
||||
|
||||
// Panel-specific (track list). There is no `tracklist.delete`:
|
||||
// it was bound to Delete and advertised in Settings as
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
import {
|
||||
test,
|
||||
expect,
|
||||
callBinding,
|
||||
resetEvents,
|
||||
waitForEvent,
|
||||
LONG_TRACK,
|
||||
} from '../support/fixtures.js';
|
||||
import type { Page } from '@playwright/test';
|
||||
|
||||
/**
|
||||
* Plan 007 phase 5: the key story, told once.
|
||||
*
|
||||
* Decision 1 keeps the unmodified single-key bindings, and until now
|
||||
* Settings was the only place they were written down — three of the
|
||||
* four categories of them, so the autotag keys were written down
|
||||
* nowhere. `?` opens the overlay from anywhere the app owns the
|
||||
* keyboard.
|
||||
*
|
||||
* The other half is the same explanation from the other side: Phase 1
|
||||
* gave the arrow keys to the grid, correctly, but all six of them —
|
||||
* and no list in this app moves horizontally, so seeking stopped
|
||||
* working from a focused row and nothing gained the keys. Reproduced
|
||||
* in the running app before the fix: two ArrowRights on a focused
|
||||
* track row produced zero `Player.Seek` calls, against one per press
|
||||
* with focus on the body.
|
||||
*/
|
||||
test.describe('the shortcuts overlay', () => {
|
||||
// Two things about locating a `wa-dialog`, both found here:
|
||||
//
|
||||
// - the host is `display: contents`, so the element carrying the
|
||||
// testid always reports hidden; what is visible is the native
|
||||
// `<dialog>` inside its shadow root, and
|
||||
// - that dialog has **no accessible name**. Web Awesome renders the
|
||||
// `label` into an `<h2 id="title">` in the same shadow root and
|
||||
// never points `aria-labelledby` at it, so every dialog in this
|
||||
// app is an unnamed dialog to a screen reader. Not fixed here:
|
||||
// it is eight call sites and a helper, and it is worth doing on
|
||||
// purpose rather than as a side effect of a spec.
|
||||
const overlay = (app: Page) =>
|
||||
app.locator('shortcuts-overlay').getByRole('dialog');
|
||||
|
||||
test('? opens it, and it names the keys', async ({ app }) => {
|
||||
await app.keyboard.press('?');
|
||||
|
||||
await expect(overlay(app)).toBeVisible();
|
||||
// The rows are slotted, so they are in the overlay's own shadow
|
||||
// root rather than inside the native `<dialog>`.
|
||||
const content = app.locator('shortcuts-overlay');
|
||||
|
||||
await expect(content).toContainText('Play / Pause');
|
||||
// Autotag's bindings are in the table Settings renders three
|
||||
// quarters of.
|
||||
await expect(content).toContainText('Apply Match');
|
||||
});
|
||||
|
||||
test('Escape closes it', async ({ app }) => {
|
||||
await app.keyboard.press('?');
|
||||
await expect(overlay(app)).toBeVisible();
|
||||
|
||||
// Not a toggle: a dialog owns every unmodified key while it is up,
|
||||
// so a second `?` never reaches the shortcut service. Escape is
|
||||
// what closes a dialog here, and `wa-dialog` brings it.
|
||||
await app.keyboard.press('Escape');
|
||||
await expect(overlay(app)).toBeHidden();
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('seeking from a focused track row', () => {
|
||||
test('Left and Right seek; Up and Down still move the row', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await callBinding(app, 'queue.Queue.Clear');
|
||||
await resetEvents(app);
|
||||
|
||||
await app
|
||||
.getByTestId('track-row')
|
||||
.filter({ hasText: LONG_TRACK })
|
||||
.first()
|
||||
.dblclick();
|
||||
await waitForEvent(app, 'TrackChanged');
|
||||
|
||||
// Focus the roving row, which is what took the arrows.
|
||||
await app.evaluate(() => {
|
||||
const list = document.querySelector('track-list');
|
||||
const row = list?.shadowRoot?.querySelector<HTMLElement>(
|
||||
'[role="row"][tabindex="0"]',
|
||||
);
|
||||
|
||||
row?.focus();
|
||||
});
|
||||
|
||||
const position = async (): Promise<number> =>
|
||||
(await callBinding(app, 'player.Player.CurrentPositionSeconds')) as number;
|
||||
|
||||
const before = await position();
|
||||
|
||||
for (let i = 0; i < 3; i++) await app.keyboard.press('ArrowRight');
|
||||
|
||||
// Three 5 s steps, against a clock that advances 1 s per second:
|
||||
// the assertion is a jump the passage of time cannot account for.
|
||||
await expect.poll(position).toBeGreaterThan(before + 8);
|
||||
|
||||
// …and the list still moves vertically, which is what the arrows
|
||||
// were given to the grid for.
|
||||
const focusedBefore = await app.evaluate(
|
||||
() =>
|
||||
(
|
||||
document.querySelector('track-list') as unknown as {
|
||||
focusedIndex: number;
|
||||
}
|
||||
).focusedIndex,
|
||||
);
|
||||
|
||||
await app.keyboard.press('ArrowDown');
|
||||
|
||||
await expect
|
||||
.poll(() =>
|
||||
app.evaluate(
|
||||
() =>
|
||||
(
|
||||
document.querySelector('track-list') as unknown as {
|
||||
focusedIndex: number;
|
||||
}
|
||||
).focusedIndex,
|
||||
),
|
||||
)
|
||||
.toBe(focusedBefore + 1);
|
||||
});
|
||||
});
|
||||
@@ -77,9 +77,12 @@ async function readClocks(
|
||||
* Move focus off the track row.
|
||||
*
|
||||
* Phase 1 made rows a real grid with roving tabindex, and global
|
||||
* single-key bindings yield to a focused control that owns the key —
|
||||
* so with a row focused the arrows navigate the list rather than
|
||||
* seeking, which is correct and is not what this spec is about.
|
||||
* single-key bindings yield to a focused control that owns the key.
|
||||
* A row owns the *vertical* arrows only now (Phase 5), so Left/Right
|
||||
* seek from a focused row too and this is no longer load-bearing — it
|
||||
* stays because this spec is about the clock, not about scope
|
||||
* resolution, and it should keep measuring the same thing if that
|
||||
* rule changes again.
|
||||
*/
|
||||
async function blurDeepActive(app: Page): Promise<void> {
|
||||
await app.evaluate(() => {
|
||||
|
||||
@@ -37,6 +37,7 @@
|
||||
</footer>
|
||||
<first-run-wizard></first-run-wizard>
|
||||
<notification-host></notification-host>
|
||||
<shortcuts-overlay></shortcuts-overlay>
|
||||
</body>
|
||||
|
||||
</html>
|
||||
|
||||
@@ -28,6 +28,10 @@ import '@components/first-run-wizard/first-run-wizard.ts';
|
||||
import '@components/notifications/notification-host.ts';
|
||||
import '@components/notifications/inline-notice.ts';
|
||||
import '@components/confirm-dialog/confirm-dialog.ts';
|
||||
// The `?` overlay: help, so it is eager for the same reason the failure
|
||||
// surface is — the moment it is asked for is the moment the user does
|
||||
// not know what is going on. It costs a dialog and a table.
|
||||
import '@components/shortcuts-overlay/shortcuts-overlay.ts';
|
||||
import '@components/jobs/job-indicator.ts';
|
||||
import '@awesome.me/webawesome/dist/styles/themes/default.css';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
|
||||
@@ -25,6 +25,10 @@ import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { GetAllPlaylists } from '@go/playlist/Service';
|
||||
import type { playlist } from '@go/models';
|
||||
import { Events } from '../../events';
|
||||
import {
|
||||
SHORTCUT_CATEGORIES,
|
||||
SHORTCUT_META,
|
||||
} from '../../services/shortcut-meta';
|
||||
import { ViewLifecycleMixin } from '../../utils/view-lifecycle';
|
||||
import type { ConfigFieldChangeEvent } from './config-field';
|
||||
import type { BackgroundShade } from '@store/theme-store';
|
||||
@@ -63,150 +67,6 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
// --- Shortcuts controller ---
|
||||
private shortcutsCtrl = new ShortcutsController(this);
|
||||
|
||||
// --- Shortcut metadata for UI grouping ---
|
||||
private static readonly SHORTCUT_META: Record<
|
||||
string,
|
||||
{
|
||||
label: string;
|
||||
category: string;
|
||||
scope: string;
|
||||
defaultKey: string;
|
||||
}
|
||||
> = {
|
||||
'player.playPause': {
|
||||
label: 'Play / Pause',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Space',
|
||||
},
|
||||
'player.next': {
|
||||
label: 'Next Track',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'N',
|
||||
},
|
||||
'player.previous': {
|
||||
label: 'Previous Track',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'P',
|
||||
},
|
||||
'player.volumeUp': {
|
||||
label: 'Volume Up',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Up',
|
||||
},
|
||||
'player.volumeDown': {
|
||||
label: 'Volume Down',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Down',
|
||||
},
|
||||
'player.seekForward': {
|
||||
label: 'Seek Forward',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Right',
|
||||
},
|
||||
'player.seekBack': {
|
||||
label: 'Seek Back',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Left',
|
||||
},
|
||||
'player.shuffle': {
|
||||
label: 'Toggle Shuffle',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'S',
|
||||
},
|
||||
'player.repeat': {
|
||||
label: 'Cycle Repeat',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'R',
|
||||
},
|
||||
'player.mute': {
|
||||
label: 'Toggle Mute',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'M',
|
||||
},
|
||||
'nav.search': {
|
||||
label: 'Focus Search',
|
||||
category: 'Navigation',
|
||||
scope: 'global',
|
||||
defaultKey: '/',
|
||||
},
|
||||
'nav.searchAlt': {
|
||||
label: 'Focus Search (Alt)',
|
||||
category: 'Navigation',
|
||||
scope: 'global',
|
||||
defaultKey: 'Ctrl+F',
|
||||
},
|
||||
'nav.queue': {
|
||||
label: 'Toggle Queue',
|
||||
category: 'Navigation',
|
||||
scope: 'global',
|
||||
defaultKey: 'Q',
|
||||
},
|
||||
'app.selectAll': {
|
||||
label: 'Select All',
|
||||
category: 'App',
|
||||
scope: 'global',
|
||||
defaultKey: 'Ctrl+A',
|
||||
},
|
||||
'tracklist.play': {
|
||||
label: 'Play Selected',
|
||||
category: 'Navigation',
|
||||
scope: 'panel:track-list',
|
||||
defaultKey: 'Enter',
|
||||
},
|
||||
'autotag.apply': {
|
||||
label: 'Apply Match',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'A',
|
||||
},
|
||||
'autotag.skip': {
|
||||
label: 'Skip Folder',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'S',
|
||||
},
|
||||
'autotag.leave': {
|
||||
label: 'Leave As Is',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'L',
|
||||
},
|
||||
'autotag.paste': {
|
||||
label: 'Paste Release URL',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'U',
|
||||
},
|
||||
'autotag.search': {
|
||||
label: 'Search Candidates',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'F',
|
||||
},
|
||||
'autotag.next': {
|
||||
label: 'Next Folder',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'Down',
|
||||
},
|
||||
'autotag.previous': {
|
||||
label: 'Previous Folder',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'Up',
|
||||
},
|
||||
};
|
||||
|
||||
// --- Now Playing state ---
|
||||
@state() private scrollMode = 'hover';
|
||||
|
||||
@@ -1338,7 +1198,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
const { action, key } = e.detail;
|
||||
|
||||
// Check for conflict — find any other action with the same key in the same or overlapping scope
|
||||
const meta = ConfigPage.SHORTCUT_META[action];
|
||||
const meta = SHORTCUT_META[action];
|
||||
const conflict = shortcutsStore.findConflict(
|
||||
key,
|
||||
meta?.scope ?? 'global',
|
||||
@@ -1901,11 +1761,10 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
private renderShortcutsSection() {
|
||||
const bindings =
|
||||
this.shortcutsCtrl.state.bindings;
|
||||
const categories = [
|
||||
'Player',
|
||||
'Navigation',
|
||||
'App',
|
||||
];
|
||||
// All four, from the shared table: the Autotag bindings are
|
||||
// persisted and rebindable like any other, and listing three of
|
||||
// four categories is how they came to be written down nowhere.
|
||||
const categories = SHORTCUT_CATEGORIES;
|
||||
|
||||
return html`
|
||||
<config-section
|
||||
@@ -1914,7 +1773,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
>
|
||||
${categories.map((cat) => {
|
||||
const actions = Object.entries(
|
||||
ConfigPage.SHORTCUT_META,
|
||||
SHORTCUT_META,
|
||||
).filter(
|
||||
([, meta]) =>
|
||||
meta.category === cat,
|
||||
@@ -1990,8 +1849,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
>
|
||||
is already bound to
|
||||
<strong
|
||||
>${ConfigPage
|
||||
.SHORTCUT_META[
|
||||
>${SHORTCUT_META[
|
||||
this
|
||||
.shortcutConflict
|
||||
.existingAction
|
||||
|
||||
@@ -0,0 +1,249 @@
|
||||
/**
|
||||
* The key story, told once.
|
||||
*
|
||||
* The single-key global bindings are staying (plan 007, Decision 1),
|
||||
* and until now Settings was the only place they were written down —
|
||||
* three of the four categories of them, at that, so the autotag keys
|
||||
* were written down nowhere. `?` opens this from anywhere the app owns
|
||||
* the keyboard.
|
||||
*
|
||||
* It reads `services/shortcut-meta` for the labels and the *store* for
|
||||
* the keys, so a rebound key shows its new binding rather than the
|
||||
* default it shipped with.
|
||||
*
|
||||
* It is a `wa-dialog` for the reason every dialog in this app is: the
|
||||
* focus trap, Escape and focus restore come with it. Being help rather
|
||||
* than a question, it is a dialog in a host template rather than a
|
||||
* `confirmAction()` call.
|
||||
*/
|
||||
import { LitElement, css, html, nothing } from 'lit';
|
||||
import { customElement, query, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import {
|
||||
SHORTCUT_CATEGORIES,
|
||||
SHORTCUT_META,
|
||||
} from '../../services/shortcut-meta';
|
||||
import { shortcutsStore } from '@store/shortcuts-store';
|
||||
|
||||
/** How a key string reads to a person: `Ctrl+F` is two keys. */
|
||||
function keyParts(key: string): string[] {
|
||||
return key.split('+');
|
||||
}
|
||||
|
||||
/** Where a binding applies, in the user's terms. A panel binding that
|
||||
* does not say so reads as broken everywhere else. */
|
||||
function scopeNote(scope: string): string {
|
||||
if (scope === 'panel:track-list') return 'in the track list';
|
||||
if (scope === 'panel:autotag') return 'on the Autotag page';
|
||||
|
||||
return '';
|
||||
}
|
||||
|
||||
@customElement('shortcuts-overlay')
|
||||
export class ShortcutsOverlay extends LitElement {
|
||||
@query('wa-dialog') private dialog?: HTMLElement & { open: boolean };
|
||||
|
||||
@state() private isOpen = false;
|
||||
|
||||
private unsubscribe: (() => void) | null = null;
|
||||
|
||||
static override styles = [
|
||||
designTokens,
|
||||
css`
|
||||
:host {
|
||||
display: contents;
|
||||
}
|
||||
|
||||
wa-dialog::part(dialog) {
|
||||
background: var(--yj-bg-surface, #212529);
|
||||
color: var(--yj-text-primary, #fff);
|
||||
}
|
||||
|
||||
.category {
|
||||
margin-bottom: 1.25em;
|
||||
}
|
||||
|
||||
.category:last-of-type {
|
||||
margin-bottom: 0;
|
||||
}
|
||||
|
||||
h3 {
|
||||
margin: 0 0 0.5em;
|
||||
font-size: var(--yj-text-sm, 0.8125rem);
|
||||
font-weight: 700;
|
||||
text-transform: uppercase;
|
||||
letter-spacing: 0.06em;
|
||||
color: var(--yj-text-secondary, #b3b3b3);
|
||||
}
|
||||
|
||||
.row {
|
||||
display: flex;
|
||||
align-items: baseline;
|
||||
justify-content: space-between;
|
||||
gap: 1em;
|
||||
padding: 0.25em 0;
|
||||
}
|
||||
|
||||
.label {
|
||||
font-size: var(--yj-text-sm, 0.8125rem);
|
||||
}
|
||||
|
||||
.scope {
|
||||
color: var(--yj-text-tertiary, #888);
|
||||
}
|
||||
|
||||
.keys {
|
||||
display: flex;
|
||||
gap: 0.25em;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
kbd {
|
||||
background: var(--yj-bg-elevated, #343a40);
|
||||
border: 1px solid var(--yj-border, #495057);
|
||||
border-radius: 4px;
|
||||
padding: 0.1em 0.45em;
|
||||
font-family: inherit;
|
||||
font-size: var(--yj-text-xs, 0.6875rem);
|
||||
}
|
||||
|
||||
.footnote {
|
||||
margin: 1em 0 0;
|
||||
font-size: var(--yj-text-xs, 0.6875rem);
|
||||
color: var(--yj-text-tertiary, #888);
|
||||
}
|
||||
`,
|
||||
];
|
||||
|
||||
override connectedCallback(): void {
|
||||
super.connectedCallback();
|
||||
document.addEventListener('shortcut:app-shortcuts', this.open);
|
||||
// The keys come from the backend, so the first open can arrive
|
||||
// before they do.
|
||||
this.unsubscribe = shortcutsStore.subscribe(() =>
|
||||
this.requestUpdate(),
|
||||
);
|
||||
}
|
||||
|
||||
override disconnectedCallback(): void {
|
||||
super.disconnectedCallback();
|
||||
document.removeEventListener('shortcut:app-shortcuts', this.open);
|
||||
this.unsubscribe?.();
|
||||
this.unsubscribe = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* `?` opens; Escape closes, as it does for every dialog here.
|
||||
*
|
||||
* It is deliberately not a toggle. A dialog owns the whole keyboard
|
||||
* while it is up — `focusedControlOwnsKey` yields every unmodified
|
||||
* key to anything inside one — so a second `?` never reaches the
|
||||
* service, and a toggle would be a promise the shortcut layer
|
||||
* cannot keep. Written after watching an e2e spec assert it and
|
||||
* fail.
|
||||
*/
|
||||
private open = (): void => {
|
||||
if (this.isOpen) return;
|
||||
|
||||
this.isOpen = true;
|
||||
void this.updateComplete.then(() => {
|
||||
if (this.dialog) this.dialog.open = true;
|
||||
});
|
||||
};
|
||||
|
||||
private close(): void {
|
||||
if (this.dialog) this.dialog.open = false;
|
||||
this.isOpen = false;
|
||||
}
|
||||
|
||||
override render() {
|
||||
if (!this.isOpen) return nothing;
|
||||
|
||||
const bindings = shortcutsStore.getBindings();
|
||||
|
||||
return html`
|
||||
<wa-dialog
|
||||
label="Keyboard Shortcuts"
|
||||
data-testid="shortcuts-overlay"
|
||||
@wa-hide=${() => this.close()}
|
||||
>
|
||||
${SHORTCUT_CATEGORIES.map((category) => {
|
||||
const rows = Object.entries(SHORTCUT_META).filter(
|
||||
([, meta]) => meta.category === category,
|
||||
);
|
||||
|
||||
if (rows.length === 0) return nothing;
|
||||
|
||||
// A note every row in the category repeats is a
|
||||
// note about the category. "— on the Autotag page"
|
||||
// seven times is noise; once is the heading.
|
||||
const first = scopeNote(rows[0]![1].scope);
|
||||
const uniform = rows.every(
|
||||
([, meta]) => scopeNote(meta.scope) === first,
|
||||
);
|
||||
// …and a note the *heading* already says is not a
|
||||
// note either: "Autotag — on the Autotag page".
|
||||
const shared =
|
||||
uniform &&
|
||||
!first
|
||||
.toLowerCase()
|
||||
.includes(category.toLowerCase())
|
||||
? first
|
||||
: '';
|
||||
|
||||
return html`
|
||||
<div class="category">
|
||||
<h3>
|
||||
${category}
|
||||
${shared
|
||||
? html`<span class="scope"
|
||||
>— ${shared}</span
|
||||
>`
|
||||
: nothing}
|
||||
</h3>
|
||||
${rows.map(([action, meta]) => {
|
||||
const key =
|
||||
bindings.get(action) ?? meta.defaultKey;
|
||||
const note =
|
||||
shared || uniform
|
||||
? ''
|
||||
: scopeNote(meta.scope);
|
||||
|
||||
return html`
|
||||
<div class="row">
|
||||
<span class="label">
|
||||
${meta.label}
|
||||
${note
|
||||
? html`<span class="scope"
|
||||
>— ${note}</span
|
||||
>`
|
||||
: nothing}
|
||||
</span>
|
||||
<span class="keys">
|
||||
${keyParts(key).map(
|
||||
(part) =>
|
||||
html`<kbd>${part}</kbd>`,
|
||||
)}
|
||||
</span>
|
||||
</div>
|
||||
`;
|
||||
})}
|
||||
</div>
|
||||
`;
|
||||
})}
|
||||
<p class="footnote">
|
||||
Every one of these can be rebound in Settings →
|
||||
Keyboard Shortcuts.
|
||||
</p>
|
||||
</wa-dialog>
|
||||
`;
|
||||
}
|
||||
}
|
||||
|
||||
declare global {
|
||||
interface HTMLElementTagNameMap {
|
||||
'shortcuts-overlay': ShortcutsOverlay;
|
||||
}
|
||||
}
|
||||
@@ -30,6 +30,14 @@ const KEY_ALIASES: Record<string, string> = {
|
||||
' ': 'Space',
|
||||
};
|
||||
|
||||
/** Whether a resolved key is a printable character that a Shift press
|
||||
* produced, rather than a key Shift was held alongside. Letters are
|
||||
* excluded: `A` and `Shift+A` are the same character, so Shift stays
|
||||
* meaningful there. */
|
||||
function isShiftedCharacter(key: string): boolean {
|
||||
return key.length === 1 && !/[A-Z0-9]/.test(key);
|
||||
}
|
||||
|
||||
/** Keys that are modifier-only presses and should be ignored. */
|
||||
const MODIFIER_KEYS = new Set([
|
||||
'Control',
|
||||
@@ -52,11 +60,6 @@ export function buildKeyString(e: KeyboardEvent): string {
|
||||
|
||||
const parts: string[] = [];
|
||||
|
||||
// Modifiers in fixed order. Treat Meta (Cmd on Mac) as Ctrl.
|
||||
if (e.ctrlKey || e.metaKey) parts.push('Ctrl');
|
||||
if (e.altKey) parts.push('Alt');
|
||||
if (e.shiftKey) parts.push('Shift');
|
||||
|
||||
// Normalize the key name.
|
||||
let key = KEY_ALIASES[e.key] ?? e.key;
|
||||
|
||||
@@ -65,6 +68,16 @@ export function buildKeyString(e: KeyboardEvent): string {
|
||||
key = key.toUpperCase();
|
||||
}
|
||||
|
||||
// Modifiers in fixed order. Treat Meta (Cmd on Mac) as Ctrl.
|
||||
if (e.ctrlKey || e.metaKey) parts.push('Ctrl');
|
||||
if (e.altKey) parts.push('Alt');
|
||||
|
||||
// Shift is only a modifier when it did not *produce* the key.
|
||||
// `?` is Shift+/ on a US layout and something else elsewhere, and
|
||||
// "Shift+?" is a binding nobody would write down; the character
|
||||
// already carries the shift.
|
||||
if (e.shiftKey && !isShiftedCharacter(key)) parts.push('Shift');
|
||||
|
||||
parts.push(key);
|
||||
|
||||
return parts.join('+');
|
||||
@@ -143,6 +156,7 @@ type ShortcutScope =
|
||||
*/
|
||||
const ACTIVATION_KEYS = new Set(['Space', 'Enter']);
|
||||
const ARROW_KEYS = new Set(['Up', 'Down', 'Left', 'Right', 'Home', 'End']);
|
||||
const VERTICAL_KEYS = new Set(['Up', 'Down', 'Home', 'End']);
|
||||
const LIST_KEYS = new Set([...ARROW_KEYS, ...ACTIVATION_KEYS]);
|
||||
const SLIDER_KEYS = new Set([...ARROW_KEYS, 'PageUp', 'PageDown']);
|
||||
|
||||
@@ -160,7 +174,14 @@ function keysOwnedBy(el: Element): ReadonlySet<string> | null {
|
||||
}
|
||||
|
||||
// A grid row or a listbox option moves with the arrow keys, which is
|
||||
// what makes a track list navigable without a mouse.
|
||||
// what makes a track list navigable without a mouse — but only the
|
||||
// *vertical* ones. Every list in this app (`track-list`'s own
|
||||
// handler, `utils/roving-rows.ts`, the card grids that use it) moves
|
||||
// on Up/Down/Home/End and does nothing with Left/Right, so granting
|
||||
// those took keyboard seeking away from a focused row and gave it to
|
||||
// nobody: two ArrowRights on a focused track row produced zero
|
||||
// `Player.Seek` calls, against one per press with focus on the body.
|
||||
// Grant them back here if a list ever moves horizontally.
|
||||
if (
|
||||
role === 'row' ||
|
||||
role === 'gridcell' ||
|
||||
@@ -168,7 +189,7 @@ function keysOwnedBy(el: Element): ReadonlySet<string> | null {
|
||||
role === 'option' ||
|
||||
role === 'treeitem'
|
||||
) {
|
||||
return ARROW_KEYS;
|
||||
return VERTICAL_KEYS;
|
||||
}
|
||||
|
||||
if (tag === 'INPUT') {
|
||||
@@ -398,6 +419,12 @@ async function dispatch(action: string): Promise<void> {
|
||||
}
|
||||
|
||||
// App actions
|
||||
case 'app.shortcuts':
|
||||
document.dispatchEvent(
|
||||
new CustomEvent('shortcut:app-shortcuts'),
|
||||
);
|
||||
break;
|
||||
|
||||
case 'app.selectAll':
|
||||
document.dispatchEvent(
|
||||
new CustomEvent('shortcut:select-all'),
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
/**
|
||||
* What each shortcut action is called, where it applies, and what it is
|
||||
* bound to out of the box.
|
||||
*
|
||||
* This lived as a private static in `config-page`, which meant Settings
|
||||
* was the only place in the app the key bindings were written down —
|
||||
* and Settings renders three of the four categories, so the autotag
|
||||
* keys were written down nowhere at all. The `?` overlay and the
|
||||
* Settings editor now read one table.
|
||||
*/
|
||||
|
||||
export interface ShortcutMeta {
|
||||
label: string;
|
||||
category: string;
|
||||
scope: string;
|
||||
defaultKey: string;
|
||||
}
|
||||
|
||||
/** The categories, in the order both surfaces show them. */
|
||||
export const SHORTCUT_CATEGORIES = [
|
||||
'Player',
|
||||
'Navigation',
|
||||
'App',
|
||||
'Autotag',
|
||||
] as const;
|
||||
|
||||
export const SHORTCUT_META: Record<string, ShortcutMeta> = {
|
||||
'player.playPause': {
|
||||
label: 'Play / Pause',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Space',
|
||||
},
|
||||
'player.next': {
|
||||
label: 'Next Track',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'N',
|
||||
},
|
||||
'player.previous': {
|
||||
label: 'Previous Track',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'P',
|
||||
},
|
||||
'player.volumeUp': {
|
||||
label: 'Volume Up',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Up',
|
||||
},
|
||||
'player.volumeDown': {
|
||||
label: 'Volume Down',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Down',
|
||||
},
|
||||
'player.seekForward': {
|
||||
label: 'Seek Forward',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Right',
|
||||
},
|
||||
'player.seekBack': {
|
||||
label: 'Seek Back',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'Left',
|
||||
},
|
||||
'player.shuffle': {
|
||||
label: 'Toggle Shuffle',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'S',
|
||||
},
|
||||
'player.repeat': {
|
||||
label: 'Cycle Repeat',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'R',
|
||||
},
|
||||
'player.mute': {
|
||||
label: 'Toggle Mute',
|
||||
category: 'Player',
|
||||
scope: 'global',
|
||||
defaultKey: 'M',
|
||||
},
|
||||
'nav.search': {
|
||||
label: 'Focus Search',
|
||||
category: 'Navigation',
|
||||
scope: 'global',
|
||||
defaultKey: '/',
|
||||
},
|
||||
'nav.searchAlt': {
|
||||
label: 'Focus Search (Alt)',
|
||||
category: 'Navigation',
|
||||
scope: 'global',
|
||||
defaultKey: 'Ctrl+F',
|
||||
},
|
||||
'nav.queue': {
|
||||
label: 'Toggle Queue',
|
||||
category: 'Navigation',
|
||||
scope: 'global',
|
||||
defaultKey: 'Q',
|
||||
},
|
||||
'app.shortcuts': {
|
||||
label: 'Keyboard Shortcuts',
|
||||
category: 'App',
|
||||
scope: 'global',
|
||||
defaultKey: '?',
|
||||
},
|
||||
'app.selectAll': {
|
||||
label: 'Select All',
|
||||
category: 'App',
|
||||
scope: 'global',
|
||||
defaultKey: 'Ctrl+A',
|
||||
},
|
||||
'tracklist.play': {
|
||||
label: 'Play Selected',
|
||||
category: 'Navigation',
|
||||
scope: 'panel:track-list',
|
||||
defaultKey: 'Enter',
|
||||
},
|
||||
'autotag.apply': {
|
||||
label: 'Apply Match',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'A',
|
||||
},
|
||||
'autotag.skip': {
|
||||
label: 'Skip Folder',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'S',
|
||||
},
|
||||
'autotag.leave': {
|
||||
label: 'Leave As Is',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'L',
|
||||
},
|
||||
'autotag.paste': {
|
||||
label: 'Paste Release URL',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'U',
|
||||
},
|
||||
'autotag.search': {
|
||||
label: 'Search Candidates',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'F',
|
||||
},
|
||||
'autotag.next': {
|
||||
label: 'Next Folder',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'Down',
|
||||
},
|
||||
'autotag.previous': {
|
||||
label: 'Previous Folder',
|
||||
category: 'Autotag',
|
||||
scope: 'panel:autotag',
|
||||
defaultKey: 'Up',
|
||||
},
|
||||
};
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* The key story, told once.
|
||||
*
|
||||
* Decision 1 keeps the unmodified single-key bindings, and Settings was
|
||||
* the only place they were written down — three of the four categories
|
||||
* of them, so the autotag keys were written down nowhere at all.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/shortcuts-overlay/shortcuts-overlay';
|
||||
import { emit } from '@test/support/harness';
|
||||
import { Events } from '../../src/events';
|
||||
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||
|
||||
/** Open it the way the shortcut service does. */
|
||||
async function openOverlay(el: HTMLElement): Promise<void> {
|
||||
document.dispatchEvent(new CustomEvent('shortcut:app-shortcuts'));
|
||||
await (el as HTMLElement & { updateComplete: Promise<boolean> })
|
||||
.updateComplete;
|
||||
}
|
||||
|
||||
describe('<shortcuts-overlay>', () => {
|
||||
it('renders nothing until it is asked for', async () => {
|
||||
const el = await fixture('shortcuts-overlay');
|
||||
|
||||
expect(shadow(el, 'wa-dialog')).toBeNull();
|
||||
});
|
||||
|
||||
it('opens on the shortcut event, and stays open on a second one', async () => {
|
||||
const el = await fixture('shortcuts-overlay');
|
||||
|
||||
await openOverlay(el);
|
||||
expect(shadow(el, '[data-testid="shortcuts-overlay"]')).toBeTruthy();
|
||||
|
||||
// Deliberately not a toggle: a dialog owns every unmodified key
|
||||
// while it is up, so a second `?` never reaches the service and a
|
||||
// toggle would be a promise the shortcut layer cannot keep.
|
||||
await openOverlay(el);
|
||||
expect(shadow(el, '[data-testid="shortcuts-overlay"]')).toBeTruthy();
|
||||
});
|
||||
|
||||
it('lists the autotag keys, which Settings never showed', async () => {
|
||||
const el = await fixture('shortcuts-overlay');
|
||||
|
||||
await openOverlay(el);
|
||||
|
||||
const headings = shadowAll(el, 'h3').map((h) =>
|
||||
h.textContent!.trim().split(' ')[0],
|
||||
);
|
||||
|
||||
expect(headings).toContain('Autotag');
|
||||
});
|
||||
|
||||
it('shows the bound key, not the default it shipped with', async () => {
|
||||
const el = await fixture('shortcuts-overlay');
|
||||
|
||||
emit(Events.ShortcutsConfigChanged, { 'player.playPause': 'K' });
|
||||
await openOverlay(el);
|
||||
|
||||
const playPause = shadowAll(el, '.row').find((row) =>
|
||||
row.textContent!.includes('Play / Pause'),
|
||||
);
|
||||
|
||||
expect(playPause?.querySelector('kbd')?.textContent).toBe('K');
|
||||
});
|
||||
|
||||
it('splits a combination into one key each', async () => {
|
||||
const el = await fixture('shortcuts-overlay');
|
||||
|
||||
emit(Events.ShortcutsConfigChanged, { 'app.selectAll': 'Ctrl+A' });
|
||||
await openOverlay(el);
|
||||
|
||||
const selectAll = shadowAll(el, '.row').find((row) =>
|
||||
row.textContent!.includes('Select All'),
|
||||
);
|
||||
|
||||
expect(
|
||||
[...selectAll!.querySelectorAll('kbd')].map((k) => k.textContent),
|
||||
).toEqual(['Ctrl', 'A']);
|
||||
});
|
||||
});
|
||||
@@ -86,6 +86,20 @@ describe('buildKeyString', () => {
|
||||
expect(names).toEqual(['Up', 'Down', 'Left', 'Right', 'Space']);
|
||||
});
|
||||
|
||||
it('does not report Shift for a character Shift produced', () => {
|
||||
// `?` is Shift+/ on a US layout and something else elsewhere, so
|
||||
// "Shift+?" is a binding nobody would write down. Letters keep it:
|
||||
// `Shift+A` and `A` are the same character.
|
||||
const question = new KeyboardEvent('keydown', {
|
||||
key: '?',
|
||||
shiftKey: true,
|
||||
});
|
||||
const letter = new KeyboardEvent('keydown', { key: 'A', shiftKey: true });
|
||||
|
||||
expect(buildKeyString(question)).toBe('?');
|
||||
expect(buildKeyString(letter)).toBe('Shift+A');
|
||||
});
|
||||
|
||||
it('leaves multi-character named keys alone', () => {
|
||||
expect(
|
||||
buildKeyString(new KeyboardEvent('keydown', { key: 'Escape' })),
|
||||
@@ -233,6 +247,37 @@ describe('shortcut dispatch: scope', () => {
|
||||
expect(calls('player.Player.ChangeVolume')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('leaves Up and Down to a focused row, which moves on them', () => {
|
||||
bindings({ 'player.volumeUp': 'Up' });
|
||||
|
||||
const row = mount(document.createElement('div'));
|
||||
|
||||
row.setAttribute('role', 'row');
|
||||
row.tabIndex = 0;
|
||||
row.focus();
|
||||
press('ArrowUp');
|
||||
|
||||
expect(calls('player.Player.ChangeVolume')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('still seeks on Left/Right from a focused row', () => {
|
||||
// Phase 1 gave the arrows to the grid, correctly — but all six of
|
||||
// them, and no list in this app moves horizontally, so seeking
|
||||
// stopped working from a focused row and nothing gained the keys.
|
||||
// Reproduced in the app: two ArrowRights on a focused track row,
|
||||
// zero Player.Seek calls, against one per press from the body.
|
||||
bindings({ 'player.seekForward': 'Right' });
|
||||
|
||||
const row = mount(document.createElement('div'));
|
||||
|
||||
row.setAttribute('role', 'row');
|
||||
row.tabIndex = 0;
|
||||
row.focus();
|
||||
press('ArrowRight');
|
||||
|
||||
expect(calls('player.Player.CurrentPositionSeconds')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('leaves every unmodified key to an open dialog', () => {
|
||||
const dialog = mount(document.createElement('div'));
|
||||
const button = document.createElement('button');
|
||||
|
||||
Reference in New Issue
Block a user