Skip to content

Commit 734de98

Browse files
committed
feat(console): settings validation errors render against the fields that caused them (objectstack#4224 follow-up)
A `SETTINGS_VALIDATION` rejection names the offending keys, and this page threw all of it away — every failure collapsed into one toast carrying the server's summary sentence, with nothing marked on the inputs. On a namespace with a dozen keys the user was told a value was wrong and left to find which. That was not the console's fault. The server sent `fields` as a `Record<key, message>` hung BESIDE `error.code`, a position `ApiErrorSchema` never declared; it survived only because the schema is a plain `z.object` that strips undeclared keys instead of rejecting them. `extractFieldErrors` reads arrays (`details.fields` / `fields` / `validationErrors`), so a map at an undeclared position matched nothing and returned null. objectstack#4224 moved it to `error.details.fields` as the declared `FieldError[]` — which is what makes this wiring a few lines rather than a parser. - `SettingsField` gains an `error` prop, rendered in the slot the help text occupies rather than stacked under it: the two say the same kind of thing, and a description sitting below a red error reads as two states at once. - The mark is on the CONTROL, not just beside it — `aria-invalid` plus `aria-describedby` pointing at a `role="alert"` message. Colour is not a cue for everyone. Wiring it in `wrapper` covers every input type at once. - Cleared per-field the moment that field is edited (the error describes the value the server saw, so it contradicts what is in the box), and wholesale on Discard, on reload, and on a successful save. - Fields the server did not name stay unmarked, and a failure carrying no field array (500, unknown namespace) behaves exactly as before. The toast still fires alongside the marks: the offending field can be scrolled out of view or hidden behind a `visible` expression, and a save that appears to do nothing is the worse failure. Five tests drive the real post-#4224 wire body rather than a hand-tuned fixture, so a server regression to the map turns them red instead of passing against a shape nobody sends. Verified to fail without the wiring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tv2NYMBTrzVVRWiWWuyec5
1 parent c4d7b20 commit 734de98

4 files changed

Lines changed: 362 additions & 6 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@object-ui/console": patch
3+
---
4+
5+
feat(settings): a rejected save marks the fields that caused it — objectstack#4224 follow-up
6+
7+
A `SETTINGS_VALIDATION` rejection names the offending keys, and the settings page
8+
threw all of it away. Every failure collapsed into one toast carrying the
9+
server's summary sentence, with nothing marked on the inputs — so on a namespace
10+
with a dozen keys the user was told a value was wrong and left to find which.
11+
12+
**That was not the console's fault, which is the part worth recording.** The
13+
server sent `fields` as a `Record<key, message>` hung *beside* `error.code`, a
14+
position `ApiErrorSchema` never declared — it survived only because the schema
15+
is a plain `z.object` and strips undeclared keys rather than rejecting them.
16+
`extractFieldErrors` reads arrays (`details.fields`, `fields`,
17+
`validationErrors`), so a map at an undeclared position matched nothing and
18+
returned `null`. objectstack#4224 moved it to `error.details.fields` as the
19+
declared `FieldError[]`, which is what makes this wiring a few lines rather than
20+
a parser.
21+
22+
What changes for a user: the server's message now renders against the input that
23+
caused it, in the slot the help text occupies, and clears the moment that field
24+
is edited, on Discard, or on the next successful save. `SettingsField` gained an
25+
`error` prop; it sets `aria-invalid` and `aria-describedby` on the control and
26+
gives the message `role="alert"`, so the rejection is announced rather than being
27+
conveyed by colour alone.
28+
29+
The toast still fires alongside the per-field marks. The offending field can be
30+
scrolled out of view or hidden behind a `visible` expression, and a save that
31+
appears to do nothing is the worse failure.
32+
33+
Fields the server did not name are left unmarked — a wrong mark on an innocent
34+
input is worse than the generic toast that was already there — and a failure
35+
carrying no field array (a 500, an unknown namespace) behaves exactly as before.

apps/console/src/pages/settings/SettingsField.tsx

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* (which carries provenance and lock state).
77
*/
88

9-
import { useId } from 'react';
9+
import { cloneElement, isValidElement, useId, type ReactElement } from 'react';
1010
import {
1111
Input,
1212
Textarea,
@@ -46,6 +46,16 @@ export interface SettingsFieldProps {
4646
locked?: boolean;
4747
/** i18n helpers bound to the parent settings namespace. */
4848
labels?: SettingsLabelHelpers;
49+
/**
50+
* Server-side rejection for THIS field, from the last failed save
51+
* (objectstack#4224). Already localized by the server — rendered verbatim,
52+
* not re-worded here.
53+
*
54+
* Replaces the help text while present, rather than stacking below it: the
55+
* two occupy the same slot and say the same kind of thing, and a description
56+
* sitting under a red error reads as if the field has two states at once.
57+
*/
58+
error?: string;
4959
}
5060

5161
function InheritanceBadges({
@@ -132,8 +142,25 @@ function FieldDescription({ description }: { description?: string }) {
132142
return <p className="text-xs text-muted-foreground mt-1">{description}</p>;
133143
}
134144

145+
/**
146+
* The server's rejection for this field, in the slot the help text normally
147+
* occupies (objectstack#4224).
148+
*
149+
* `role="alert"` so a screen reader announces it when it appears after a save
150+
* attempt — the sighted cue is colour, which is not a cue at all for everyone.
151+
* The `id` is what the input points `aria-describedby` at, so the association
152+
* survives for assistive tech rather than being purely visual adjacency.
153+
*/
154+
function FieldError({ id, message }: { id: string; message: string }) {
155+
return (
156+
<p id={id} role="alert" className="text-xs text-destructive mt-1">
157+
{message}
158+
</p>
159+
);
160+
}
161+
135162
export function SettingsField(props: SettingsFieldProps) {
136-
const { spec, resolved, value, onChange, onAction, locked, saving, labels } = props;
163+
const { spec, resolved, value, onChange, onAction, locked, saving, labels, error } = props;
137164
const id = useId();
138165
const disabled = Boolean(locked || saving);
139166
const literalLabel = resolveLabel(spec.label);
@@ -231,11 +258,26 @@ export function SettingsField(props: SettingsFieldProps) {
231258

232259
// -------- Inputs --------
233260

261+
// Wired onto the control itself, not just rendered beside it: `aria-invalid`
262+
// is what announces "this one was rejected", and `aria-describedby` is what
263+
// ties the message to the input for a screen reader. Every input type goes
264+
// through `wrapper`, so marking it here covers all of them at once instead of
265+
// per-case (objectstack#4224).
266+
const errorId = `${id}-error`;
234267
const wrapper = (children: React.ReactNode) => (
235268
<div className="space-y-1.5 py-2">
236269
<FieldHeader spec={spec} resolved={resolved} labelText={fieldLabel} labels={labels} />
237-
{children}
238-
<FieldDescription description={fieldHelp} />
270+
{error && isValidElement(children)
271+
? cloneElement(children as ReactElement<Record<string, unknown>>, {
272+
'aria-invalid': true,
273+
'aria-describedby': errorId,
274+
})
275+
: children}
276+
{error ? (
277+
<FieldError id={errorId} message={error} />
278+
) : (
279+
<FieldDescription description={fieldHelp} />
280+
)}
239281
</div>
240282
);
241283

apps/console/src/pages/settings/SettingsView.tsx

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { useParams, useNavigate } from 'react-router-dom';
1111
import { toast } from 'sonner';
1212
import { Loader2, ArrowLeft, RotateCcw } from 'lucide-react';
1313
import { Button, Card, CardContent, Skeleton, Badge } from '@object-ui/components';
14+
import { extractFieldErrors } from '@object-ui/react';
1415
import { getIcon } from '../../utils/getIcon';
1516
import { SettingsField } from './SettingsField';
1617
import {
@@ -42,6 +43,12 @@ function evalVisibility(expr: string | undefined, data: Record<string, unknown>)
4243
}
4344
}
4445

46+
/** `{ ...rest }` without `key`, returned as a new object so React sees the change. */
47+
function omitKey<T>(map: Record<string, T>, key: string): Record<string, T> {
48+
const { [key]: _dropped, ...rest } = map;
49+
return rest;
50+
}
51+
4552
export function SettingsView() {
4653
const params = useParams<{ namespace?: string }>();
4754
const navigate = useNavigate();
@@ -53,6 +60,16 @@ export function SettingsView() {
5360
const [error, setError] = useState<string | null>(null);
5461
/** Live edit map keyed by spec.key. Falls back to resolved value when undefined. */
5562
const [draft, setDraft] = useState<Record<string, unknown>>({});
63+
/**
64+
* Per-field rejections from the last failed save, keyed by field name
65+
* (objectstack#4224). Empty until a save comes back `SETTINGS_VALIDATION`.
66+
*
67+
* Before #4224 the server sent this as a `Record<key, message>` hung beside
68+
* `error.code`, which `extractFieldErrors` could not read — so the whole
69+
* batch collapsed into one toast that named no field. It now arrives as the
70+
* declared `FieldError[]` under `error.details.fields`.
71+
*/
72+
const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({});
5673

5774
const load = useCallback(async () => {
5875
setLoading(true);
@@ -61,6 +78,7 @@ export function SettingsView() {
6178
const p = await getSettingsNamespace(namespace);
6279
setPayload(p);
6380
setDraft({});
81+
setFieldErrors({});
6482
} catch (err: any) {
6583
setError(err?.message ?? 'Failed to load settings');
6684
} finally {
@@ -127,6 +145,7 @@ export function SettingsView() {
127145
const res = await saveSettingsNamespace(namespace, draft);
128146
setPayload({ ...payload, values: { ...values, ...res.values } });
129147
setDraft({});
148+
setFieldErrors({});
130149
toast.success('Settings saved');
131150
} catch (err: any) {
132151
const apiError = err?.payload?.error;
@@ -135,6 +154,19 @@ export function SettingsView() {
135154
const key = lockedKeyOf(apiError);
136155
toast.error(key ? `Locked by environment: ${key}` : 'Locked by environment');
137156
} else {
157+
// Per-field rejections render against the inputs that caused them
158+
// (objectstack#4224). `extractFieldErrors` reads `details.fields`, so it
159+
// is handed `error` — the object that carries `details` — not the
160+
// whole body.
161+
//
162+
// The toast still fires: the offending field can be scrolled out of
163+
// view, or hidden behind a `visible` expression, and a save that
164+
// silently does nothing is the worse failure. It carries the server's
165+
// summary message, which already names the fields.
166+
const perField = extractFieldErrors(apiError);
167+
if (perField?.length) {
168+
setFieldErrors(Object.fromEntries(perField.map((f) => [f.field, f.message])));
169+
}
138170
toast.error(err?.message ?? 'Save failed');
139171
}
140172
} finally {
@@ -194,11 +226,20 @@ export function SettingsView() {
194226
spec={spec}
195227
resolved={resolved}
196228
value={current}
197-
onChange={(v) => key && setDraft((d) => ({ ...d, [key]: v }))}
229+
onChange={(v) => {
230+
if (!key) return;
231+
setDraft((d) => ({ ...d, [key]: v }));
232+
// Clear this field's rejection the moment it is edited. The
233+
// error describes the value the server saw, so keeping it on
234+
// screen while the user types contradicts what is in the box —
235+
// and the next save re-derives the set from scratch anyway.
236+
setFieldErrors((e) => (key in e ? omitKey(e, key) : e));
237+
}}
198238
onAction={spec.type === 'action_button' ? () => onAction(spec.id ?? key ?? 'test') : undefined}
199239
locked={resolved?.locked}
200240
saving={saving}
201241
labels={labels}
242+
error={key ? fieldErrors[key] : undefined}
202243
/>
203244
);
204245
})}
@@ -211,7 +252,17 @@ export function SettingsView() {
211252
{dirtyKeys.length} unsaved change{dirtyKeys.length > 1 ? 's' : ''}
212253
</div>
213254
<div className="flex gap-2">
214-
<Button variant="ghost" size="sm" onClick={() => setDraft({})} disabled={saving}>
255+
<Button
256+
variant="ghost"
257+
size="sm"
258+
onClick={() => {
259+
setDraft({});
260+
// Discarding reverts to the stored values, so rejections of
261+
// the edits being thrown away go with them.
262+
setFieldErrors({});
263+
}}
264+
disabled={saving}
265+
>
215266
<RotateCcw className="h-4 w-4 mr-1" /> Discard
216267
</Button>
217268
<Button onClick={onSave} disabled={saving}>

0 commit comments

Comments
 (0)