Skip to content
This repository was archived by the owner on Oct 26, 2026. It is now read-only.

Commit de96f3d

Browse files
fix(app-shell,i18n): the keyboard-shortcuts dialog lists only shortcuts a mounted handler advertises (objectui#11674) (#11708)
Fixes #11674 Clause-②: no (narrowing) The claim wrote `Clause-②: no`, with the rule that it becomes `no (narrowing)` if published `console.shortcuts.*` locale keys retire. Seven do, so the line reads `no (narrowing)`. No accept set widens, and `@object-ui/app-shell` exports nothing new. ## What was wrong (measured on `origin/main` at `c3623eb`) The dialog was a static list, kept apart from every key handler. Every row, by the handler it had: | Old row | Handler on `main` | Verdict | |---|---|---| | `⌘K` Open command palette | `CommandPaletteProvider` keydown (`k` with ⌘ or Ctrl) | wired | | `?` Show keyboard shortcuts | the dialog's own keydown | wired | | `Esc` Close dialog / panel | Radix dismiss, on every `Dialog` and `Sheet` | wired | | `B` Toggle sidebar | the `SidebarProvider` listener (synced Shadcn primitive) answers `b` with ⌘ or Ctrl only. A bare `B` does nothing | wrong chord | | `⌘/` Focus search | none | dead | | `N` Create new record | none. `useCrudShortcuts` / `useKeyboardShortcuts` have no caller outside their module | dead | | `R` Refresh data | none. The only `r` listener is the approvals inbox's reject key, on that page only | dead | | `⌘E` Edit selected record | none | dead | | `⌘⇧O` New chat, `⌘⇧S` Toggle conversations list | `AiChatPage` only (`matchAiChatShortcut`). The dialog mounts in `AppContent` (`/apps/...`) and `/ai` is not under it, so neither handler is mounted where the rows were shown | not wired where listed | | `⌘D` Toggle dark mode | none | dead | The card named five of these. The order asked for every row to be measured, which added `R` and the AI pair. ## What changed - **`packages/app-shell/src/chrome/advertisedShortcuts.ts` (new).** A small module store, the same shape as `assistantBus`. The code that owns a handler calls `useAdvertiseShortcut(descriptor, enabled?)` beside it, so the shortcut is advertised while the handler is mounted. `useAdvertisedShortcuts()` reads the current set. A descriptor is `{ id, group, chord, label }`. The keycaps come from the chord (`shortcutKeycaps`), and `label` is a literal `t('…')` call so the i18n gates read the key (see the probe below). - **`KeyboardShortcutsDialog`** has no list of its own any more. It renders the advertised shortcuts, grouped (`general`, `navigation`, `aiChat`) and sorted by their text, and each row carries `data-shortcut-id`. It advertises its own `?`, and `Esc` (Radix's dismiss) beside the `Dialog` that `Esc` closes. - **`CommandPaletteProvider`** advertises `⌘K` beside its keydown handler. - **`ConsoleLayout`'s inner component**, which sits inside `SidebarProvider`, advertises `⌘B`. The listener belongs to the primitive. `packages/components/src/ui/sidebar.tsx` was read and not edited. - **`AiChatPage`** advertises `⌘⇧O` and `⌘⇧S` beside the listener that calls `matchAiChatShortcut`. So they are listed only where that page is mounted, and today the dialog is not mounted there. - **`@object-ui/i18n`.** Seven keys were read only by the dead rows, and they are removed from all ten packs: `console.shortcuts.focusSearch`, `createRecord`, `refreshData`, `editRecord`, `toggleDarkMode`, `groups.dataViews`, `groups.preferences`. Two i18n test comments cited `toggleDarkMode` as an example of ё, and they are reworded. One of them also carried a stale ё count, which is dropped, not re-counted. - The app-shell README gains a "Keyboard-shortcuts dialog" section. The changeset is `.changeset/11674-shortcuts-dialog-wired-only.md`, `minor` for both packages, with the key removal spelled out as BREAKING at `minor` under the repo's version policy. Inside an app, the dialog now lists `⌘K`, `?`, `Esc` and `⌘B`. Landing point: the order expected the static list in `KeyboardShortcutsDialog.tsx`. That file is where the list was. The fix also touches three files that register handlers, and only to advertise them, as the claim's file surface allows. ## The pin `packages/app-shell/src/chrome/KeyboardShortcutsDialog.wiredOnly-11674.test.tsx`: - **Every listed row fires.** The test mounts the dialog where `AppContent` mounts it: inside the real `ConsoleLayout`, with the real `AppShell`, `SidebarProvider` and `CommandPaletteProvider`. It opens the dialog with `?shortcuts=1`. For each row it fires the chord that the row's own advertisement declares, once with ⌘ and once with Ctrl, each on a fresh mount, and checks the real handler's effect: the palette opens, the dialog opens, the dialog closes, the sidebar collapses. The rows must equal the advertisements, keycaps included, and must equal the probe table's keys. So a listed row that nothing can fire fails, and the loop cannot pass by running nothing. - **The list is derived.** A row is listed while its advertiser is mounted and leaves when the advertiser unmounts. - **The AI page's chords are the ones its real matcher answers.** `matchAiChatShortcut` returns the right action for both chords. - **Control.** A bare `b`, the old row, does not toggle the sidebar. ## Ablations (one-off, run from committed HEAD `05c1f48`, restore proved by blob hash) - **The defect's own shape.** The sidebar's advertisement was set back to a bare `b` (`chord: { key: 'b' }`) with `ablation-replace` (anchor 1 → 0, blob `de7e99c048be` → `93c37705f9e4`). The pin went red, `Tests 1 failed | 4 passed (5)`, at the sidebar probe: `data-sidebar-open` stayed `"true"`. Restored: blob == HEAD `de7e99c048be`, `git diff HEAD` empty. - **The old static dialog.** `KeyboardShortcutsDialog.tsx` was swapped for its `c3623eb` bytes (`createRecord` marker 1, `useAdvertiseShortcut` 0, blob `ad882b0dc950`). The test went red, `Tests 2 failed | 3 passed (5)`: the pin and the derivation case. Restored: blob == HEAD `f766b2845931`. - **The i18n gate reads the descriptor labels.** `t('console.shortcuts.toggleSidebar')` in the sidebar descriptor was renamed to a key no pack has. `check:i18n-keys` exited 1 with `[missing-key] console.shortcuts.toggleSidebarProbe` at `ConsoleLayout.tsx`. Restored: blob == HEAD. ## Gates (objectui's own; derived by hand from `package.json` and `.github/workflows/`) All at HEAD `05c1f48`. Each exit code was captured before any pipe. | Gate | Exit | Reading | |---|---|---| | `pnpm exec vitest run --maxWorkers=2 packages/i18n/ packages/app-shell/src/chrome/ packages/app-shell/src/layout/ packages/app-shell/src/context/ packages/app-shell/src/console/__tests__/ packages/app-shell/src/console/ai/__tests__/` (under the verify lock) | 0 | `Test Files 193 passed (193)`, `Tests 2096 passed, 13 skipped` | | `pnpm --workspace-concurrency=2 --filter "@object-ui/app-shell^..." build` | 0 | dependency closure built | | `pnpm --filter @object-ui/app-shell type-check` (both `tsc` legs) | 0 | `tsconfig.test.json --listFilesOnly` lists the new pin | | `pnpm --filter @object-ui/i18n type-check` | 0 | | | `eslint` over the 6 changed app-shell files, `--format json` | 0 | 0 errors. 38 warnings, against 36 on the 4 modified files at `c3623eb` (read through `--stdin`). The +2 are `react-refresh/only-export-components` on the two exported AI-page descriptors | | `eslint` over the 12 changed i18n files, `--format json` | 0 | 0 errors, 0 warnings | | `check:i18n-keys` | 0 | every call-site key resolves (3273 keys) | | `check:i18n-drift` | 0 | 0 `en` values changed; 7 keys removed | | `check:i18n-dead-keys` (report-only) | 0 | 0 `console.shortcuts.*` candidates; pack keys 3273 | | `check:i18n-designer-parity`, `check:control-bytes`, `check:new-line-citations` (0 new), `check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:unreferenced-sources`, `check:test-path-roots`, `check:changeset-claims`, `check:pending-changeset-literals` | 0 each | | | `node scripts/check-changeset-no-major.mjs`, `check-changeset-presence.mjs` (18 source files, 2 packages, 1 changeset), `check-changeset-fixed.mjs` | 0 each | | | `check:readme-exports` | NOT MEASURED | PREREQUISITE: it needs every package's `dist`. No export moved, and the new README section has no fenced import | | `check:eager-locale-catalogues` | NOT MEASURED | PREREQUISITE: it needs `apps/console/dist/eager-closure.json` | Why the lint narrowing holds: - The population is each package's `lint` script, `eslint .`. - `eslint.config.js` sets no `parserOptions.project` or `projectService`, so no lint is type-aware. - No rule under `eslint-rules/` reads another file. So this diff cannot change the verdict on a file it did not touch. The full `pnpm test`, repo-wide `pnpm lint` and the `apps/console` suites are left to CI. Declared narrowing: the verify lock was held for about 26 minutes by a sibling's full-suite run. During that time the single pin file, and the ablations, ran without the lock (`--maxWorkers=1`). The final runs in the table above went through the lock. ## Acceptance notes (observations, not filed) - **`⌘⇧I`, the chat-dock toggle in `ConsoleLayout`, is wired and not advertised.** A row needs a new `console.shortcuts.*` key in ten packs. That is a widening, outside this claim's `Clause-②: no`. With this mechanism it costs one `useAdvertiseShortcut` call and the key. Carrier: none. - **Page-scoped shortcuts are wired and not advertised:** QuickFind `⌘⇧M`, the resource editor's `⌘S` / `⌘\` / `Esc`, the approvals inbox's list keys, the performance panel's `Ctrl+Shift+P`, and `useDebugMode`'s `Ctrl+Shift+D`. Once advertised, each would be listed only on its own page. This PR reads "lists exactly the shortcuts that are wired" as "lists no shortcut that is not wired", plus the chrome layer that owned the old list. - **`useKeyboardShortcuts`, `useCrudShortcuts` and `getShortcutDescriptions` in `@object-ui/react` are published and have zero callers.** This is dead code, and wiring it is not this card's question. - **The keycaps draw `⌘` on every platform**, as before. Every advertised handler also accepts Ctrl. - **The dialog is not mounted on `/ai`**, and the `/ai` header hides the Help-menu entry. So the AI pair is listed nowhere today. That is correct under the rule, and it is unchanged. - **Not measured, inferred from source:** the `SidebarProvider` listener does not skip editable targets, so `⌘B` inside a rich-text field may toggle the sidebar. --- _Generated by [Claude Code](https://claude.ai/code/session_01FngvPpdrnhHMdHHq6vwwju)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent eb4552e commit de96f3d

20 files changed

Lines changed: 600 additions & 128 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/app-shell': minor
3+
'@object-ui/i18n': minor
4+
---
5+
6+
The keyboard-shortcuts dialog (`?`) lists only shortcuts that do something (objectui#11674).
7+
8+
**Clause-②: no (narrowing)**: no accept set changes; the dialog shows fewer rows, and seven unused `console.shortcuts.*` keys leave all ten locale packs.
9+
10+
The dialog was a static list, separate from every key handler, and six of its rows did nothing. `N` (create record), `R` (refresh data), `⌘/Ctrl+E` (edit record), `⌘/Ctrl+/` (focus search) and `⌘/Ctrl+D` (dark mode) had no handler anywhere, and the sidebar row said `B` while the sidebar toggles on `⌘/Ctrl+B`. The AI-assistant rows (`⌘/Ctrl+Shift+O`, `⌘/Ctrl+Shift+S`) were listed inside apps, where their only handler, on the AI chat page, is not mounted.
11+
12+
`@object-ui/app-shell`: each shortcut is now advertised beside its handler, for as long as that handler is mounted, and `KeyboardShortcutsDialog` lists what is advertised. Inside an app it lists `⌘K` (command palette), `?` (this dialog), `Esc` (close a dialog or panel) and `⌘B` (toggle the sidebar). The AI chat page advertises its own two shortcuts, so they are listed only where that page is mounted. Rows are grouped as before and sorted by their text. A host that mounts `KeyboardShortcutsDialog` outside the console layout now sees the shortcuts whose handlers it mounts, not a fixed list.
13+
14+
`@object-ui/i18n` (BREAKING; `minor` under this repository's release model, where objectui's major follows the `@objectstack` major): `console.shortcuts.focusSearch`, `createRecord`, `refreshData`, `editRecord`, `toggleDarkMode`, `groups.dataViews` and `groups.preferences` are removed from all ten packs. Nothing reads them now. A host that calls `t()` with one of them gets the key back; supply the string from your own resources if you still need it.

‎packages/app-shell/README.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,19 @@ a `data-testid` onto their content element and emit Radix `data-state="open|clos
974974
so overlays are locatable and their open/closed state is machine-readable by
975975
construction (C4).
976976

977+
## Keyboard-shortcuts dialog
978+
979+
`KeyboardShortcutsDialog` (`?`, or `?shortcuts=1`) has no list of its own. Each
980+
shortcut is advertised by the code that handles it, beside the handler and for as
981+
long as the handler is mounted, and the dialog lists what is advertised at that
982+
moment (objectui#11674). Inside an app that is the command palette (`⌘K`), the
983+
dialog itself (`?`), closing a dialog or panel (`Esc`) and the sidebar (`⌘B`, the
984+
`SidebarProvider` listener). The AI chat page advertises `⌘⇧O` and `⌘⇧S` itself,
985+
so they are listed only where that page is mounted. A shortcut without a mounted
986+
handler is never listed. Each listed row carries `data-shortcut-id`, and
987+
`KeyboardShortcutsDialog.wiredOnly-11674.test.tsx` fires every row the console
988+
lists against its real handler.
989+
977990
## Settle signal (is the app idle?)
978991

979992
`<ConsoleShell>` exposes one global "no requests in flight" predicate so an

‎packages/app-shell/src/chrome/KeyboardShortcutsDialog.tsx‎

Lines changed: 80 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
/**
22
* KeyboardShortcutsDialog
33
*
4-
* A dialog listing all available keyboard shortcuts, triggered by pressing "?".
4+
* A dialog listing the keyboard shortcuts that are wired right now, triggered
5+
* by pressing "?".
6+
*
7+
* Its rows are not a list of its own: each one is a shortcut some mounted
8+
* handler advertises beside itself (`advertisedShortcuts.ts`, objectui#11674).
9+
* A shortcut whose handler is not mounted is not listed.
510
* @module
611
*/
712

8-
import { useEffect, useMemo } from 'react';
13+
import { useEffect } from 'react';
914
import {
1015
Dialog,
1116
DialogContent,
@@ -16,62 +21,85 @@ import {
1621
import { useObjectTranslation } from '@object-ui/i18n';
1722
import { useUrlOverlay } from '../hooks/useUrlOverlay.js';
1823
import { KEYBOARD_SHORTCUTS_PARAM } from '../urlParams.js';
24+
import {
25+
SHORTCUT_GROUPS,
26+
shortcutKeycaps,
27+
useAdvertiseShortcut,
28+
useAdvertisedShortcuts,
29+
type AdvertisedShortcut,
30+
type ShortcutGroup,
31+
type ShortcutLabelTranslate,
32+
} from './advertisedShortcuts.js';
33+
34+
/** `?` — the handler is this component's own keydown listener below. */
35+
const SHOW_SHORTCUTS_SHORTCUT: AdvertisedShortcut = {
36+
id: 'shortcuts-help',
37+
group: 'general',
38+
chord: { key: '?' },
39+
label: (t: ShortcutLabelTranslate) => t('console.shortcuts.showShortcuts'),
40+
};
1941

20-
interface ShortcutEntry {
42+
/**
43+
* `Esc` — the handler is Radix's dismiss listener, which this dialog and every
44+
* other dialog and panel in the shell carry. Advertised here, beside the
45+
* `Dialog` it closes.
46+
*/
47+
const CLOSE_OVERLAY_SHORTCUT: AdvertisedShortcut = {
48+
id: 'close-overlay',
49+
group: 'general',
50+
chord: { key: 'Escape' },
51+
label: (t: ShortcutLabelTranslate) => t('console.shortcuts.closeDialog'),
52+
};
53+
54+
function groupTitle(group: ShortcutGroup, t: ShortcutLabelTranslate): string {
55+
switch (group) {
56+
case 'general':
57+
return t('console.shortcuts.groups.general');
58+
case 'navigation':
59+
return t('console.shortcuts.groups.navigation');
60+
case 'aiChat':
61+
return t('console.shortcuts.groups.aiChat');
62+
}
63+
}
64+
65+
interface ShortcutRow {
66+
id: string;
2167
keys: string[];
2268
description: string;
2369
}
2470

25-
interface ShortcutGroup {
71+
interface ShortcutSection {
72+
group: ShortcutGroup;
2673
title: string;
27-
shortcuts: ShortcutEntry[];
74+
rows: ShortcutRow[];
2875
}
2976

3077
export function KeyboardShortcutsDialog() {
31-
const { t } = useObjectTranslation();
78+
const { t, language } = useObjectTranslation();
3279
// URL-addressable (?shortcuts=1) so the dialog is deep-linkable and openable
3380
// from the header Help menu, not only via the `?` keyboard accelerator (ADR-0054
3481
// C1/C2/C3).
3582
const { open, setOpen, toggleOverlay } = useUrlOverlay(KEYBOARD_SHORTCUTS_PARAM);
3683

37-
const shortcutGroups: ShortcutGroup[] = useMemo(() => [
38-
{
39-
title: t('console.shortcuts.groups.general'),
40-
shortcuts: [
41-
{ keys: ['⌘', 'K'], description: t('console.shortcuts.openCommandPalette') },
42-
{ keys: ['?'], description: t('console.shortcuts.showShortcuts') },
43-
{ keys: ['Esc'], description: t('console.shortcuts.closeDialog') },
44-
],
45-
},
46-
{
47-
title: t('console.shortcuts.groups.navigation'),
48-
shortcuts: [
49-
{ keys: ['B'], description: t('console.shortcuts.toggleSidebar') },
50-
{ keys: ['⌘', '/'], description: t('console.shortcuts.focusSearch') },
51-
],
52-
},
53-
{
54-
title: t('console.shortcuts.groups.dataViews'),
55-
shortcuts: [
56-
{ keys: ['N'], description: t('console.shortcuts.createRecord') },
57-
{ keys: ['R'], description: t('console.shortcuts.refreshData') },
58-
{ keys: ['⌘', 'E'], description: t('console.shortcuts.editRecord') },
59-
],
60-
},
61-
{
62-
title: t('console.shortcuts.groups.aiChat', { defaultValue: 'AI assistant' }),
63-
shortcuts: [
64-
{ keys: ['⌘', '⇧', 'O'], description: t('console.shortcuts.newChat', { defaultValue: 'New chat' }) },
65-
{ keys: ['⌘', '⇧', 'S'], description: t('console.shortcuts.toggleChatsList', { defaultValue: 'Toggle conversations list' }) },
66-
],
67-
},
68-
{
69-
title: t('console.shortcuts.groups.preferences'),
70-
shortcuts: [
71-
{ keys: ['⌘', 'D'], description: t('console.shortcuts.toggleDarkMode') },
72-
],
73-
},
74-
], [t]);
84+
useAdvertiseShortcut(SHOW_SHORTCUTS_SHORTCUT);
85+
useAdvertiseShortcut(CLOSE_OVERLAY_SHORTCUT);
86+
const advertised = useAdvertisedShortcuts();
87+
88+
// Sections in `SHORTCUT_GROUPS` order, rows alphabetical by their text: the
89+
// order handlers mount in is not an order a reader should see.
90+
const translate: ShortcutLabelTranslate = (key) => String(t(key));
91+
const sections: ShortcutSection[] = SHORTCUT_GROUPS.map((group) => ({
92+
group,
93+
title: groupTitle(group, translate),
94+
rows: advertised
95+
.filter((shortcut) => shortcut.group === group)
96+
.map((shortcut) => ({
97+
id: shortcut.id,
98+
keys: shortcutKeycaps(shortcut.chord),
99+
description: shortcut.label(translate),
100+
}))
101+
.sort((a, b) => a.description.localeCompare(b.description, language)),
102+
})).filter((section) => section.rows.length > 0);
75103

76104
useEffect(() => {
77105
function handleKeyDown(e: KeyboardEvent) {
@@ -108,20 +136,21 @@ export function KeyboardShortcutsDialog() {
108136
</DialogDescription>
109137
</DialogHeader>
110138
<div className="space-y-6 pt-2">
111-
{shortcutGroups.map(group => (
112-
<div key={group.title}>
139+
{sections.map(section => (
140+
<div key={section.group}>
113141
<h3 className="text-sm font-medium text-muted-foreground mb-3">
114-
{group.title}
142+
{section.title}
115143
</h3>
116144
<div className="space-y-2">
117-
{group.shortcuts.map((shortcut, idx) => (
145+
{section.rows.map(row => (
118146
<div
119-
key={idx}
147+
key={row.id}
148+
data-shortcut-id={row.id}
120149
className="flex items-center justify-between py-1.5"
121150
>
122-
<span className="text-sm">{shortcut.description}</span>
151+
<span className="text-sm">{row.description}</span>
123152
<div className="flex items-center gap-1">
124-
{shortcut.keys.map((key, kidx) => (
153+
{row.keys.map((key, kidx) => (
125154
<kbd
126155
key={kidx}
127156
className="inline-flex h-6 min-w-[24px] items-center justify-center rounded border bg-muted px-1.5 text-xs font-medium text-muted-foreground"

0 commit comments

Comments
 (0)