diff --git a/.changeset/retired-action-keys-unauthorable.md b/.changeset/retired-action-keys-unauthorable.md new file mode 100644 index 000000000..fe80f6376 --- /dev/null +++ b/.changeset/retired-action-keys-unauthorable.md @@ -0,0 +1,27 @@ +--- +"@object-ui/app-shell": minor +"@object-ui/core": minor +--- + +Stop offering the retired `action.shortcut` / `action.bulkEnabled` keys. + +`@objectstack/spec` 17 retired both as `retiredKey()` tombstones: authoring +either one is a hard PARSE REJECTION, so a draft carrying it cannot be saved +at all. The designer still offered controls for both — a "Bulk — apply to +multiple selected rows" checkbox and a "Shortcut" text field — which meant the +Studio action inspector let an author build a draft the platform would then +refuse, with the rejection arriving later and nowhere near the checkbox. + +- **Action inspector**: both controls removed. The keys stay hidden from the + fallback form (the server's live schema still advertises them, so dropping + them from the hidden list would put the inputs straight back) — now under a + `RETIRED_FIELDS` list that says why, so nobody "restores the missing + control". `bulkEnabled`'s replacement is the list view's `bulkActions` / + `bulkActionDefs`; `shortcut` has none. +- **Action preview**: the `shortcut` and `bulk` pills are gone — they could + only ever render for metadata the platform now refuses. +- **`ActionEngine.registerActions`**: no longer harvests the two retired keys + from authored metadata, which made two dead registration options look + load-bearing. Both are still accepted on the single-action + `registerAction(action, options)` overload, where a HOST passes them + explicitly. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx index 3db0d4c09..174283242 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx @@ -180,11 +180,29 @@ function ActionTargetField({ type, value, onCommit, cfg, readOnly }: { const CURATED_FIELDS = [ 'name', 'label', 'objectName', 'icon', 'variant', 'component', 'type', 'target', 'execute', 'body', 'method', - 'params', 'locations', 'bulkEnabled', - 'confirmText', 'successMessage', 'errorMessage', 'refreshAfter', 'undoable', 'mode', 'shortcut', + 'params', 'locations', + 'confirmText', 'successMessage', 'errorMessage', 'refreshAfter', 'undoable', 'mode', 'visible', 'disabled', 'aiExposed', 'aiDescription', ]; +/** + * Keys hidden from the fallback form for the OPPOSITE reason: not because this + * inspector edits them, but because `@objectstack/spec` 17 retired them as + * `retiredKey()` tombstones — authoring either one is a hard PARSE REJECTION, + * so a draft carrying it cannot be saved at all. + * + * They stay listed because the fallback renders from the server's live schema, + * which still advertises both; dropping them here would put the inputs back. + * This inspector used to offer its own controls for them (a "Bulk — apply to + * multiple selected rows" checkbox and a "Shortcut" text field), which is how + * the designer let an author build a draft the platform would then refuse. + * + * Do not add controls back. `bulkEnabled`'s replacement is the LIST VIEW's + * `bulkActions` / `bulkActionDefs`; `shortcut` has none — register the key in + * the Console keyboard stack and have its handler invoke the action by name. + */ +const RETIRED_FIELDS = ['bulkEnabled', 'shortcut']; + /* ─────────────── small helpers ─────────────── */ function SectionHeader({ title, hint }: { title: string; hint?: string }) { @@ -420,7 +438,9 @@ export function ActionDefaultInspector({
onPatch({ component: v })} disabled={readOnly} />
- onPatch({ bulkEnabled: v })} disabled={readOnly} /> + {/* No "Bulk" checkbox here: `action.bulkEnabled` is a spec-17 + tombstone (see RETIRED_FIELDS). Selection placement is declared on + the LIST VIEW, in `bulkActions` / `bulkActionDefs`. */} {/* 5 ─ Feedback */} @@ -429,9 +449,11 @@ export function ActionDefaultInspector({ onPatch({ confirmText: v })} placeholder="Ask before running (leave blank to skip)" disabled={readOnly} /> onPatch({ successMessage: v })} disabled={readOnly} /> onPatch({ errorMessage: v })} disabled={readOnly} /> + {/* No "Shortcut" field beside Mode: `action.shortcut` is a spec-17 + tombstone (see RETIRED_FIELDS) — nothing ever read it, and + authoring it now fails the parse. */}
onPatch({ mode: v })} disabled={readOnly} /> - onPatch({ shortcut: v })} placeholder="e.g. Ctrl+S" disabled={readOnly} mono />
onPatch({ refreshAfter: v })} disabled={readOnly} /> @@ -475,7 +497,7 @@ export function ActionDefaultInspector({ onPatch(next)} /> diff --git a/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx b/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx index 45c4fdf4a..c85fb82dc 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx @@ -10,7 +10,7 @@ * and `label` so authors can see the visual weight before they * ship it (primary buttons are highlighted, danger turns red, * icon-only actions render a compact icon button). - * 2. A metadata strip: type, target, locations, shortcut, bulk + * 2. A metadata strip: type, target, locations * flag, AI exposure, refreshAfter, confirmText. * 3. A params table when the action prompts the user — this is the * modal/drawer it would open on click. We render it as a static @@ -30,14 +30,12 @@ import { Code2, Eye, Globe, - Keyboard, LayoutGrid, Link2, Lock, MoreHorizontal, Pencil, RefreshCw, - ScanLine, Sparkles, Square, Workflow, @@ -158,8 +156,9 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) { const variant = (d.variant as string | undefined) || undefined; const component = String(d.component ?? ''); const locations = Array.isArray(d.locations) ? (d.locations as string[]) : []; - const shortcut = (d.shortcut as string | undefined) || undefined; - const bulkEnabled = !!d.bulkEnabled; + // No `shortcut` / `bulkEnabled` here: both are spec-17 `retiredKey()` + // tombstones, so a preview of them could only ever render for metadata the + // platform now refuses to parse. See ActionDefaultInspector's RETIRED_FIELDS. const refreshAfter = !!d.refreshAfter; const aiExposed = d.aiExposed; const confirmText = localize(d.confirmText); @@ -204,8 +203,6 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) { {objectName && } {variant && } {component && } - {shortcut && } - {bulkEnabled && } {refreshAfter && } {aiExposed === false && } {aiExposed === true && } diff --git a/packages/core/src/actions/ActionEngine.ts b/packages/core/src/actions/ActionEngine.ts index ea33146ce..75697f38d 100644 --- a/packages/core/src/actions/ActionEngine.ts +++ b/packages/core/src/actions/ActionEngine.ts @@ -142,13 +142,21 @@ export class ActionEngine { } } - /** Register multiple actions from an ActionSchema array */ + /** + * Register multiple actions from an ActionSchema array. + * + * Only `locations` is harvested from the metadata. `shortcut` and + * `bulkEnabled` used to be read here too, but spec 17 retired both as + * `retiredKey()` tombstones — a declaration carrying either one no longer + * parses, so harvesting them read a key that cannot exist and made two dead + * options look load-bearing. Both remain accepted on the single-action + * `registerAction(action, options)` overload, where a HOST passes them + * explicitly; they are simply no longer sourced from authored metadata. + */ registerActions(actions: ActionDef[]): void { for (const action of actions) { this.registerAction(action, { locations: (action as any).locations, - shortcut: (action as any).shortcut, - bulkEnabled: (action as any).bulkEnabled, }); } } diff --git a/packages/core/src/actions/__tests__/ActionEngine.test.ts b/packages/core/src/actions/__tests__/ActionEngine.test.ts index b5bb548f4..18bf69951 100644 --- a/packages/core/src/actions/__tests__/ActionEngine.test.ts +++ b/packages/core/src/actions/__tests__/ActionEngine.test.ts @@ -56,6 +56,19 @@ describe('ActionEngine', () => { expect(engine.getAction('save')).toBeDefined(); expect(engine.getAction('delete')).toBeDefined(); }); + + it('does not harvest the retired `shortcut` / `bulkEnabled` keys from metadata', () => { + // spec 17 retired both as `retiredKey()` tombstones — a declaration + // carrying either no longer parses, so reading them here made two dead + // options look load-bearing. A host may still pass them explicitly to + // `registerAction`; they are just not sourced from authored metadata. + engine.registerActions([ + { name: 'stale', type: 'api', shortcut: 'ctrl+k', bulkEnabled: true } as never, + ]); + expect(engine.getAction('stale')).toBeDefined(); + expect(engine.getShortcuts()).toHaveLength(0); + expect(engine.getBulkActions().map((a) => a.name)).not.toContain('stale'); + }); }); describe('unregisterAction', () => { diff --git a/packages/react/src/hooks/__tests__/useActionEngine.test.ts b/packages/react/src/hooks/__tests__/useActionEngine.test.ts index 4ab1c15bd..4bfcd9ca2 100644 --- a/packages/react/src/hooks/__tests__/useActionEngine.test.ts +++ b/packages/react/src/hooks/__tests__/useActionEngine.test.ts @@ -78,14 +78,20 @@ describe('useActionEngine', () => { }); describe('getBulkActions', () => { - it('returns only bulk-enabled actions', () => { + // Deliberately INVERTED. `sampleActions` still carries the stale + // `bulkEnabled: true` — spec 17 retired the key as a `retiredKey()` + // tombstone, so metadata like this no longer parses at all, and harvesting + // it here made a dead registration option look load-bearing. Same posture + // as plugin-grid's "ignores a stale bulkEnabled flag on an object action". + // The engine's bulk mechanics keep their coverage in ActionEngine.test.ts, + // where a HOST passes `{ bulkEnabled: true }` to `registerAction` + // explicitly — which is still supported. + it('does not harvest the retired bulkEnabled key from metadata', () => { const { result } = renderHook(() => useActionEngine({ actions: sampleActions }), ); - const bulkActions = result.current.getBulkActions(); - expect(bulkActions.length).toBe(1); - expect(bulkActions[0].name).toBe('mark_complete'); + expect(result.current.getBulkActions()).toEqual([]); }); }); @@ -119,7 +125,14 @@ describe('useActionEngine', () => { }); describe('handleShortcut', () => { - it('handles registered keyboard shortcut', async () => { + // Deliberately INVERTED, for the same reason as getBulkActions above: + // `sampleActions` still declares the stale `shortcut: 'ctrl+k'`, which + // spec 17 retired. Its tombstone is explicit that nothing ever consumed it + // ("no keydown listener feeds ActionEngine.getShortcuts(), and objectui's + // keyboard stack is hand-registered and never consults action metadata"), + // so harvesting it only kept a dead path looking alive. A host that wants + // a shortcut still passes one to `registerAction` explicitly. + it('does not harvest the retired shortcut key from metadata', async () => { const { result } = renderHook(() => useActionEngine({ actions: sampleActions }), ); @@ -129,8 +142,7 @@ describe('useActionEngine', () => { shortcutResult = await result.current.handleShortcut('ctrl+k'); }); - expect(shortcutResult).not.toBeNull(); - expect(shortcutResult.success).toBe(true); + expect(shortcutResult).toBeNull(); }); it('returns null for unregistered shortcut', async () => {