Skip to content

Commit 9a528f5

Browse files
os-helpclaude
andauthored
fix(showcase): a type: 'modal' target names a page — showcase_new_task becomes a form action (#6739) (#7237)
`showcase_new_task` declared `type: 'modal'` + `target: 'showcase_component_gallery'` — the showcase HOME PAGE. The dispatch opens the welcome page inside a dialog with zero form controls, so a command labelled "New Task" creates nothing. The issue's suggested one-key fix (point the modal at the `showcase_task` OBJECT) does not build: `defineStack`'s cross-reference walk accepts only declared PAGE names for a modal target. Per the maintainer ruling on #6739, that walk — together with the spec TSDoc and the published docs — IS the contract; objectui's page-then-object resolution is consumer leniency the renderer itself labels "Back-compat" and is being retired on its own sequenced card. So the fix is the TYPE, not the target: `type: 'form'` + `target: 'showcase_task.edit'`, structurally identical to `LogTimeAction`. PR #6737's inline `element:button` action of the same name is re-worked to the same shape in this commit (ruling item 1) — it depended on the object branch and only built because the cross-reference walk never visits an inline action (#6889). Pin tests assert the ruled shape on both sites plus the corpus-wide rule, so the reference corpus cannot drift back. Claude-Session: https://claude.ai/code/session_016R9de1FqP7NvwKvqXi92Gh Co-authored-by: Claude <noreply@anthropic.com>
1 parent bf05fa7 commit 9a528f5

3 files changed

Lines changed: 179 additions & 9 deletions

File tree

examples/app-showcase/src/ui/actions/index.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,37 @@ export const LogTimeAction = defineAction({
173173
refreshAfter: true,
174174
});
175175

176-
/** global nav command-palette action. */
176+
/**
177+
* global nav command action — "New Task".
178+
*
179+
* `type: 'form'` + an `<object>.<view>` FORM-view target, structurally
180+
* identical to `LogTimeAction` above. It used to be `type: 'modal'` +
181+
* `target: 'showcase_component_gallery'` — a command labelled "New Task" whose
182+
* target named the showcase HOME PAGE, so the dispatch opened the welcome page
183+
* inside a dialog with zero form controls and nothing to create a task with
184+
* (#6739).
185+
*
186+
* The fix is the TYPE, not the target. A `type: 'modal'` target names a PAGE
187+
* and only a page: the spec TSDoc (`packages/spec/src/ui/action.zod.ts`), the
188+
* published docs (`content/docs/ui/actions.mdx`) and `defineStack`'s
189+
* cross-reference walk (`packages/spec/src/stack.zod.ts`) all say so, and the
190+
* walk REJECTS a registered modal action whose target is not a declared page —
191+
* so "just point the modal at `showcase_task`" is a build error, not a fix
192+
* (maintainer ruling, #6739). Opening an object's form is what `type: 'form'`
193+
* is for, and it is validated: a form target pointing at a LIST view is itself
194+
* a build error (#2554, see LogTimeAction).
195+
*
196+
* Coverage is not lost: `QuickViewAction` above is the corpus's
197+
* modal-targeting-a-page specimen, and there the "open a dialog/page" semantics
198+
* match its "Quick View" label.
199+
*/
177200
export const NewTaskAction = defineAction({
178201
name: 'showcase_new_task',
179202
label: 'New Task',
180203
icon: 'plus',
181204
objectName: task,
182-
type: 'modal',
183-
target: 'showcase_component_gallery',
205+
type: 'form',
206+
target: 'showcase_task.edit',
184207
locations: ['global_nav'],
185208
refreshAfter: true,
186209
});

examples/app-showcase/src/ui/pages/index.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,22 @@ export const ComponentGalleryPage = definePage({
8888
// so the button rendered, was clickable, and the click did nothing at all
8989
// (no request, no dialog, no navigation). #6597.
9090
//
91-
// `type: 'modal'` + a string `target` is resolved page-first, then object
92-
// (objectui `useActionModal.resolveModalTarget`); `showcase_task` names no
93-
// page, so it lands on the object and opens the Task create form — which is
94-
// what a button labelled "Create Task" should do.
95-
{ type: 'element:button', properties: { label: 'Create Task', icon: 'plus', action: { name: 'showcase_new_task', type: 'modal', target: 'showcase_task', refreshAfter: true } } },
91+
// #6597's first fix wrote this as `type: 'modal'` + `target: 'showcase_task'`,
92+
// relying on objectui `useActionModal.resolveModalTarget` resolving a string
93+
// target page-first and then falling back to an OBJECT. It is now
94+
// `type: 'form'` + the object's `edit` FORM view, matching the registered
95+
// `NewTaskAction` of the same name (src/ui/actions/index.ts) — one action
96+
// name, one shape, in one corpus.
97+
//
98+
// Why the change (maintainer ruling on #6739): a `type: 'modal'` target
99+
// names a PAGE, only — spec TSDoc, published docs and `defineStack`'s
100+
// cross-reference walk all agree, and the walk rejects a registered modal
101+
// action targeting a non-page. The object fallback is consumer leniency the
102+
// renderer itself labels "Back-compat" and is being retired. This line only
103+
// ever built because the cross-reference walk visits `config.actions` and
104+
// never an INLINE action (#6889) — so it depended on a branch under
105+
// retirement and on a validation hole, both at once.
106+
{ type: 'element:button', properties: { label: 'Create Task', icon: 'plus', action: { name: 'showcase_new_task', type: 'form', target: 'showcase_task.edit', refreshAfter: true } } },
96107
],
97108
},
98109
{

examples/app-showcase/test/actions.test.ts

Lines changed: 137 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
import { describe, it, expect } from 'vitest';
44
import { actionBodyRunnerFactory, QuickJSScriptRunner } from '@objectstack/runtime';
55

6-
import { allActions, MarkDoneAction, PortfolioSnapshotAction } from '../src/ui/actions/index.js';
6+
import * as pages from '../src/ui/pages/index.js';
7+
import { allActions, MarkDoneAction, NewTaskAction, PortfolioSnapshotAction } from '../src/ui/actions/index.js';
78

89
/**
910
* Execution-path coverage for declared actions.
@@ -154,3 +155,138 @@ describe('showcase actions — the object-less (`global`) specimen', () => {
154155
});
155156
});
156157
});
158+
159+
/**
160+
* `type: 'modal'` targets a PAGE, only — the corpus pin (#6739).
161+
*
162+
* `showcase_new_task` used to declare `type: 'modal'` +
163+
* `target: 'showcase_component_gallery'`: a command labelled "New Task" whose
164+
* target named the showcase HOME PAGE. The dispatch was measured in a browser
165+
* while #6597 was being fixed — the dialog rendered the welcome page inside
166+
* itself, with **zero** form controls.
167+
*
168+
* The tempting one-key fix (point the modal at the `showcase_task` OBJECT) is
169+
* a BUILD ERROR, not a fix: `defineStack`'s cross-reference walk
170+
* (`packages/spec/src/stack.zod.ts`) accepts only declared PAGE names for a
171+
* modal target, which is also what the spec TSDoc and the published docs say.
172+
* objectui's page-then-object resolution is consumer leniency the renderer
173+
* itself labels "Back-compat" and is being retired (maintainer ruling on
174+
* #6739). Opening an object's form is `type: 'form'`'s job.
175+
*
176+
* These assertions pin the ruled shape on BOTH sites the name appears at, so
177+
* the corpus — which is reference material humans and AI authors copy — cannot
178+
* drift back:
179+
* - registered actions: no `type: 'modal'` target that is not a declared page.
180+
* That mirrors the build gate, so a regression fails here first with a
181+
* readable message instead of as an import-time crash in nine other files;
182+
* - INLINE page-element actions: the same rule, which the build gate does NOT
183+
* enforce — the cross-reference walk visits `config.actions` only and never
184+
* an inline action (#6889). This is the corpus's own guard over that hole,
185+
* and it is exactly how the old `element:button` line built while depending
186+
* on the object branch;
187+
* - `showcase_new_task` itself is `type: 'form'` at a FORM view, structurally
188+
* identical to `showcase_log_time`.
189+
*/
190+
describe("showcase actions — a `type: 'modal'` target names a page (#6739)", () => {
191+
type AnyAction = { name?: unknown; type?: unknown; target?: unknown };
192+
type AnyComponent = { type?: unknown; properties?: Record<string, unknown> };
193+
194+
/** Every page name the showcase declares — the set a modal target may name. */
195+
const pageNames = new Set(
196+
(Object.values(pages) as unknown[])
197+
.filter(
198+
(p): p is { name: string } =>
199+
!!p && typeof p === 'object' && !Array.isArray(p) && typeof (p as { name?: unknown }).name === 'string',
200+
)
201+
.map((p) => p.name),
202+
);
203+
204+
/** Every component on every page — regions and slots alike, nesting included. */
205+
function allComponents(page: Record<string, unknown>): AnyComponent[] {
206+
const out: AnyComponent[] = [];
207+
const visit = (node: unknown): void => {
208+
if (!node || typeof node !== 'object' || Array.isArray(node)) return;
209+
const component = node as AnyComponent;
210+
out.push(component);
211+
const props = component.properties;
212+
if (!props) return;
213+
for (const item of Array.isArray(props.items) ? props.items : []) {
214+
const children = (item as { children?: unknown })?.children;
215+
for (const child of Array.isArray(children) ? children : []) visit(child);
216+
}
217+
for (const child of Array.isArray(props.children) ? props.children : []) visit(child);
218+
};
219+
for (const region of (page.regions as { components?: unknown[] }[] | undefined) ?? []) {
220+
for (const c of region.components ?? []) visit(c);
221+
}
222+
for (const slot of Object.values((page.slots as Record<string, unknown>) ?? {})) {
223+
for (const c of Array.isArray(slot) ? slot : [slot]) visit(c);
224+
}
225+
return out;
226+
}
227+
228+
/** Inline actions authored on a page element (`element:button`'s `action`). */
229+
const inlineActions = (): { page: string; action: AnyAction }[] => {
230+
const out: { page: string; action: AnyAction }[] = [];
231+
for (const page of Object.values(pages) as unknown[]) {
232+
if (!page || typeof page !== 'object' || Array.isArray(page)) continue;
233+
const p = page as Record<string, unknown>;
234+
if (typeof p.name !== 'string') continue;
235+
for (const component of allComponents(p)) {
236+
const action = component.properties?.action;
237+
if (action && typeof action === 'object' && !Array.isArray(action)) {
238+
out.push({ page: p.name, action: action as AnyAction });
239+
}
240+
}
241+
}
242+
return out;
243+
};
244+
245+
it('the page set the corpus can target is non-empty', () => {
246+
// Guards the assertions below against passing vacuously: an empty page set
247+
// would make "targets a declared page" trivially unfalsifiable.
248+
expect(pageNames.size).toBeGreaterThan(10);
249+
expect(pageNames.has('showcase_component_gallery')).toBe(true);
250+
});
251+
252+
it('every REGISTERED modal action targets a declared page', () => {
253+
const modals = (allActions as AnyAction[]).filter((a) => a.type === 'modal');
254+
// Keep the modal-targeting-a-page specimen alive: if this ever hits zero
255+
// the rule below stops being exercised by anything.
256+
expect(modals.length).toBeGreaterThan(0);
257+
for (const a of modals) {
258+
expect(
259+
pageNames.has(String(a.target)),
260+
`action '${String(a.name)}': a modal target names a PAGE, but '${String(a.target)}' is not a declared page — ` +
261+
`use type: 'form' with an <object>.<view> target to open a form (#6739)`,
262+
).toBe(true);
263+
}
264+
});
265+
266+
it('every INLINE page-element modal action targets a declared page too', () => {
267+
// The build gate cannot see these (#6889): `defineStack`'s cross-reference
268+
// walk visits `config.actions` only. This is the corpus's own guard.
269+
const inline = inlineActions();
270+
expect(inline.length).toBeGreaterThan(0);
271+
for (const { page, action } of inline) {
272+
if (action.type !== 'modal') continue;
273+
expect(
274+
pageNames.has(String(action.target)),
275+
`page '${page}': inline action '${String(action.name)}' is type:'modal' targeting ` +
276+
`'${String(action.target)}', which is not a declared page (#6739)`,
277+
).toBe(true);
278+
}
279+
});
280+
281+
it("`showcase_new_task` opens the Task form — type: 'form' at a FORM view", () => {
282+
expect(NewTaskAction.type).toBe('form');
283+
expect(NewTaskAction.target).toBe('showcase_task.edit');
284+
285+
// Both sites carrying the name agree, so the corpus teaches one shape.
286+
const inline = inlineActions()
287+
.map(({ action }) => action)
288+
.filter((a) => a.name === 'showcase_new_task');
289+
expect(inline).toHaveLength(1);
290+
expect(inline[0]).toMatchObject({ type: 'form', target: 'showcase_task.edit' });
291+
});
292+
});

0 commit comments

Comments
 (0)