diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a1c58a..1de963c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] +### Added +- Surface-aware keyboard shortcuts for SQL Browser and Dashboard (#417). The + shared, platform-aware shortcut catalog now drives both help and dispatch; + Dashboard gains refresh, View/Edit, and `G` navigation commands while stale + viewer sessions, loading routes, stacked overlays, and nested text inputs + fail closed. Contextual help is torn down on route and authentication + transitions so it cannot survive onto another surface or Login. + ## [0.6.4] - 2026-07-24 ### Fixed diff --git a/src/styles.css b/src/styles.css index a74f959..d23f36e 100644 --- a/src/styles.css +++ b/src/styles.css @@ -732,8 +732,11 @@ body { border: 1px solid var(--border); border-radius: 10px; padding: 18px 20px; min-width: 360px; max-width: 480px; + max-height: calc(100dvh - 32px); + display: flex; flex-direction: column; box-shadow: 0 20px 60px rgba(0,0,0,.5); } +.modal-card-body { overflow-y: auto; } .modal-card h2 { margin: 0 0 12px; font-size: 14px; font-weight: 600; color: var(--fg); } @@ -754,6 +757,8 @@ body { border: 1px solid var(--border); border-radius: 4px; color: var(--fg); } +.shortcut-keys { display: inline-flex; align-items: center; gap: 4px; } +.shortcut-then { color: var(--fg-faint); font-size: 11px; } .modal-card .close-row { margin-top: 12px; display: flex; justify-content: flex-end; } diff --git a/src/ui/app.ts b/src/ui/app.ts index 9c8ca53..a48d698 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -60,10 +60,10 @@ import { recentOptions } from '../core/recent-values.js'; import { paramComparisonColumns } from '../core/param-comparison.js'; import type { SchemaDb } from '../core/from-scope.js'; import { renderLogin } from './login.js'; -import { openShortcuts } from './shortcuts.js'; +import { openShortcuts, resetShortcutChord } from './shortcuts.js'; import { startDrag } from './splitters.js'; import { flashToast } from './toast.js'; -import type { App, ActionsRegistry, SchemaFocus, WorkspaceChangedMessage } from './app.types.js'; +import type { App, ActionsRegistry, KeyboardOwner, SchemaFocus, WorkspaceChangedMessage } from './app.types.js'; import type { CreateAppEnv, BroadcastChannelPort } from '../env.types.js'; import { createQueryExecutionService } from '../application/query-execution-service.js'; import { createConnectionSession } from '../application/connection-session.js'; @@ -236,6 +236,31 @@ export function createApp(env: CreateAppEnv = {}): App { app.sqlRoute = parseSqlRoute(routeSearch); app.currentWorkspace = null; app.workspaceRouteStatus = 'ready'; + app.keyboardOwner = null; + app.resetShortcutChord = () => resetShortcutChord(app); + const keyboardOwners: KeyboardOwner[] = []; + app.acquireKeyboardOwner = (kind) => { + const owner = { kind }; + keyboardOwners.push(owner); + app.keyboardOwner = owner; + resetShortcutChord(app); + let released = false; + return () => { + if (released) return; + released = true; + const index = keyboardOwners.indexOf(owner); + if (index >= 0) keyboardOwners.splice(index, 1); + app.keyboardOwner = keyboardOwners.at(-1) ?? null; + resetShortcutChord(app); + }; + }; + app.shortcutDialog = null; + app.closeShortcutDialog = () => { + const dialog = app.shortcutDialog; + app.shortcutDialog = null; + dialog?.close(); + }; + app.surfaceCommands = null; app.captureSurfaceGeneration = () => surfaceGeneration; app.isSurfaceGenerationCurrent = (generation) => generation === surfaceGeneration; app.refreshCurrentSurfaceAfterStale = (generation, committed = false) => { @@ -379,7 +404,11 @@ export function createApp(env: CreateAppEnv = {}): App { // structural-only reinterpretation, not a new runtime assumption (a null // root would already throw inside login.ts's own `app.root.replaceChildren` // either way). - const renderLoginApp = (msg?: string): void => renderLogin(app as App & { root: Element }, msg); + const renderLoginApp = (msg?: string): void => { + app.closeShortcutDialog(); + resetShortcutChord(app); + renderLogin(app as App & { root: Element }, msg); + }; // The auth + config + ClickHouse connection lifecycle (#276 Phase 2) — OAuth // PKCE login/refresh, Basic probing, and IdP config resolution live in // `application/connection-session.ts`, @@ -413,6 +442,8 @@ export function createApp(env: CreateAppEnv = {}): App { // workbench session stays reusable after destroy(): the next renderApp // re-attaches its shell effects. app.signOut = () => { + app.closeShortcutDialog(); + resetShortcutChord(app); workbench.destroy(); // Plain abort (no clearResult settle) — the login render replaces the // whole DOM next, so settling the visible result would be a wasted paint. @@ -1260,11 +1291,13 @@ export function createApp(env: CreateAppEnv = {}): App { function anchoredPopover( node: HTMLElement, anchorEl: HTMLElement, refKey: 'savePopover' | 'userMenu', ): { close: () => void } { + const releaseKeyboard = app.acquireKeyboardOwner('popover'); const close = (): void => { anchoredPopoverClosers.delete(close); doc.removeEventListener('keydown', onKey, true); doc.removeEventListener('mousedown', onOutside, true); if (app.dom[refKey]) { app.dom[refKey]!.remove(); app.dom[refKey] = undefined; } + releaseKeyboard(); }; const onKey = (e: KeyboardEvent): void => { if (e.key === 'Escape') { e.preventDefault(); close(); } }; const onOutside = (e: MouseEvent): void => { @@ -1537,7 +1570,10 @@ export function createApp(env: CreateAppEnv = {}): App { let disposeWorkbenchMount: (() => void) | null = null; const ignoreExternalWorkspaceChange = (): void => {}; app.renderDashboard = () => { + app.closeShortcutDialog(); + resetShortcutChord(app); surfaceGeneration += 1; + app.surfaceCommands = null; closeAnchoredPopovers(); disposeFileMenuOverlays(app); disposeWorkbenchMount?.(); @@ -1546,7 +1582,10 @@ export function createApp(env: CreateAppEnv = {}): App { return renderDashboard(app); }; const disposeCurrentSurface = (): void => { + app.closeShortcutDialog(); + resetShortcutChord(app); surfaceGeneration += 1; + app.surfaceCommands = null; for (const control of app.root?.querySelectorAll( 'button, input, select, textarea', ) ?? []) control.disabled = true; @@ -1930,6 +1969,8 @@ export function createApp(env: CreateAppEnv = {}): App { }; app.navigateSqlRoute = async (route, method) => { + app.closeShortcutDialog(); + resetShortcutChord(app); const workspaceChanged = route.workspaceKey !== app.sqlRoute.workspaceKey; writeRoute(route, method); if (workspaceChanged) { @@ -1944,6 +1985,8 @@ export function createApp(env: CreateAppEnv = {}): App { }; app.handleSqlPopState = async () => { + app.closeShortcutDialog(); + resetShortcutChord(app); const previousKey = app.sqlRoute.workspaceKey; routeSearch = loc.search; app.sqlRoute = parseSqlRoute(routeSearch); @@ -2032,7 +2075,10 @@ export function createApp(env: CreateAppEnv = {}): App { openNodeDetail, insertCreate: async (target) => { await insertCreate(target); toEditorOnMobile(); }, openCreateInNewTab: (target, name) => openCreateInNewTab(target, name), - openShortcuts: () => openShortcuts(app), + openShortcuts: () => { + const dialog = openShortcuts(app, () => { app.shortcutDialog = null; }); + if (dialog) app.shortcutDialog = dialog; + }, openDashboard, // #302: Dashboard import/export invoked from the Dashboard page's own File // menu (and still from the Workbench during the transition). Export is a @@ -2051,7 +2097,10 @@ export function createApp(env: CreateAppEnv = {}): App { }; app.renderApp = () => { + app.closeShortcutDialog(); + resetShortcutChord(app); surfaceGeneration += 1; + app.surfaceCommands = null; closeAnchoredPopovers(); disposeFileMenuOverlays(app); disposeDashboardSurface(); diff --git a/src/ui/app.types.ts b/src/ui/app.types.ts index 8e4dc09..0dbd8b5 100644 --- a/src/ui/app.types.ts +++ b/src/ui/app.types.ts @@ -24,6 +24,7 @@ import type { WorkspaceDiagnostic } from '../dashboard/model/workspace-diagnosti import type { StoredWorkspaceV2 } from '../generated/json-schema.types.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; import type { SqlRoute } from '../core/sql-route.js'; +import type { SurfaceCommandPort } from './shortcuts.js'; import type { DynamicSources } from '../core/spec-completion.js'; import type { WorkbenchSession } from './workbench/workbench-session.js'; import type { WorkbenchParameterSession } from '../application/workbench-parameter-session.js'; @@ -154,6 +155,17 @@ export interface AppDom { varStripDeferHooked?: boolean; } +/** The currently open UI primitive that has exclusive keyboard handling. */ +export interface KeyboardOwner { + kind: 'modal' | 'menu' | 'popover'; +} +export type KeyboardOwnerRelease = () => void; + +export interface ShortcutDialogHandle { + backdrop: HTMLElement; + close(): void; +} + /** The live ClickHouse auth context every query call site reads/mutates — * a structural alias of `application/connection-session.ts`'s own * `SessionChCtx` (the session is the one place that constructs and mutates @@ -221,6 +233,14 @@ export interface App { dom: AppDom; root: Element | null; document: Document; + /** Set by shared overlay primitives for the duration of their open lifecycle. */ + keyboardOwner: KeyboardOwner | null; + /** Acquire exclusive application-keyboard ownership. The returned idempotent + * release removes only this acquisition, preserving any owner below it. */ + acquireKeyboardOwner(kind: KeyboardOwner['kind']): KeyboardOwnerRelease; + resetShortcutChord(): void; + shortcutDialog: ShortcutDialogHandle | null; + closeShortcutDialog(): void; /** The auth + config + ClickHouse connection lifecycle (#276 Phase 2) — * OAuth PKCE login/refresh, Basic probing, and IdP config resolution, @@ -429,6 +449,9 @@ export interface App { sqlRoute: SqlRoute; currentWorkspace: StoredWorkspaceV2 | null; workspaceRouteStatus: 'loading' | 'ready' | 'not-found' | 'error'; + /** Route-local commands registered by the mounted surface. They are cleared + * before every transition, so a disposed Dashboard viewer cannot be called. */ + surfaceCommands: SurfaceCommandPort | null; /** Renderer lifetime, distinct from workspace-load ordering. Any surface * teardown/remount advances it so obsolete async callbacks can finish their * durable work without settling against a replacement renderer. */ diff --git a/src/ui/dashboard.ts b/src/ui/dashboard.ts index 093ff2a..3125b66 100644 --- a/src/ui/dashboard.ts +++ b/src/ui/dashboard.ts @@ -133,6 +133,10 @@ export interface DashboardApp { currentWorkspace: StoredWorkspaceV2 | null; sqlRoute: SqlRoute; navigateSqlRoute(route: SqlRoute, method: 'push' | 'replace'): Promise; + surfaceCommands: App['surfaceCommands']; + keyboardOwner: App['keyboardOwner']; + acquireKeyboardOwner: App['acquireKeyboardOwner']; + resetShortcutChord: App['resetShortcutChord']; renderDashboard(): void; captureSurfaceGeneration(): number; isSurfaceGenerationCurrent(generation: number): boolean; @@ -194,6 +198,14 @@ let installedDashboardChartInteraction: DashboardChartInteractionController | nu let installedDashboardCleanup: (() => void) | null = null; /** Tear down every resource owned by the currently mounted Dashboard surface. */ +function keyboardOwnerChannel(app: Pick): (owner: App['keyboardOwner']) => void { + let release: (() => void) | null = null; + return (owner) => { + release?.(); + release = owner ? app.acquireKeyboardOwner(owner.kind) : null; + }; +} + export function disposeDashboardSurface(): void { if (installedGridResizeListener) { installedGridResizeListener.win.removeEventListener('resize', installedGridResizeListener.handler); @@ -216,7 +228,7 @@ export function disposeDashboardSurface(): void { * reflects session changes without adding a second header label. */ type LayoutOption = [value: string, label: string, title?: string]; function buildLayoutMenu( - doc: Document, + doc: Document, onKeyboardOwnerChange: (owner: App['keyboardOwner']) => void, options: LayoutOption[], getActive: () => string, onPick: (value: string) => void, ariaLabel: string, ): { el: HTMLButtonElement; sync: () => void } { const label = h('span'); @@ -247,6 +259,7 @@ function buildLayoutMenu( onClick: () => onPick(value), })), onClose: () => { handle = null; }, + onKeyboardOwnerChange, }); }; el.onclick = () => { if (handle) { handle.close(); el.focus(); } else open(); }; @@ -404,6 +417,7 @@ function buildDashboardFileMenu(app: DashboardApp, readOnly = false): HTMLButton }, h('span', null, 'File'), Icon.chevDown()) as HTMLButtonElement; let handle: MenuHandle | null = null; + const onKeyboardOwnerChange = keyboardOwnerChannel(app); const open = (): void => { const rows: MenuRow[] = [ @@ -423,6 +437,7 @@ function buildDashboardFileMenu(app: DashboardApp, readOnly = false): HTMLButton handle = openMenu({ document: doc, trigger: btn, rows, menuClass: 'dash-file-menu', onClose: () => { handle = null; }, + onKeyboardOwnerChange, }); }; @@ -442,6 +457,7 @@ export async function renderDashboard(app: DashboardApp): Promise { // call installed on this window before this call installs its own (see // `installedGridResizeListener`'s own doc comment above). disposeDashboardSurface(); + app.surfaceCommands = null; const workspace = app.currentWorkspace; const readOnly = app.sqlRoute.surface === 'dashboard' && app.sqlRoute.mode === 'view'; @@ -523,6 +539,11 @@ export async function renderDashboard(app: DashboardApp): Promise { recordBoundParams: (bp) => app.params.recordBoundParams(bp), initialFilters: initialBag, }); + // The global shortcut reaches this route-local port only while its renderer + // generation is current. It is cleared by both Dashboard cleanup and every + // application surface transition. + const commandPort = { surface: 'dashboard' as const, generation: surfaceGeneration, refresh: () => session.refresh() }; + app.surfaceCommands = commandPort; let trackedSessionTileIds = new Set(viewerDoc.tiles.map((tile) => tile.id)); const syncSessionDocument = (next: DashboardDocumentV1): void => { session.syncDocument(next); @@ -570,7 +591,7 @@ export async function renderDashboard(app: DashboardApp): Promise { ? (gridRenderMode === 'full' ? 'full' : 'grafana-grid') : typeof currentDoc.layout.preset === 'string' ? currentDoc.layout.preset : 'report'); const layoutMenu = buildLayoutMenu( - doc, + doc, keyboardOwnerChannel(app), readOnly ? READONLY_LAYOUT_OPTIONS : EDITABLE_LAYOUT_OPTIONS, getActiveLayoutOption, (value) => { @@ -878,7 +899,8 @@ export async function renderDashboard(app: DashboardApp): Promise { }; const bar = buildFilterBar( filterBarApp, session.controls, onCommit, getField, - { curatedFields, document: doc, onApplyCurated, timeRange, onApplyTimeRange }, + { curatedFields, document: doc, onApplyCurated, timeRange, onApplyTimeRange, + onKeyboardOwnerChange: keyboardOwnerChannel(app) }, ); timeFilterHost.replaceChildren(bar.timeEl); ordinaryFilterHost.replaceChildren(bar.ordinaryEl); @@ -2199,6 +2221,7 @@ export async function renderDashboard(app: DashboardApp): Promise { // rebuild must not leave Chart.js observers, signal effects, popovers, or // viewer requests attached to the replaced page. installedDashboardCleanup = () => { + if (app.surfaceCommands === commandPort) app.surfaceCommands = null; currentFilterBar?.dispose(); currentFilterBar = null; if (tileSearchTimer != null) clearTimeout(tileSearchTimer); diff --git a/src/ui/detached-view.ts b/src/ui/detached-view.ts index df1dd02..7ff367d 100644 --- a/src/ui/detached-view.ts +++ b/src/ui/detached-view.ts @@ -10,6 +10,7 @@ import { h, withDocument, attachBackdropClose } from './dom.js'; import { Icon } from './icons.js'; import type { Signal } from '@preact/signals-core'; +import type { KeyboardOwner } from './app.types.js'; /** The window-like object `app.openWindow()` returns — a real `Window` in * production. Accessing `.document` may legitimately throw (COOP severing @@ -29,6 +30,7 @@ export interface DetachedViewApp { stylesText?: string; faviconHref?: string; state?: { detachedView?: Signal }; + acquireKeyboardOwner?(kind: KeyboardOwner['kind']): () => void; openWindow(url: string, target: string): DetachedWindowLike | null; } @@ -128,6 +130,7 @@ function openAsOverlay( mount: MountFn, onClose: () => void, ): DetachedView { return withDocument(mainDoc, () => { + const releaseKeyboard = app?.acquireKeyboardOwner?.('modal') ?? (() => {}); const { panel, bar, body } = buildPanel(mode, title); let teardown: (() => void) | null = null; let closed = false; @@ -140,6 +143,7 @@ function openAsOverlay( backdrop.remove(); if (teardown) teardown(); onClose(); + releaseKeyboard(); }; backdrop = h('div', { class: 'graph-overlay' }, panel); detachBackdrop = attachBackdropClose(backdrop, close); diff --git a/src/ui/file-menu.ts b/src/ui/file-menu.ts index e09a417..8b5cbcf 100644 --- a/src/ui/file-menu.ts +++ b/src/ui/file-menu.ts @@ -49,6 +49,13 @@ import type { WorkspaceDiagnostic } from '../dashboard/model/workspace-diagnosti const fileBase = (name: unknown): string => (String(name || '')).replace(/[\\/:*?"<>|]+/g, '-').replace(/\s+/g, ' ').trim() || 'queries'; const queries = (n: number): string => n + (n === 1 ? ' query' : ' queries'); const first = (diagnostics: readonly WorkspaceDiagnostic[], fallback: string): string => diagnostics[0]?.message || fallback; +function keyboardOwnerChannel(app: Pick): (owner: App['keyboardOwner']) => void { + let release: (() => void) | null = null; + return (owner) => { + release?.(); + release = owner ? app.acquireKeyboardOwner(owner.kind) : null; + }; +} /** Build the header File button + editable workspace title; returns the nodes * to splice into the app header (after the connection chip). */ @@ -170,7 +177,8 @@ export function openFileMenu(app: App): void { { kind: 'custom', node: countRow }, ]; - const handle = openMenu({ document: doc, trigger: app.dom.fileBtn!, rows }); + const handle = openMenu({ document: doc, trigger: app.dom.fileBtn!, rows, + onKeyboardOwnerChange: keyboardOwnerChannel(app) }); // The hidden file pickers aren't menu ROWS (no label/click chrome of their // own) — they're display:none inputs `.click()`-triggered by the Import // queries / Import workspace items above. Parent them to the mounted menu @@ -727,6 +735,7 @@ export function disposeFileMenuOverlays(app: Pick): voi * `openConfirm`/the conflict dialog/the dashboard picker all build on. */ function openDialogShell(app: App, title: string, content: unknown[], extraCardClass?: string): DialogHandle { const doc = app.document; + const releaseKeyboard = app.acquireKeyboardOwner('modal'); let backdrop: HTMLElement; const close = (): void => { doc.removeEventListener('keydown', onKey, true); @@ -734,6 +743,7 @@ function openDialogShell(app: App, title: string, content: unknown[], extraCardC dialogClosers.delete(backdrop); backdrop.remove(); if (app.dom.fileDialog === backdrop) app.dom.fileDialog = undefined; + releaseKeyboard(); }; const onKey = (e: KeyboardEvent): void => { if (e.key === 'Escape') { e.preventDefault(); close(); } }; const card = h('div', { class: extraCardClass ? `fm-dialog-card ${extraCardClass}` : 'fm-dialog-card' }, diff --git a/src/ui/filter-bar.ts b/src/ui/filter-bar.ts index 6f69e03..58cfe1b 100644 --- a/src/ui/filter-bar.ts +++ b/src/ui/filter-bar.ts @@ -25,6 +25,7 @@ import { buildFilterOptionField } from './filter-option-field.js'; import type { FilterFieldOption } from './filter-option-field.js'; import { buildMultiSelectField } from './multi-select-field.js'; import { buildTimeRangeField } from './time-range-field.js'; +import type { KeyboardOwner } from './app.types.js'; import type { DashboardTimeRangeGroup, TimeRangeRecent } from '../core/time-range.js'; import type { WorkbenchParameterSession } from '../application/workbench-parameter-session.js'; @@ -89,6 +90,7 @@ export interface BuildFilterBarOptions { * `session.applyFilters` over the group's from/to filter ids. Only reached * when `timeRange` built at least one control. */ onApplyTimeRange?(group: DashboardTimeRangeGroup, from: string, to: string): void; + onKeyboardOwnerChange?: (owner: KeyboardOwner | null) => void; } /** #360: `status`/`stale`/`waitingFor` mirror `ViewerFilterState`'s own @@ -450,6 +452,7 @@ export function buildFilterBar( app.params.saveFilterActive(); onCommit(p.name); }, + onKeyboardOwnerChange: options.onKeyboardOwnerChange, }); // #335: a multiselect handle already satisfies `FieldHandle` directly. handles.set(p.name, msField); @@ -592,6 +595,7 @@ export function buildFilterBar( fromValue: tr.fromValue, toValue: tr.toValue, active: tr.active, waveNowMs: tr.waveNowMs, wallNow: app.wallNow, getRecents: tr.recents, onApply: (from, to) => options.onApplyTimeRange?.(tr.group, from, to), + onKeyboardOwnerChange: options.onKeyboardOwnerChange, }); handles.set(`group:${tr.group.key}`, trField); timeSection.push(trField.el); diff --git a/src/ui/menu.ts b/src/ui/menu.ts index 9ba0e2c..3406a21 100644 --- a/src/ui/menu.ts +++ b/src/ui/menu.ts @@ -23,6 +23,7 @@ // convention), so this is fully unit-testable under happy-dom. import { h, fixedAnchor } from './dom.js'; +import type { KeyboardOwner } from './app.types.js'; /** One row of a dropdown menu. * - `item` — an actionable `.fm-item` row: icon + label + optional meta text @@ -56,6 +57,7 @@ export interface MenuOptions { * overlay click, an item's own click, or an explicit `handle.close()`) — * lets the caller clear its own open/closed bookkeeping. */ onClose?: () => void; + onKeyboardOwnerChange?: (owner: KeyboardOwner | null) => void; } export interface MenuHandle { @@ -141,9 +143,11 @@ export function openMenu(opts: MenuOptions): MenuHandle { overlay.remove(); trigger.setAttribute('aria-expanded', 'false'); onClose?.(); + opts.onKeyboardOwnerChange?.(null); } trigger.setAttribute('aria-haspopup', 'menu'); + opts.onKeyboardOwnerChange?.({ kind: 'menu' }); trigger.setAttribute('aria-expanded', 'true'); doc.body.appendChild(overlay); doc.body.appendChild(menu); diff --git a/src/ui/multi-select-field.ts b/src/ui/multi-select-field.ts index 0c29dd5..686deba 100644 --- a/src/ui/multi-select-field.ts +++ b/src/ui/multi-select-field.ts @@ -39,6 +39,7 @@ import { h } from './dom.js'; import { openAnchoredDialog } from './popover.js'; import { idSafe } from './combobox.js'; import { canonicalizeSelection, sameSelection } from '../core/filter-selection.js'; +import type { KeyboardOwner } from './app.types.js'; /** One selectable option — value/label only (no grouping; #189 doesn't need it). */ export interface MultiSelectOption { @@ -86,6 +87,7 @@ export interface MultiSelectFieldOpts { onApply(next: string[], active: boolean): void; /** Error-mode plain-input commit (Enter, or blur after an edit). */ onFallbackCommit(raw: string, active: boolean): void; + onKeyboardOwnerChange?: (owner: KeyboardOwner | null) => void; } /** `buildMultiSelectField`'s return value. */ @@ -459,6 +461,7 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi minWidthFromTrigger: true, initialFocus: () => searchInput, // focus moves into the dialog on open onClose: () => { closeCurrent = null; openPopoverBusy = null; }, + onKeyboardOwnerChange: opts.onKeyboardOwnerChange, }); // #189 F2a: `skipFocus` flows through to the primitive so `applyStatus`'s // forced error-close can skip refocusing a trigger that's about to be diff --git a/src/ui/popover.ts b/src/ui/popover.ts index b34fa3a..82e3b00 100644 --- a/src/ui/popover.ts +++ b/src/ui/popover.ts @@ -33,6 +33,7 @@ import { h, fixedAnchor, attachBackdropClose } from './dom.js'; import type { FixedAnchorOptions } from './dom.js'; +import type { KeyboardOwner } from './app.types.js'; /** `openAnchoredDialog`'s options bag. */ export interface AnchoredDialogOptions { @@ -62,6 +63,7 @@ export interface AnchoredDialogOptions { /** Fires after teardown + focus return, on EVERY dismissal path, exactly * once (idempotent close never double-fires it). */ onClose?: () => void; + onKeyboardOwnerChange?: (owner: KeyboardOwner | null) => void; } /** `openAnchoredDialog`'s return value. */ @@ -127,9 +129,11 @@ export function openAnchoredDialog(opts: AnchoredDialogOptions): AnchoredDialogH trigger.setAttribute('aria-expanded', 'false'); if (!closeOpts.skipFocus) trigger.focus(); opts.onClose?.(); + opts.onKeyboardOwnerChange?.(null); } trigger.setAttribute('aria-expanded', 'true'); + opts.onKeyboardOwnerChange?.({ kind: 'popover' }); d.body.appendChild(overlay); d.body.appendChild(dialog); const detachBackdrop = attachBackdropClose(overlay, () => close()); diff --git a/src/ui/results.ts b/src/ui/results.ts index 12dc4ff..52c8ea4 100644 --- a/src/ui/results.ts +++ b/src/ui/results.ts @@ -33,7 +33,7 @@ import type { FilterBarApp } from './filter-bar.js'; import { buildDrawerChrome, attachDrawerResize } from './drawer.js'; import { panelExecution } from '../core/panel-execution.js'; import { renderFilterPreview } from './filter-preview.js'; -import type { AppDom, App } from './app.types.js'; +import type { AppDom, App, KeyboardOwner } from './app.types.js'; import type { PanelResolution } from '../core/panel-cfg.js'; import type { ResultSource } from '../core/query-source.js'; import type { SchemaGraphFocus } from '../core/schema-graph.js'; @@ -180,6 +180,7 @@ export interface ResultsApp { updateSaveBtn(): void; updateEditorModeUi?(): void; openWindow?(url: string, target: string): DetachedWindowLike | null; + acquireKeyboardOwner(kind: KeyboardOwner['kind']): () => void; } /** The Chart.js instance shape a readonly panel's `setChart` ever receives — @@ -471,15 +472,19 @@ export interface RowsViewerEntry { */ export function openRowsViewer(app: ResultsApp, entry: RowsViewerEntry): HTMLElement { const doc = app.document; + const releaseKeyboard = app.acquireKeyboardOwner('modal'); let backdrop: HTMLElement; let cancelDrawerDrag: () => void; // assigned by attachDrawerResize below, before close() can possibly fire let detachBackdrop: () => void; - const onKey = (ev: KeyboardEvent): void => { if (ev.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; + const onKey = (ev: KeyboardEvent): void => { + if (ev.key === 'Escape' && isTopDrawer(doc, backdrop)) { ev.preventDefault(); close(); } + }; function close(): void { cancelDrawerDrag(); detachBackdrop(); if (backdrop) backdrop.remove(); doc.removeEventListener('keydown', onKey, true); + releaseKeyboard(); } const n = entry.rows.length; const { panel } = buildDrawerChrome(doc, { @@ -1118,16 +1123,20 @@ function isTopDrawer(doc: Document, el: Element | undefined): boolean { export function openCellDetail(app: ResultsApp, name: string, type: string, value: unknown, targetDoc?: Document): HTMLElement { const doc = targetDoc || app.document; + const releaseKeyboard = doc === app.document ? app.acquireKeyboardOwner('modal') : () => {}; const text = value == null ? '' : String(value); let backdrop: HTMLElement; let cancelDrawerDrag: () => void; // assigned by attachDrawerResize below, before close() can possibly fire let detachBackdrop: () => void; - const onKey = (e: KeyboardEvent): void => { if (e.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; + const onKey = (e: KeyboardEvent): void => { + if (e.key === 'Escape' && isTopDrawer(doc, backdrop)) { e.preventDefault(); close(); } + }; function close(): void { cancelDrawerDrag(); detachBackdrop(); if (backdrop) backdrop.remove(); doc.removeEventListener('keydown', onKey, true); + releaseKeyboard(); } // withDocument(doc, ...) so every element (including the ones built later, diff --git a/src/ui/shortcuts.ts b/src/ui/shortcuts.ts index 6749d95..a8ca40c 100644 --- a/src/ui/shortcuts.ts +++ b/src/ui/shortcuts.ts @@ -1,199 +1,248 @@ -// Keyboard-shortcuts modal + the global key handler. +// Surface-aware keyboard shortcuts: one catalog drives both dispatch and help. import { h, attachBackdropClose } from './dom.js'; -import type { ActionsRegistry, State, Tab } from './app.types.js'; +import type { ActionsRegistry, KeyboardOwner, State, Tab } from './app.types.js'; import type { ConnectionSession } from '../application/connection-session.js'; import type { SqlRoute } from '../core/sql-route.js'; -/** The narrow slice of the real `app` controller this module reads — not - * the full ~50-member `App` contract (app.types.ts). A real `App` satisfies - * this directly, and so does tests/helpers/fake-app.js's long-standing - * minimal `makeApp()` fixture (this module predates ADR-0002's App - * contract) — no cast needed on either side. */ +type ShortcutSurface = 'workspace' | 'dashboard' | 'all'; +type Section = 'application' | 'workspace' | 'dashboard' | 'general' | 'gestures'; +type ShortcutDispatch = 'application' | 'editor'; +type KeyName = 'mod-enter' | 'mod-shift-enter' | 'mod-s' | 'mod-shift-s' | 'mod-alt-1' | 'mod-alt-2' | 'mod-z' | 'mod-shift-z' | 'f1' | 'g-d' | 'g-w' | 'g-v' | 'g-e' | 'question' | 'escape'; + +export interface ShortcutDefinition { + id: string; + label: string; + section: Section; + surface: ShortcutSurface; + key: KeyName; + dispatch: ShortcutDispatch; + sequence?: readonly [string, string]; + available?: (app: ShortcutsApp) => boolean; + matches?: (e: ShortcutKeydownEvent, app: ShortcutsApp) => boolean; + run?: (e: ShortcutKeydownEvent, app: ShortcutsApp) => string | null; +} + +/** The single shortcut catalogue. Help never documents a command unavailable to + * the dispatcher because both paths resolve this list against the same app. */ +export const SHORTCUT_CATALOG: readonly ShortcutDefinition[] = [ + { id: 'open-dashboard', label: 'Open Dashboard', section: 'application', surface: 'workspace', key: 'g-d', dispatch: 'application', sequence: ['g', 'd'], run: (e, a) => { e.preventDefault(); void a.navigateSqlRoute?.({ surface: 'dashboard', workspaceKey: a.state.workspaceKey, mode: 'edit' }, 'push'); return 'openDashboard'; } }, + { id: 'open-workbench', label: 'Open SQL Browser', section: 'application', surface: 'dashboard', key: 'g-w', dispatch: 'application', sequence: ['g', 'w'], run: (e, a) => { e.preventDefault(); void a.navigateSqlRoute?.({ surface: 'workspace', workspaceKey: a.state.workspaceKey }, 'push'); return 'openWorkbench'; } }, + { id: 'run-query', label: 'Run query', section: 'workspace', surface: 'workspace', key: 'mod-enter', dispatch: 'application', available: (a) => a.activeTab().editorMode !== 'spec', matches: (e) => modKey(e) && e.key === 'Enter' && !e.shiftKey, run: (e, a) => { e.preventDefault(); a.actions.run(); return 'run'; } }, + { id: 'format-document', label: 'Format active document', section: 'workspace', surface: 'workspace', key: 'mod-shift-enter', dispatch: 'application', matches: (e) => modKey(e) && e.key === 'Enter' && !!e.shiftKey, run: (e, a) => { e.preventDefault(); if (a.activeTab().editorMode === 'spec') { a.actions.formatSpec(); return 'formatSpec'; } a.actions.formatQuery(); return 'formatQuery'; } }, + { id: 'save-query', label: 'Save query', section: 'workspace', surface: 'workspace', key: 'mod-s', dispatch: 'application', matches: (e) => modKey(e) && !e.shiftKey && e.key.toLowerCase() === 's', run: (e, a) => { e.preventDefault(); a.actions.save(); return 'save'; } }, + { id: 'share-query', label: 'Share query', section: 'workspace', surface: 'workspace', key: 'mod-shift-s', dispatch: 'application', available: (a) => a.activeTab().editorMode !== 'spec', matches: (e) => modKey(e) && !!e.shiftKey && e.key.toLowerCase() === 's', run: (e, a) => { e.preventDefault(); a.actions.share(); return 'share'; } }, + { id: 'sql-mode', label: 'SQL editor mode', section: 'workspace', surface: 'workspace', key: 'mod-alt-1', dispatch: 'application', matches: (e) => modKey(e) && !!e.altKey && e.key === '1', run: (e, a) => { e.preventDefault(); a.actions.setEditorMode('sql'); return 'sqlMode'; } }, + { id: 'spec-mode', label: 'Spec editor mode', section: 'workspace', surface: 'workspace', key: 'mod-alt-2', dispatch: 'application', matches: (e) => modKey(e) && !!e.altKey && e.key === '2', run: (e, a) => { e.preventDefault(); a.actions.setEditorMode('spec'); return 'specMode'; } }, + { id: 'undo', label: 'Undo', section: 'workspace', surface: 'workspace', key: 'mod-z', dispatch: 'editor' }, + { id: 'redo', label: 'Redo', section: 'workspace', surface: 'workspace', key: 'mod-shift-z', dispatch: 'editor' }, + { id: 'open-reference', label: 'Open reference for symbol', section: 'workspace', surface: 'workspace', key: 'f1', dispatch: 'editor' }, + { id: 'dashboard-refresh', label: 'Refresh all tiles', section: 'dashboard', surface: 'dashboard', key: 'mod-enter', dispatch: 'application', available: (a) => validDashboardPort(a), matches: (e) => modKey(e) && e.key === 'Enter' && !e.shiftKey, run: (e, a) => { e.preventDefault(); a.surfaceCommands!.refresh(); return 'dashboardRefresh'; } }, + { id: 'dashboard-view', label: 'View mode', section: 'dashboard', surface: 'dashboard', key: 'g-v', dispatch: 'application', sequence: ['g', 'v'], run: (e, a) => { e.preventDefault(); if (a.sqlRoute.mode === 'view') return null; void a.navigateSqlRoute?.({ surface: 'dashboard', workspaceKey: a.state.workspaceKey, mode: 'view' }, 'replace'); return 'dashboardView'; } }, + { id: 'dashboard-edit', label: 'Edit mode', section: 'dashboard', surface: 'dashboard', key: 'g-e', dispatch: 'application', sequence: ['g', 'e'], run: (e, a) => { e.preventDefault(); if (a.sqlRoute.mode === 'edit') return null; void a.navigateSqlRoute?.({ surface: 'dashboard', workspaceKey: a.state.workspaceKey, mode: 'edit' }, 'replace'); return 'dashboardEdit'; } }, + { id: 'open-help', label: 'Show this dialog', section: 'general', surface: 'all', key: 'question', dispatch: 'application', matches: (e) => e.key === '?' && !modKey(e) && !isTypingTarget(e.target), run: (e, a) => { e.preventDefault(); a.actions.openShortcuts(); return 'shortcuts'; } }, + { id: 'close-overlay', label: 'Close dialog', section: 'general', surface: 'all', key: 'escape', dispatch: 'application', matches: (e) => e.key === 'Escape', run: (e, a) => { if (a.sqlRoute.surface === 'workspace' && a.closeDocPane?.()) { e.preventDefault(); return 'close-doc-pane'; } if (a.sqlRoute.surface === 'workspace' && a.state.running.value) { e.preventDefault(); a.actions.cancel(); return 'cancel'; } return null; } }, +]; + +const GESTURES = [ + ['Expand / collapse', 'Click'], ['Insert into editor', 'Double-click'], ['Insert DDL / col::type', 'Shift-click'], +] as const; + +export interface SurfaceCommandPort { + surface: 'dashboard'; + generation: number; + refresh(): void; +} + +/** Narrow controller contract; it deliberately avoids importing the full App. */ export interface ShortcutsApp { document?: Document; state: Pick; conn: Pick; - sqlRoute: Pick; + sqlRoute: Pick & { mode?: 'view' | 'edit' }; workspaceRouteStatus: 'loading' | 'ready' | 'not-found' | 'error'; - /** #60 — closes the docs reference pane when one is open (returns true), - * no-op returning false otherwise. Injected by app.ts (bound to - * ui/doc-pane's isDocPaneOpen/closeDocPane) so Esc closes the pane from - * ANYWHERE — not only with focus inside it — layered BEFORE the - * cancel-running-query action below. Optional: minimal test apps omit it. */ + surfaceCommands?: SurfaceCommandPort | null; + keyboardOwner?: KeyboardOwner | null; + acquireKeyboardOwner(kind: KeyboardOwner['kind']): () => void; + captureSurfaceGeneration?: () => number; + navigateSqlRoute?: (route: SqlRoute, method: 'push' | 'replace') => Promise; closeDocPane?: () => boolean; activeTab(): Pick; - actions: Pick< - ActionsRegistry, - 'cancel' | 'run' | 'formatSpec' | 'formatQuery' | 'setEditorMode' | 'share' | 'save' | 'openShortcuts' - >; -} - -const SHORTCUTS: string[][] = [ - ['Run query', '⌘↵'], - ['Format active document', '⌘⇧↵'], - ['Save query', '⌘S'], - ['Share query', '⌘⇧S'], - ['SQL editor mode', '⌘⌥1'], - ['Spec editor mode', '⌘⌥2'], - ['Undo', '⌘Z'], - ['Redo', '⌘⇧Z'], - ['Open reference for symbol', 'F1'], // #313 — in-editor only (CM6 keymap, codemirror-adapter.ts) - ['Show this dialog', '?'], - ['Close dialog', 'Esc'], -]; + actions: Pick; +} -// Mouse gestures on the schema tree (db / table / column). Kept terse — the -// per-row tooltips carry the detail; this just signals the gestures exist. -const GESTURES: string[][] = [ - ['Expand / collapse', 'Click'], - ['Insert into editor', 'Double-click'], - ['Insert DDL / col::type', 'Shift-click'], -]; +function platformIsMac(doc: Document): boolean { + return /mac/i.test(doc.defaultView?.navigator.platform || ''); +} + +function keyParts(key: KeyName, mac: boolean): string[] { + const mod = mac ? '⌘' : 'Ctrl'; const alt = mac ? '⌥' : 'Alt'; const shift = mac ? '⇧' : 'Shift'; + const keys: Record = { + 'mod-enter': [mod, 'Enter'], 'mod-shift-enter': [mod, shift, 'Enter'], 'mod-s': [mod, 'S'], + 'mod-shift-s': [mod, shift, 'S'], 'mod-alt-1': [mod, alt, '1'], 'mod-alt-2': [mod, alt, '2'], + 'mod-z': [mod, 'Z'], 'mod-shift-z': [mod, shift, 'Z'], f1: ['F1'], 'g-d': ['G', 'then', 'D'], + 'g-w': ['G', 'then', 'W'], 'g-v': ['G', 'then', 'V'], 'g-e': ['G', 'then', 'E'], question: ['?'], escape: ['Esc'], + }; + return keys[key]; +} + +function visibleDefinitions(app: ShortcutsApp): ShortcutDefinition[] { + const surface = app.sqlRoute.surface; + return SHORTCUT_CATALOG.filter((definition) => ( + (definition.surface === 'all' || definition.surface === surface) && (!definition.available || definition.available(app)) + )); +} + +function keyCaps(parts: string[]): HTMLElement[] { + return parts.map((part, index) => part === 'then' + ? h('span', { class: 'shortcut-then' }, 'then') + : h('kbd', { key: String(index) }, part)); +} -/** Open the shortcuts modal. Idempotent while open (tracked on state). */ -export function openShortcuts(app: ShortcutsApp): { backdrop: HTMLElement; close: () => void } | null { +const sectionNames: Record = { + application: 'Application', workspace: 'SQL Browser', dashboard: 'Dashboard', general: 'General', gestures: 'Schema tree — database · table · column', +}; + +/** Open accessible, surface-specific help. */ +export function openShortcuts(app: ShortcutsApp, onClose?: () => void): { backdrop: HTMLElement; close: () => void } | null { const doc = app.document || document; if (app.state.shortcutsOpen.value) return null; app.state.shortcutsOpen.value = true; + const releaseKeyboard = app.acquireKeyboardOwner('modal'); + resetShortcutChord(app); + let previousFocus = doc.activeElement as HTMLElement | null; + let closed = false; + const headingId = 'shortcuts-heading'; const close = (): void => { + if (closed) return; + closed = true; app.state.shortcutsOpen.value = false; - detachBackdrop(); - backdrop.remove(); - doc.removeEventListener('keydown', escHandler); + releaseKeyboard(); + resetShortcutChord(app); + detachBackdrop(); backdrop.remove(); doc.removeEventListener('keydown', onKeydown); + previousFocus?.focus?.(); previousFocus = null; + onClose?.(); }; - const escHandler = (e: KeyboardEvent): void => { - if (e.key === 'Escape') close(); + const onKeydown = (event: KeyboardEvent): void => { + if (event.key === 'Escape') { event.preventDefault(); close(); return; } + if (event.key !== 'Tab') return; + const focusable = [...card.querySelectorAll('button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])')] + .filter((element) => !element.hasAttribute('disabled')); + if (!focusable.length) return; + const first = focusable[0]; const last = focusable[focusable.length - 1]; + if (event.shiftKey && doc.activeElement === first) { event.preventDefault(); last.focus(); } + else if (!event.shiftKey && doc.activeElement === last) { event.preventDefault(); first.focus(); } }; - doc.addEventListener('keydown', escHandler); - const rowOf = ([label, key]: string[]): HTMLElement => - h('div', { class: 'row' }, h('span', { class: 'label' }, label), h('kbd', null, key)); - const card = h('div', { class: 'modal-card' }, - h('h2', null, 'Keyboard shortcuts'), - ...SHORTCUTS.map(rowOf), - h('div', { class: 'section-label' }, 'Schema tree — database · table · column'), - ...GESTURES.map(rowOf), - h('div', { class: 'close-row' }, h('button', { class: 'close-btn', onclick: close }, 'Close')), + const rows = visibleDefinitions(app); + const mac = platformIsMac(doc); + const sections: HTMLElement[] = []; + for (const section of ['application', 'workspace', 'dashboard', 'general'] as const) { + const entries = rows.filter((row) => row.section === section); + if (!entries.length) continue; + sections.push(h('section', { class: 'shortcut-section', 'aria-label': sectionNames[section] }, + h('h3', { class: 'section-label' }, sectionNames[section]), + ...entries.map((row) => h('div', { class: 'row' }, h('span', { class: 'label' }, row.label), + h('span', { class: 'shortcut-keys' }, ...keyCaps(keyParts(row.key, mac))))), + )); + } + if (app.sqlRoute.surface === 'workspace') { + sections.push(h('section', { class: 'shortcut-section', 'aria-label': sectionNames.gestures }, + h('h3', { class: 'section-label' }, sectionNames.gestures), + ...GESTURES.map(([label, key]) => h('div', { class: 'row' }, h('span', { class: 'label' }, label), h('kbd', null, key))), + )); + } + const closeButton = h('button', { class: 'close-btn', 'aria-label': 'Close keyboard shortcuts', onclick: close }, 'Close'); + const card = h('div', { class: 'modal-card', role: 'dialog', 'aria-modal': 'true', 'aria-labelledby': headingId }, + h('h2', { id: headingId }, 'Keyboard shortcuts'), + h('div', { class: 'modal-card-body' }, ...sections), + h('div', { class: 'close-row' }, closeButton), ); const backdrop = h('div', { class: 'modal-backdrop' }, card); const detachBackdrop = attachBackdropClose(backdrop, close); doc.body.appendChild(backdrop); + doc.addEventListener('keydown', onKeydown); + closeButton.focus(); return { backdrop, close }; } -/** The event target shape `handleKeydown`'s ⌘A/`?` arms duck-type over — a - * real `Element` satisfies it directly (structurally); test fixtures pass a - * plain object instead of a full DOM node. */ export interface ShortcutEventTarget { - tagName?: string; - isContentEditable?: boolean; - ownerDocument?: Document | null; + tagName?: string; isContentEditable?: boolean; ownerDocument?: Document | null; + getAttribute?(name: string): string | null; closest?(selector: string): Element | null; } - -/** The minimal keydown-event shape this handler reads. A real `KeyboardEvent` - * (from the app's global `keydown` listener) satisfies it directly; tests - * build a small plain-object fixture instead of a real event. */ export interface ShortcutKeydownEvent { - key: string; - metaKey?: boolean; - ctrlKey?: boolean; - shiftKey?: boolean; - altKey?: boolean; - defaultPrevented?: boolean; - preventDefault(): void; - target?: ShortcutEventTarget | null; -} - -/** - * Handle a global keydown. Returns the action name it dispatched (or null). - * `app` provides state + the action callbacks; `signedIn` gates editing keys. - */ + key: string; metaKey?: boolean; ctrlKey?: boolean; shiftKey?: boolean; altKey?: boolean; + defaultPrevented?: boolean; preventDefault(): void; target?: ShortcutEventTarget | null; +} + +function isTypingTarget(target?: ShortcutEventTarget | null): boolean { + if (!target) return false; + return ['INPUT', 'TEXTAREA', 'SELECT'].includes(target.tagName || '') || !!target.isContentEditable + || target.getAttribute?.('role') === 'textbox' || !!target.closest?.('.cm-editor, .cm-content, [contenteditable], [role="textbox"]'); +} +function ownsKeyboard(app: ShortcutsApp): boolean { return !!app.keyboardOwner; } +function ready(app: ShortcutsApp): boolean { + return app.workspaceRouteStatus === 'ready' + && !!app.sqlRoute.workspaceKey && app.sqlRoute.workspaceKey === app.state.workspaceKey; +} + +interface Chord { timer: ReturnType | null; blur: () => void; surface: 'workspace' | 'dashboard'; workspaceKey: string | null; generation: number | null; } +const chords = new WeakMap(); +export function resetShortcutChord(app: ShortcutsApp): void { + const chord = chords.get(app); if (!chord) return; + if (chord.timer) clearTimeout(chord.timer); + (app.document || document).defaultView?.removeEventListener('blur', chord.blur); + chords.delete(app); +} +function beginChord(app: ShortcutsApp): void { + resetShortcutChord(app); + const chord: Chord = { timer: null, blur: () => resetShortcutChord(app), surface: app.sqlRoute.surface, workspaceKey: app.sqlRoute.workspaceKey, generation: app.captureSurfaceGeneration?.() ?? null }; + chord.timer = setTimeout(() => resetShortcutChord(app), 1500); + chords.set(app, chord); + const win = (app.document || document).defaultView; + win?.addEventListener('blur', chord.blur, { once: true }); +} +function consumeChord(e: ShortcutKeydownEvent, app: ShortcutsApp): string | null | undefined { + const chord = chords.get(app); if (!chord) return undefined; + resetShortcutChord(app); + const stillCurrent = chord.surface === app.sqlRoute.surface && chord.workspaceKey === app.sqlRoute.workspaceKey + && (chord.generation === null || chord.generation === app.captureSurfaceGeneration?.()); + if (!stillCurrent) return null; + const key = e.key.toLowerCase(); + if (key === 'g') { beginChord(app); e.preventDefault(); return 'chord'; } + const command = visibleDefinitions(app).find((definition) => definition.sequence?.[0] === 'g' + && definition.sequence[1] === key); + return command?.run ? command.run(e, app) : null; +} + +/** Global dispatcher. Commands are gated by current route, identity and surface. */ export function handleKeydown(e: ShortcutKeydownEvent, app: ShortcutsApp): string | null { - // A key the editor already consumed (CM6 preventDefaults what it handles — - // e.g. Esc closing the completion popup or search panel) must not ALSO - // trigger a global action like cancelling the running query. if (e.defaultPrevented) return null; - // Fail closed unless the visible Workbench and projected workspace agree - // with the canonical ready route. During async navigation the old editor - // session still exists as an object even after its DOM has been removed. - if (app.workspaceRouteStatus !== 'ready' - || app.sqlRoute.surface !== 'workspace' - || app.sqlRoute.workspaceKey !== app.state.workspaceKey) return null; - const mod = e.metaKey || e.ctrlKey; - const signedIn = app.conn.isSignedIn(); - const editorMode = app.activeTab().editorMode || 'sql'; - // Esc closes the docs reference pane first — from anywhere, not only with - // focus inside it (#60 live finding). A second Esc then cancels a query. - if (e.key === 'Escape' && app.closeDocPane?.()) { - e.preventDefault(); - return 'close-doc-pane'; - } - // Esc cancels an in-flight query (aborts the stream + KILL QUERY). - if (e.key === 'Escape' && app.state.running.value) { - e.preventDefault(); - app.actions.cancel(); - return 'cancel'; - } - if (mod && e.key === 'Enter') { - // Format targets the active document. Plain Mod-Enter is SQL-only. - if (e.shiftKey) { - if (!signedIn) return null; - e.preventDefault(); - if (editorMode === 'spec') { - app.actions.formatSpec(); - return 'formatSpec'; - } - app.actions.formatQuery(); - return 'formatQuery'; - } - if (editorMode !== 'sql') return null; - e.preventDefault(); - app.actions.run(); - return 'run'; - } - if (mod && e.altKey && (e.key === '1' || e.key === '2')) { - if (!signedIn) return null; - e.preventDefault(); - const mode = e.key === '1' ? 'sql' : 'spec'; - app.actions.setEditorMode(mode); - return mode + 'Mode'; - } - if (mod && e.shiftKey && e.key.toLowerCase() === 's') { - if (!signedIn || editorMode !== 'sql') return null; - e.preventDefault(); - app.actions.share(); - return 'share'; - } - if (mod && e.key.toLowerCase() === 's') { - if (!signedIn) return null; - e.preventDefault(); - app.actions.save(); - return 'save'; - } + if (!ready(app)) { resetShortcutChord(app); return null; } + const mod = modKey(e); + if (ownsKeyboard(app)) { resetShortcutChord(app); return null; } + if (!app.conn.isSignedIn()) { resetShortcutChord(app); return null; } + if (e.key === 'Escape') resetShortcutChord(app); + const command = visibleDefinitions(app).find((definition) => definition.matches?.(e, app)); + if (command?.run) return command.run(e, app); if (mod && e.key.toLowerCase() === 'a') { - // When a selectable text pane is on screen and the user isn't typing, - // ⌘/Ctrl+A selects just that text so it can be copied — not the whole page. - // Keyed off "not editing + pane present" rather than pane focus, because - // macOS WebKit doesn't focus a tabindex
on click (so e.target stays - // ). A focused editor/input keeps the native select-all (whole query). - // The cell-detail drawer (.cd-pre) is a modal overlay — when open it wins - // over the result pane behind it, so select all of *its* text. - const t = e.target; - if (t && (t.tagName === 'TEXTAREA' || t.tagName === 'INPUT' || t.isContentEditable)) return null; - const doc = (t && t.ownerDocument) || document; + if (isTypingTarget(e.target)) return null; + const doc = e.target?.ownerDocument || app.document || document; const box = doc.querySelector('.cd-pre') || doc.querySelector('.raw-text-view, .json-view'); - if (!box) return null; - e.preventDefault(); - box.ownerDocument.defaultView!.getSelection()!.selectAllChildren(box); - return 'selectAll'; + if (!box) return null; e.preventDefault(); box.ownerDocument.defaultView!.getSelection()!.selectAllChildren(box); return 'selectAll'; } - if (e.key === '?' && !mod) { - const t = e.target; - if (t && (t.tagName === 'INPUT' || t.tagName === 'TEXTAREA' || t.isContentEditable)) return null; - if (!signedIn) return null; - e.preventDefault(); - app.actions.openShortcuts(); - return 'shortcuts'; + if (!mod && !isTypingTarget(e.target)) { + const result = consumeChord(e, app); if (result !== undefined) return result; + if (visibleDefinitions(app).some((definition) => definition.sequence?.[0] === e.key.toLowerCase())) { + beginChord(app); e.preventDefault(); return 'chord'; + } } return null; } + +function modKey(e: ShortcutKeydownEvent): boolean { return !!(e.metaKey || e.ctrlKey); } +function validDashboardPort(app: ShortcutsApp): boolean { + const port = app.surfaceCommands; + return !!port && port.surface === 'dashboard' + && port.generation === (app.captureSurfaceGeneration?.() ?? port.generation); +} diff --git a/src/ui/time-range-field.ts b/src/ui/time-range-field.ts index 0d693bd..b91deee 100644 --- a/src/ui/time-range-field.ts +++ b/src/ui/time-range-field.ts @@ -48,6 +48,7 @@ import { idSafe } from './combobox.js'; import { openAnchoredDialog } from './popover.js'; import { formatTimeRangeDisplayValue, validateTimeRangeDraft } from '../core/time-range.js'; import type { DashboardTimeRangeGroup, TimeRangeRecent, TimeRangeBoundDraft } from '../core/time-range.js'; +import type { KeyboardOwner } from './app.types.js'; import { TIME_RANGE_CONSTANTS, filterTokenList } from './relative-time-field.js'; /** `buildTimeRangeField`'s options bag. */ @@ -72,6 +73,7 @@ export interface TimeRangeFieldOpts { getRecents: () => readonly TimeRangeRecent[]; /** Both the Apply button and a recents pick route here, with TRIMMED text. */ onApply: (from: string, to: string) => void; + onKeyboardOwnerChange?: (owner: KeyboardOwner | null) => void; } /** `buildTimeRangeField`'s return value. */ @@ -344,6 +346,7 @@ export function buildTimeRangeField(opts: TimeRangeFieldOpts): TimeRangeFieldHan minWidthFromTrigger: false, initialFocus: () => fromInput, onClose: () => { closeCurrent = null; }, + onKeyboardOwnerChange: opts.onKeyboardOwnerChange, }); closeCurrent = (closeOpts) => handle.close(closeOpts); openingFocus = false; diff --git a/tests/helpers/fake-app.ts b/tests/helpers/fake-app.ts index 1160706..81d25c4 100644 --- a/tests/helpers/fake-app.ts +++ b/tests/helpers/fake-app.ts @@ -432,6 +432,30 @@ const appDefaults: App = { sqlRoute: { surface: 'workspace', workspaceKey: null }, currentWorkspace: null, workspaceRouteStatus: 'ready', + keyboardOwner: null, + acquireKeyboardOwner(kind) { + const token = { kind }; + const host = this as App & { __keyboardOwners?: Array }; + const owners = (host.__keyboardOwners ??= []); + owners.push(token); + this.keyboardOwner = token; + let released = false; + return () => { + if (released) return; + released = true; + const index = owners.indexOf(token); + if (index >= 0) owners.splice(index, 1); + this.keyboardOwner = owners.at(-1) ?? null; + }; + }, + resetShortcutChord: () => {}, + shortcutDialog: null, + closeShortcutDialog() { + const dialog = this.shortcutDialog; + this.shortcutDialog = null; + dialog?.close(); + }, + surfaceCommands: null, captureSurfaceGeneration: () => 0, isSurfaceGenerationCurrent: (generation) => generation === 0, refreshCurrentSurfaceAfterStale: (generation) => generation === 0, diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index f524bfe..1b8cda3 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -5652,6 +5652,71 @@ describe('unified /sql routing', () => { expect(qs(app.root, '.workspace-loading')).toBeNull(); }); + it('closes Dashboard shortcut help before a Dashboard-to-Workbench popstate', async () => { + const location = { + origin: 'https://ch.example', pathname: '/sql', + search: '?ws=a&surface=dashboard&mode=view', hash: '', host: 'ch.example', + href: 'https://ch.example/sql?ws=a&surface=dashboard&mode=view', + } as Location; + const app = createApp(env({ location })); + const workspace: StoredWorkspaceV2 = { + storageVersion: 2, id: 'a', key: 'a', name: 'A', queries: [], dashboard: null, + }; + app.applyCommittedWorkspace(workspace); + await app.renderDashboard(); + app.actions.openShortcuts(); + expect(app.state.shortcutsOpen.value).toBe(true); + expect(document.querySelector('.modal-backdrop')).not.toBeNull(); + location.search = '?ws=a'; + await app.handleSqlPopState(); + expect(app.state.shortcutsOpen.value).toBe(false); + expect(document.querySelector('.modal-backdrop')).toBeNull(); + expect(qs(app.root, '.workbench')).not.toBeNull(); + }); + + it('closes Workbench shortcut help before Dashboard navigation without restoring detached focus', async () => { + const app = createApp(env()); + const workspace: StoredWorkspaceV2 = { + storageVersion: 2, id: 'a', key: 'a', name: 'A', queries: [], dashboard: null, + }; + app.applyCommittedWorkspace(workspace); + app.sqlRoute = { surface: 'workspace', workspaceKey: 'a' }; + app.renderApp(); + const oldFocus = qs(app.root, 'button'); + oldFocus.focus(); + app.actions.openShortcuts(); + await app.navigateSqlRoute({ surface: 'dashboard', workspaceKey: 'a', mode: 'edit' }, 'push'); + expect(app.state.shortcutsOpen.value).toBe(false); + expect(document.querySelector('.modal-backdrop')).toBeNull(); + expect(oldFocus.isConnected).toBe(false); + expect(document.activeElement).not.toBe(oldFocus); + }); + + it('authentication loss tears down shortcut help and leaves only Login', () => { + const app = createApp(env()); + app.renderApp(); + app.actions.openShortcuts(); + expect(app.state.shortcutsOpen.value).toBe(true); + app.showLogin('Session expired'); + expect(app.state.shortcutsOpen.value).toBe(false); + expect(document.querySelector('.modal-backdrop')).toBeNull(); + expect(app.root!.textContent).toContain('Session expired'); + expect(app.keyboardOwner).toBeNull(); + }); + + it('releases keyboard-owner acquisitions by identity and idempotently', () => { + const app = createApp(env()); + app.resetShortcutChord(); + const releaseMenu = app.acquireKeyboardOwner('menu'); + const releaseModal = app.acquireKeyboardOwner('modal'); + expect(app.keyboardOwner?.kind).toBe('modal'); + releaseMenu(); + expect(app.keyboardOwner?.kind).toBe('modal'); + releaseMenu(); + releaseModal(); + expect(app.keyboardOwner).toBeNull(); + }); + it('keeps imported favorite queries visible after visiting an empty Dashboard', async () => { const imported = savedQueryFixture({ id: 'imported-favorite', name: 'Imported favorite', sql: 'SELECT 1', favorite: true, diff --git a/tests/unit/dashboard.test.ts b/tests/unit/dashboard.test.ts index 547e4a3..6cb170e 100644 --- a/tests/unit/dashboard.test.ts +++ b/tests/unit/dashboard.test.ts @@ -4116,6 +4116,18 @@ describe('renderDashboard — unified live modes (#407)', () => { expect(qs(app.root, '.dash-contextbar')).toBeNull(); }); + it('registers only the mounted viewer as the Dashboard shortcut refresh port', async () => { + const { app } = modeApp({ workspace: wsWith(), mode: 'edit' }); + await render(app); + const port = app.surfaceCommands; + expect(port?.surface).toBe('dashboard'); + expect(port?.generation).toBe(app.captureSurfaceGeneration()); + expect(port?.refresh).toBeTypeOf('function'); + port?.refresh(); + app.renderDashboard(); + expect(app.surfaceCommands).not.toBe(port); + }); + it('an old Dashboard refresh hook is inert after the route leaves Dashboard', async () => { const { app } = modeApp({ workspace: wsWith(), mode: 'view' }); await render(app); diff --git a/tests/unit/detached-view.test.ts b/tests/unit/detached-view.test.ts index fecaefc..868f5d6 100644 --- a/tests/unit/detached-view.test.ts +++ b/tests/unit/detached-view.test.ts @@ -2,6 +2,8 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { signal } from '@preact/signals-core'; import { openInDetachedTab } from '../../src/ui/detached-view.js'; import type { DetachedViewApp, DetachedWindowLike, MountCtx } from '../../src/ui/detached-view.js'; +import { handleKeydown } from '../../src/ui/shortcuts.js'; +import { makeApp } from '../helpers/fake-app.js'; const qs = (root: ParentNode, selector: string): T => root.querySelector(selector) as T; @@ -157,6 +159,18 @@ describe('openInDetachedTab — overlay fallback', () => { expect(document.body.contains(overlay)).toBe(false); }); + it('owns the application keyboard until the fallback overlay closes', () => { + const app = makeApp({ openWindow: () => null }); + const view = openInDetachedTab(app, { title: 'Widget', mode: 'graph', mount: () => {} }); + expect(app.keyboardOwner?.kind).toBe('modal'); + expect(handleKeydown({ + key: 'Enter', metaKey: true, preventDefault: vi.fn(), target: document.body, + }, app)).toBeNull(); + expect(app.actions.run).not.toHaveBeenCalled(); + view.close(); + expect(app.keyboardOwner).toBeNull(); + }); + it('does not append closeBtn anywhere itself — a mount() that ignores it gets no ✕ at all', () => { openInDetachedTab({ document, openWindow: () => null, state: detachedState() }, { title: 'X', mode: 'graph', mount: () => {}, // never touches closeBtn diff --git a/tests/unit/file-menu.test.ts b/tests/unit/file-menu.test.ts index ff8b272..187f24f 100644 --- a/tests/unit/file-menu.test.ts +++ b/tests/unit/file-menu.test.ts @@ -11,6 +11,7 @@ import { savedQuery } from '../helpers/saved-query.js'; import type { SavedQueryFixture } from '../helpers/saved-query.js'; import type { App } from '../../src/ui/app.types.js'; import type { DashboardDocumentV1, PortableBundleV1, SavedQueryV2, StoredWorkspaceV2 } from '../../src/generated/json-schema.types.js'; +import { handleKeydown } from '../../src/ui/shortcuts.js'; const click = (el: Element): boolean => el.dispatchEvent(new Event('click', { bubbles: true })); const key = (target: EventTarget, k: string, mods: KeyboardEventInit = {}): boolean => @@ -441,7 +442,26 @@ describe('Import queries', () => { const dialog = document.querySelector('.fm-dialog-card')!; expect(dialog.textContent).toContain('Resolve 1 conflicting query'); expect(dialog.textContent).toContain('OldName'); // row shows the EXISTING query's name + const shortcut = (keyValue: string, mods: { metaKey?: boolean; altKey?: boolean; shiftKey?: boolean } = {}) => ({ + key: keyValue, preventDefault: vi.fn(), target: document.body, ...mods, + }); + expect(app.keyboardOwner?.kind).toBe('modal'); + expect(handleKeydown(shortcut('Enter', { metaKey: true }), app)).toBeNull(); + expect(handleKeydown(shortcut('s', { metaKey: true }), app)).toBeNull(); + expect(handleKeydown(shortcut('1', { metaKey: true, altKey: true }), app)).toBeNull(); + expect(handleKeydown(shortcut('g'), app)).toBeNull(); + expect(app.actions.run).not.toHaveBeenCalled(); + expect(app.actions.save).not.toHaveBeenCalled(); + expect(app.actions.setEditorMode).not.toHaveBeenCalled(); + app.sqlRoute = { surface: 'dashboard', workspaceKey: app.state.workspaceKey, mode: 'view' }; + app.surfaceCommands = { surface: 'dashboard', generation: 0, refresh: vi.fn() }; + expect(handleKeydown(shortcut('Enter', { metaKey: true }), app)).toBeNull(); + expect(app.surfaceCommands.refresh).not.toHaveBeenCalled(); + app.sqlRoute = { surface: 'workspace', workspaceKey: app.state.workspaceKey }; click(document.querySelector('.fm-dialog-confirm')!); // Apply with the default (use-existing) + expect(app.keyboardOwner?.kind).toBe('menu'); // the still-mounted File menu remains underneath + disposeFileMenuOverlays(app); + expect(app.keyboardOwner).toBeNull(); await flush(); expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['OldName']); expect(toast()).toBe('Imported 1 query'); diff --git a/tests/unit/results.test.ts b/tests/unit/results.test.ts index 38dacf5..deb9cbc 100644 --- a/tests/unit/results.test.ts +++ b/tests/unit/results.test.ts @@ -2,6 +2,8 @@ import { describe, it, expect, vi } from 'vitest'; import { renderResults, renderJson, renderTable, openCellDetail, openRowsViewer, expandDataPane, } from '../../src/ui/results.js'; +import { handleKeydown } from '../../src/ui/shortcuts.js'; +import type { ShortcutKeydownEvent } from '../../src/ui/shortcuts.js'; import type { QueryResult, ScriptResult, ScriptExportResult, ScriptEntry, ScriptExportEntry, } from '../../src/ui/results.js'; @@ -1578,12 +1580,33 @@ describe('multiquery script grid (#83)', () => { it('Escape closes only the topmost stacked drawer (cell first, then the rows pane)', () => { const app = makeApp(); openRowsViewer(app, { columns: [{ name: 'n', type: 'String' }], rows: [['x']] }); + expect(app.keyboardOwner?.kind).toBe('modal'); click(qs(document, '.cd-backdrop tbody td.cell')); // opens a stacked cell drawer expect(qsa(document, '.cd-backdrop')).toHaveLength(2); document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); expect(qsa(document, '.cd-backdrop')).toHaveLength(1); // only the cell drawer closed + expect(app.keyboardOwner?.kind).toBe('modal'); // rows viewer still owns the keyboard document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); expect(qsa(document, '.cd-backdrop')).toHaveLength(0); // now the rows pane + expect(app.keyboardOwner).toBeNull(); + }); + + it('blocks application shortcuts and consumes Escape before a running-query cancel', () => { + const app = makeApp(); + app.state.running.value = true; + openRowsViewer(app, { columns: [{ name: 'n', type: 'String' }], rows: [['x']] }); + expect(handleKeydown({ + key: 'Enter', metaKey: true, preventDefault: vi.fn(), target: document.body, + }, app)).toBeNull(); + expect(app.actions.run).not.toHaveBeenCalled(); + const dispatchGlobal = (event: KeyboardEvent): void => { + handleKeydown(event as unknown as ShortcutKeydownEvent, app); + }; + document.addEventListener('keydown', dispatchGlobal); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + document.removeEventListener('keydown', dispatchGlobal); + expect(qsa(document, '.cd-backdrop')).toHaveLength(0); + expect(app.actions.cancel).not.toHaveBeenCalled(); }); it('toolbar shows live elapsed + Cancel while running, with a running footer', () => { diff --git a/tests/unit/shortcuts.test.ts b/tests/unit/shortcuts.test.ts index 814721c..2b8e0b0 100644 --- a/tests/unit/shortcuts.test.ts +++ b/tests/unit/shortcuts.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { openShortcuts, handleKeydown } from '../../src/ui/shortcuts.js'; +import { SHORTCUT_CATALOG, openShortcuts, handleKeydown, resetShortcutChord } from '../../src/ui/shortcuts.js'; import type { ShortcutKeydownEvent } from '../../src/ui/shortcuts.js'; import { makeApp } from '../helpers/fake-app.js'; @@ -13,6 +13,7 @@ describe('openShortcuts', () => { expect(document.querySelector('.modal-backdrop')).not.toBeNull(); expect(openShortcuts(app)).toBeNull(); // already open r!.close(); + r!.close(); // idempotent lifecycle handle expect(app.state.shortcutsOpen.value).toBe(false); expect(document.querySelector('.modal-backdrop')).toBeNull(); }); @@ -69,12 +70,47 @@ describe('openShortcuts', () => { expect(text).toContain('Double-click'); expect(text).toContain('Shift-click'); }); + it('renders Dashboard-only help from the shared catalog and supplies dialog accessibility', () => { + const refresh = vi.fn(); + const app = makeApp({ document, sqlRoute: { surface: 'dashboard', workspaceKey: 'w', mode: 'view' }, + surfaceCommands: { surface: 'dashboard', generation: 0, refresh } }); + const invoke = document.createElement('button'); document.body.appendChild(invoke); invoke.focus(); + const opened = openShortcuts(app)!; + const card = document.querySelector('.modal-card')!; + expect(card.getAttribute('role')).toBe('dialog'); + expect(card.getAttribute('aria-modal')).toBe('true'); + expect(card.textContent).toContain('Refresh all tiles'); + expect(card.textContent).toContain('Open SQL Browser'); + expect(card.textContent).not.toContain('Run query'); + expect(card.textContent).not.toContain('Schema tree'); + expect(card.querySelectorAll('kbd').length).toBeGreaterThan(0); + const close = card.querySelector('.close-btn')!; + close.focus(); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', bubbles: true, cancelable: true })); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', shiftKey: true, bubbles: true, cancelable: true })); + close.remove(); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', bubbles: true, cancelable: true })); + opened.close(); + expect(document.activeElement).toBe(invoke); + }); + it('omits Dashboard refresh when no viewer session is mounted', () => { + const app = makeApp({ document, sqlRoute: { surface: 'dashboard', workspaceKey: 'w', mode: 'view' }, surfaceCommands: null }); + openShortcuts(app); + expect(document.querySelector('.modal-card')!.textContent).not.toContain('Refresh all tiles'); + }); }); describe('handleKeydown', () => { const ev = (over: Partial = {}): ShortcutKeydownEvent => ({ preventDefault: vi.fn(), key: '', metaKey: false, ctrlKey: false, shiftKey: false, target: {}, ...over }); + it('gives every application-dispatched catalog entry executable metadata', () => { + for (const command of SHORTCUT_CATALOG.filter((entry) => entry.dispatch === 'application')) { + expect(command.run, command.id).toBeTypeOf('function'); + expect(!!command.matches || !!command.sequence, command.id).toBe(true); + } + }); + it('does not dispatch hidden Workbench shortcuts on a Dashboard route', () => { const app = makeApp({ sqlRoute: { surface: 'dashboard', workspaceKey: 'w', mode: 'view' }, @@ -85,6 +121,76 @@ describe('handleKeydown', () => { expect(app.actions.run).not.toHaveBeenCalled(); }); + it('dispatches Dashboard refresh and mode navigation only for the current viewer generation', () => { + const refresh = vi.fn(); + const app = makeApp({ sqlRoute: { surface: 'dashboard', workspaceKey: 'sql_library', mode: 'edit' }, + captureSurfaceGeneration: () => 7, + surfaceCommands: { surface: 'dashboard', generation: 7, refresh }, + navigateSqlRoute: vi.fn(async () => {}) }); + expect(handleKeydown(ev({ metaKey: true, key: 'Enter' }), app)).toBe('dashboardRefresh'); + expect(refresh).toHaveBeenCalledOnce(); + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + expect(handleKeydown(ev({ key: 'v' }), app)).toBe('dashboardView'); + expect(app.navigateSqlRoute).toHaveBeenCalledWith({ surface: 'dashboard', workspaceKey: 'sql_library', mode: 'view' }, 'replace'); + (app.sqlRoute as { mode?: 'view' | 'edit' }).mode = 'view'; + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + expect(handleKeydown(ev({ key: 'e' }), app)).toBe('dashboardEdit'); + expect(app.navigateSqlRoute).toHaveBeenLastCalledWith({ surface: 'dashboard', workspaceKey: 'sql_library', mode: 'edit' }, 'replace'); + const viewing = makeApp({ sqlRoute: { surface: 'dashboard', workspaceKey: 'sql_library', mode: 'view' } }); + expect(handleKeydown(ev({ key: 'g' }), viewing)).toBe('chord'); + expect(handleKeydown(ev({ key: 'v' }), viewing)).toBeNull(); + const editing = makeApp({ sqlRoute: { surface: 'dashboard', workspaceKey: 'sql_library', mode: 'edit' } }); + expect(handleKeydown(ev({ key: 'g' }), editing)).toBe('chord'); + expect(handleKeydown(ev({ key: 'e' }), editing)).toBeNull(); + app.surfaceCommands = { surface: 'dashboard', generation: 6, refresh }; + expect(handleKeydown(ev({ metaKey: true, key: 'Enter' }), app)).toBeNull(); + }); + + it('routes the G chord by current surface, and resets it on mismatch, timeout, blur and stale generation', () => { + vi.useFakeTimers(); + const navigateSqlRoute = vi.fn(async () => {}); + let generation = 1; + const app = makeApp({ navigateSqlRoute, captureSurfaceGeneration: () => generation }); + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + // A repeated G deliberately restarts the chord's expiration window. + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + expect(handleKeydown(ev({ key: 'x' }), app)).toBeNull(); + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + generation = 2; + expect(handleKeydown(ev({ key: 'd' }), app)).toBeNull(); + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + expect(handleKeydown(ev({ key: 'd' }), app)).toBe('openDashboard'); + expect(navigateSqlRoute).toHaveBeenLastCalledWith({ surface: 'dashboard', workspaceKey: 'sql_library', mode: 'edit' }, 'push'); + app.sqlRoute = { surface: 'dashboard', workspaceKey: 'sql_library', mode: 'view' }; + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + expect(handleKeydown(ev({ key: 'w' }), app)).toBe('openWorkbench'); + expect(navigateSqlRoute).toHaveBeenLastCalledWith({ surface: 'workspace', workspaceKey: 'sql_library' }, 'push'); + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + window.dispatchEvent(new Event('blur')); + expect(handleKeydown(ev({ key: 'w' }), app)).toBeNull(); + expect(handleKeydown(ev({ key: 'g' }), app)).toBe('chord'); + vi.advanceTimersByTime(1500); + expect(handleKeydown(ev({ key: 'w' }), app)).toBeNull(); + resetShortcutChord(app); + vi.useRealTimers(); + }); + + it('keeps surface actions behind keyboard-owning overlays and ignores plain keys while typing', () => { + const refresh = vi.fn(); + const app = makeApp({ sqlRoute: { surface: 'dashboard', workspaceKey: 'sql_library', mode: 'view' }, + surfaceCommands: { surface: 'dashboard', generation: 0, refresh } }); + app.keyboardOwner = { kind: 'popover' }; + expect(handleKeydown(ev({ metaKey: true, key: 'Enter' }), app)).toBeNull(); + app.keyboardOwner = null; + expect(handleKeydown(ev({ key: 'g', target: { tagName: 'SELECT' } }), app)).toBeNull(); + expect(handleKeydown(ev({ key: 'g', target: { getAttribute: () => 'textbox' } }), app)).toBeNull(); + expect(handleKeydown(ev({ key: 'g', target: { + closest: (selector) => selector.includes('[role="textbox"]') ? document.body : null, + } }), app)).toBeNull(); + app.keyboardOwner = { kind: 'modal' }; + expect(handleKeydown(ev({ key: 'x' }), app)).toBeNull(); + }); + it('fails closed for every Workbench action shortcut while the route is loading', () => { const app = makeApp({ workspaceRouteStatus: 'loading' }); const shortcuts = [ @@ -120,10 +226,21 @@ describe('handleKeydown', () => { expect(app.actions.run).not.toHaveBeenCalled(); }); - it('⌘Enter runs (even when signed out)', () => { - const app = makeApp({ conn: { isSignedIn: () => false } }); + it('runs a SQL document when signed in', () => { + const app = makeApp(); expect(handleKeydown(ev({ metaKey: true, key: 'Enter' }), app)).toBe('run'); - expect(app.actions.run).toHaveBeenCalled(); + expect(app.actions.run).toHaveBeenCalledOnce(); + }); + + it('fails closed for every application action when signed out', () => { + const app = makeApp({ conn: { isSignedIn: () => false } }); + for (const event of [ + { metaKey: true, key: 'Enter' }, { metaKey: true, key: 's' }, + { metaKey: true, shiftKey: true, key: 'Enter' }, { key: 'g' }, { key: '?' }, + ]) expect(handleKeydown(ev(event), app)).toBeNull(); + expect(app.actions.run).not.toHaveBeenCalled(); + expect(app.actions.save).not.toHaveBeenCalled(); + expect(app.actions.formatQuery).not.toHaveBeenCalled(); }); it('a key the editor already consumed (defaultPrevented) never triggers a global action', () => { const app = makeApp();