Skip to content

Commit d8c11dc

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/youthful-pasteur-be09ad
# Conflicts: # packages/components/src/renderers/form/form.tsx
2 parents aacaa64 + f59f2c1 commit d8c11dc

32 files changed

Lines changed: 2115 additions & 662 deletions
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
'@object-ui/types': major
3+
'@object-ui/core': major
4+
'@object-ui/components': major
5+
'@object-ui/app-shell': major
6+
---
7+
8+
**`target` is the only action handler slot — the `execute` alias is gone from the renderer (framework#3856).**
9+
10+
`ActionRunner.executeScript` read `action.target || action.execute`. That fallback
11+
is unreachable against `@objectstack/spec` 17: `execute` is now a tombstoned key
12+
(framework#3855) that the parser **rejects** with the rename prescription, so no
13+
parsed action can carry it and the `||` could only ever yield `target`. Verified
14+
against 17.0.0-rc.0 — an action declaring `execute` fails `ActionSchema.safeParse`,
15+
and a `target` action's parsed output has no `execute` key at all.
16+
17+
Deleted rather than left as harmless residue: two handler slots is what let one
18+
action run one script server-side and a different one client-side (framework#3713,
19+
where this renderer preferred the alias while the spec transform preferred
20+
`target`). A dead slot still reads as a live contract to the next maintainer.
21+
22+
`execute` is also **removed from the types**, which is the part that had never
23+
landed. framework#3856 predicted a compile error here; there wasn't one, because
24+
neither reader was typed against the spec's `z.infer`:
25+
26+
- `@object-ui/types` `ActionSchema` hand-declared `execute?: string`. Removed, so
27+
`execute: '…'` now fails `tsc` at the authoring site (TS2353).
28+
- `@object-ui/core` `ActionDef` hand-declared it too. Removed — but `ActionDef`
29+
carries a `[key: string]: any` index signature, so stale hand-authored metadata
30+
that never passed through the parser still compiles. For that path
31+
`executeScript` now returns the rename prescription instead of a bare
32+
"No script provided", matching the spec tombstone's rule that removing an
33+
authorable key must be audible: silently binding no handler is the
34+
"Mark Done does nothing" shape (framework#2169).
35+
36+
The four action renderers (`action:button`, `action:icon`, `action:menu`,
37+
`action:group`) no longer forward `execute` into the runner, and Studio's
38+
`ActionPreview` no longer falls back to it — previewing an alias-only draft as
39+
"bound" contradicted the parse that rejects it on save.
40+
41+
Requires `@objectstack/spec` 17. Metadata still on the alias is rewritten by
42+
`os migrate meta --from 16`.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@object-ui/types": minor
3+
"@object-ui/core": minor
4+
---
5+
6+
refactor(actions): `navigation` becomes a named alias of the spec's `url`, sharing one navigator (#2944)
7+
8+
The last open item of #2944: `ActionRunner` dispatched a seventh action type,
9+
`navigation`, that `@objectstack/spec`'s `ActionType` does not contain. The issue
10+
asked for a decision — promote it upstream or delete the case. Neither, as stated.
11+
12+
- **Promoting it is wrong.** The spec already has `url` for "go to a location",
13+
with `openIn` for the new-tab/same-tab choice. A seventh type would put a
14+
second spec name on one operation, which is the exact failure the #2901 audit
15+
is named after: *a second definition of the vocabulary exists, and the renderer
16+
is faithful to the wrong one*.
17+
- **Deleting it is worse, because it is silent.** `{ type: 'navigation', to: … }`
18+
is authored today (`element:button` CTAs). Without the case the action falls
19+
through to `executeActionSchema`, which returns `{ success: true }` — a green
20+
toast that navigates nowhere. That is #2960's trap.
21+
22+
So it stays, but stops being dialect. `ObjectUiLocalActionType` /
23+
`OBJECTUI_LOCAL_ACTION_TYPES` in `@object-ui/types` declare it as objectui's own
24+
alias of `url` — the same treatment #2985 gave `PageVisualizationAlias` — and the
25+
runner routes both names through one navigator.
26+
27+
**The alias had already drifted, which is the point.** `executeNavigation` was
28+
quietly the weaker of the two implementations: no `${param.X}` / `${ctx.X}`
29+
interpolation, `openIn` ignored, and no `/api/…` full-page short-circuit (the
30+
redirect-dance case `url` exists to handle). An author who wrote
31+
`{ type: 'navigation', to: '/x?p=${param.p}' }` shipped the literal `${param.p}`,
32+
while the identical `url` action resolved it. Both names now behave identically;
33+
`url` in turn gains `replace` pass-through, the one modifier only the alias had.
34+
35+
Additive only. `replace` is omitted from the `NavigationHandler` options object
36+
when unset, so hosts see the option shape they already saw.
37+
38+
The new guard is structural rather than another assertion. The runner's built-in
39+
dispatch is a table typed `Record<RunnableActionType, …>` instead of a `switch`,
40+
so an `ActionType` the spec **adds** stops compiling until an executor exists for
41+
it — the Tier-2 "validates at save, renders nothing at run time" failure (#2942)
42+
becomes a build error for actions. `spec-derived-unions.test.ts` additionally
43+
asserts `navigation` is *absent* from the spec enum, so the day it is adopted
44+
upstream, the test fails and names the alias to retire.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@object-ui/types": patch
3+
"@object-ui/components": patch
4+
"@object-ui/plugin-form": patch
5+
---
6+
7+
fix(form): a tabbed/sectioned create-edit form no longer loses the tabs you are not looking at (#2959, #2153)
8+
9+
The explicit-`sections` path rendered one `SchemaRenderer` — one react-hook-form
10+
instance and one `<form>` element — **per section**, all sharing the same
11+
`formId`. Two failures compounded:
12+
13+
1. the footer submit button (`form={formId}`) can only be associated with the
14+
**first** of those forms, so section 2+ never reached the payload; and
15+
2. in the `tabbed` variant Radix unmounted the inactive panel, destroying that
16+
tab's form state outright.
17+
18+
Reported flow (HotCRM, 3 tabs, required `description` on tab 3): fill tab 1 →
19+
submit → server 400 `description is required` → switch to tab 3, fill it →
20+
submit → the server now reports `subject; description; status; priority` **all**
21+
missing, because the second submit's body had lost every earlier value.
22+
23+
`ModalForm` (stacked and `contentLayout: 'tabbed'`) and `TabbedForm` now render
24+
ONE form for all sections, matching `ObjectForm` / `DrawerForm`. Stacked sections
25+
use the existing inline `section-divider` header (which now also renders the
26+
section's `description`); tabbed sections go through a new
27+
`FormSchema.fieldTabs` (+ `defaultFieldTab`, `fieldTabsPosition`) that the form
28+
renderer distributes into **force-mounted** Radix panels — CSS-hidden rather
29+
than unmounted, since react-hook-form skips validation for unmounted fields,
30+
which is how a required field on a tab nobody opened used to sail past the
31+
client and come back as a server 400.
32+
33+
Validation feedback now points at the tab: a rejected field activates its tab and
34+
every tab holding one is marked on its trigger, for client-side rules and server
35+
`fields[]` rejections alike.
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"@object-ui/components": minor
3+
"@object-ui/plugin-view": patch
4+
---
5+
6+
fix(view,components): the spec→FilterBuilder operator table covers the whole view vocabulary, and the dead write direction is gone
7+
8+
`view-config-utils`' `SPEC_TO_BUILDER_OP` resolved **10 of the spec's 19
9+
canonical `VIEW_FILTER_OPERATORS`**. The nine it missed —
10+
`not_equals`, `starts_with`, `ends_with`, `greater_than`, `less_than`,
11+
`greater_than_or_equal`, `less_than_or_equal`, `is_null`, `is_not_null` — all
12+
appear in stored view metadata (they are canonical; `ViewFilterRuleSchema`
13+
validates against exactly this list), and each reached the FilterBuilder as a
14+
raw spelling its operator dropdown cannot select.
15+
16+
Same defect and same cause as #2974, one table over: spellings were enumerated
17+
by hand. That table is now derived from the spec's own canonical list and
18+
`VIEW_FILTER_OPERATOR_ALIASES`, matched case- and separator-insensitively, so
19+
`not_in` / `notIn` / `'not in'` / `NOT_IN` are one entry rather than four
20+
chances to miss one.
21+
22+
Four canonical operators have no FilterBuilder equivalent —
23+
`starts_with`/`ends_with` (absent from its vocabulary) and `is_null`/
24+
`is_not_null` (distinct from the `is_empty`/`is_not_empty` it does have). They
25+
are recorded as explicit `null`s and asserted, and deliberately left unmapped:
26+
folding them onto a near-equivalent would silently rewrite the author's
27+
operator on the next save, whereas an unmapped operator surfaces as a condition
28+
row the author must complete.
29+
30+
Also retired `BUILDER_TO_SPEC_OP` and `toSpecFilter` — the write direction,
31+
dead since the legacy `buildViewConfigSchema` engine was replaced by the
32+
studio's spec-driven inspector (no caller anywhere in the repo, and not part of
33+
`@object-ui/plugin-view`'s public exports). It was objectui's last emitter of
34+
`'not in'` with a space, plus `before`/`after`, as *filter-AST* operators —
35+
spellings that reached the server outside `VALID_AST_OPERATORS` and were dropped
36+
without an error (objectstack-ai/objectstack#3948).
37+
38+
`@object-ui/components` now exports `FILTER_BUILDER_OPERATORS` (and the
39+
`FilterBuilderOperator` type), derived from the operators the FilterBuilder
40+
actually renders, so tables mapping onto that vocabulary can assert against it
41+
instead of restating it.
42+
43+
Refs objectstack-ai/objectui#2945, #2901.

content/docs/plugins/plugin-form.mdx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,35 @@ interface FormField {
6262
}
6363
```
6464

65+
### Tabbed field layout (`fieldTabs`)
66+
67+
A sectioned form stays **one** form: declare the tabs on it and the renderer
68+
distributes the fields into panels, instead of rendering a form per section
69+
(which strands every section but the first outside the submit, and lets an
70+
inactive tab unmount along with its values).
71+
72+
```plaintext
73+
{
74+
type: 'form',
75+
fields: [/* every tab's fields, in one flat list */],
76+
fieldTabs: [
77+
{ key: 'basics', label: 'Basics', fields: ['subject', 'status'] },
78+
{ key: 'detail', label: 'Detail', description: 'Anything else', fields: ['description'] },
79+
],
80+
defaultFieldTab?: 'basics', // defaults to the first tab
81+
fieldTabsPosition?: 'top', // 'top' | 'bottom' | 'left' | 'right'
82+
}
83+
```
84+
85+
- Panels are force-mounted and only CSS-hidden, so a tab the user leaves keeps
86+
its values **and** its validation.
87+
- A failed submit activates the tab holding the first offending field and marks
88+
every tab with a rejected field — client rules and server `fields[]` alike.
89+
- Fields no tab claims render above the tab strip rather than disappearing.
90+
- Needs at least two tabs; ignored when the form uses `children`.
91+
92+
`ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` build on this.
93+
6594
## Usage
6695

6796
### Auto-registration (Side-effect Import)

docs/audits/2901-spec-enum-renderer-coverage.md

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Audited against `@objectstack/spec@16.0.0-rc.0` and `objectui@cd09a7b90`.
44
**Re-validated against `17.0.0-rc.0`** — see the addendum at the end, which also
5-
carries three corrections. Read it before acting on any row below.
5+
carries four corrections. Read it before acting on any row below.
66

77
Every row below was confirmed by reading the dispatch code. Nothing was exercised at
88
runtime, so symptoms are read off the code path, not observed. Rows the auditors could
@@ -165,7 +165,7 @@ value corruption rather than disclosure.
165165

166166
| Name | Location | Call |
167167
|---|---|---|
168-
| `navigation` action type | [ActionRunner.ts:539](../../packages/core/src/actions/ActionRunner.ts) | Fully implemented (`executeNavigation`). **Promote** — it is a real capability the spec doesn't name. |
168+
| `navigation` action type | [ActionRunner.ts](../../packages/core/src/actions/ActionRunner.ts) | ~~Promote~~**resolved as a declared alias instead.** See the correction below. |
169169
| `combo` chart type | [normalizeChartSchema.ts:63](../../packages/plugin-charts/src/normalizeChartSchema.ts) | Drawn at `AdvancedChartImpl.tsx:819`. Promote or delete. |
170170
| `system` theme mode | ThemeProvider ×2 | Delete — the spec name is `auto`, and `react/ThemeContext.tsx` already gets it right. |
171171
| `scale-fade` preset | [useAnimation.ts:71](../../packages/react/src/hooks/useAnimation.ts) | Delete with the hyphen/underscore fix. |
@@ -358,6 +358,33 @@ On a list whose only narrowing is that filter, the user sees rows they had asked
358358
to exclude. It is not a permission bypass — row-scoping `$and`-composes as a
359359
separate arm and survives — but it ranks with Tier 1, not Tier 2.
360360

361+
## Correction 4 — "promote `navigation` upstream" was the wrong call
362+
363+
Direction B told the reader to promote the `navigation` action type into the spec,
364+
on the grounds that it is "a real capability the spec doesn't name". The capability
365+
is real; the framing was not. The spec already names it — `type: 'url'`, with
366+
`openIn` for the new-tab choice — so promoting `navigation` would have put a second
367+
spec name on one operation. That is the failure this audit exists to describe,
368+
committed one level up.
369+
370+
The row *also* under-read the cost of the stated alternative. Deleting the case
371+
does not surface an error: the action falls through to `executeActionSchema`, which
372+
returns `{ success: true }`, so the user gets a success toast and no navigation —
373+
#2960's trap, reached by way of a cleanup.
374+
375+
What was missing from the row is that the alias had **already drifted**, which is
376+
the ordinary outcome of two implementations of one concept. `executeNavigation` did
377+
no `${param.X}` interpolation, ignored `openIn`, and skipped the `/api/…`
378+
full-page short-circuit — so `{ type: 'navigation', to: '/x?p=${param.p}' }`
379+
shipped the literal `${param.p}` while the identical `url` action resolved it.
380+
381+
Resolved by declaring it (`ObjectUiLocalActionType`) and routing both names
382+
through one navigator — the same treatment #2985 gave the page-visualization names.
383+
Generalizing: a "renderer-local dialect" row needs a third option next to promote
384+
and delete — **alias to the spec name it duplicates** — and picking between the
385+
three starts with asking whether the spec really lacks the concept or just spells
386+
it differently.
387+
361388
## Status of the items above
362389

363390
| Item | Status |
@@ -366,7 +393,9 @@ separate arm and survives — but it ranks with Tier 1, not Tier 2.
366393
| `before`/`after`, `'not in'`, + 8 legacy spellings | Fixed — #2974 |
367394
| `DensityModeSchema` inert | Resolved upstream (removed in spec 17) |
368395
| Packages lacking the `@objectstack/spec` devDep | **6**`plugin-list` and `data-objectstack` gained it in #2974; `plugin-charts`, `plugin-dashboard`, `plugin-report`, `components`, `mobile` and `fields` remain. (The body above says 6 because it counted a 13-package "needs a guard" list that omitted `fields`; adding `fields` and removing the two now fixed leaves 6 either way.) |
369-
| Everything else | Open — #2941 (Tier 1), #2942 (Tier 2), #2943 (Tier 3), #2944 (forks), #2945 (vocabularies) |
396+
| The five shadowing forks | Fixed — #2985 (closes #2944) |
397+
| `navigation` action type | Resolved as a declared alias, not promoted — see Correction 4 |
398+
| Everything else | Open — #2941 (Tier 1), #2942 (Tier 2), #2943 (Tier 3), #2945 (vocabularies) |
370399

371400
One method note for anyone repeating this: the original run read a checkout that
372401
was behind `origin/main`, which produced a spurious "objectui resolves two spec

packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) {
151151
const label = localize(d.label) || actionName;
152152
const icon = (d.icon as string | undefined) || undefined;
153153
const type = String(d.type ?? 'script');
154-
const target = (d.target as string | undefined) ?? (d.execute as string | undefined);
154+
// `target` only: the `execute` alias was removed in @objectstack/spec 17
155+
// (#3855, #3856). Reading it here would preview a draft as bound when the
156+
// spec rejects it at save, which is the opposite of what a preview is for.
157+
const target = d.target as string | undefined;
155158
const variant = (d.variant as string | undefined) || undefined;
156159
const component = String(d.component ?? '');
157160
const locations = Array.isArray(d.locations) ? (d.locations as string[]) : [];

packages/components/src/custom/filter-builder.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,23 @@ const defaultOperators = [
7171
{ value: "between", label: "Between" },
7272
{ value: "in", label: "In" },
7373
{ value: "notIn", label: "Not in" },
74-
]
74+
] as const
75+
76+
/**
77+
* The FilterBuilder's own operator vocabulary — every id its dropdown can
78+
* hold, derived from the operators it actually renders.
79+
*
80+
* Exported because several translation tables map an external vocabulary
81+
* (the spec's `VIEW_FILTER_OPERATORS`, Mongo `$`-tokens) *onto* this one, and
82+
* a value that is not in this list produces a condition row whose operator
83+
* select has nothing to select. Those tables assert against this export
84+
* rather than restating it, so adding an operator here is the only edit
85+
* needed to widen them.
86+
*/
87+
export const FILTER_BUILDER_OPERATORS = defaultOperators.map(o => o.value)
88+
89+
/** An operator id the FilterBuilder can render. */
90+
export type FilterBuilderOperator = (typeof defaultOperators)[number]['value']
7591

7692
const useSafeFilterTranslation = createSafeTranslation(
7793
{

packages/components/src/renderers/action/action-button.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ const ActionButtonRenderer = forwardRef<HTMLButtonElement, ActionButtonProps>(
9999
// Static "open in new tab" switch for url actions — forward so the
100100
// runner's executeUrl honors it (dropped, the toggle is silently lost).
101101
openIn: (schema as any).openIn,
102-
execute: schema.execute,
103102
endpoint: schema.endpoint,
104103
method: schema.method,
105104
...paramsPayload,

packages/components/src/renderers/action/action-group.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ const ActionGroupRenderer = forwardRef<HTMLDivElement, { schema: ActionGroupSche
200200
name: action.name,
201201
target: action.target,
202202
openIn: (action as any).openIn,
203-
execute: action.execute,
204203
endpoint: action.endpoint,
205204
method: action.method,
206205
params: action.params as Record<string, any> | undefined,

0 commit comments

Comments
 (0)