Skip to content

Commit eddd4a1

Browse files
authored
feat(console): settings validation errors render against the fields that caused them (objectstack#4224 follow-up) (#3083)
A `SETTINGS_VALIDATION` rejection names the offending keys, and the settings page threw all of it away — every failure collapsed into one toast carrying the server's summary sentence, with nothing marked on the inputs. 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 that 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 mark is on the CONTROL, not just beside it — `aria-invalid` plus `aria-describedby` pointing at a `role="alert"` message, wired in `wrapper` so every input type is covered at once. Colour is not a cue for everyone. - Cleared per-field the moment that field is edited, and wholesale on Discard, on reload, and on a successful save. - Fields the server did not name stay unmarked; a failure carrying no field array behaves exactly as before. The toast still fires alongside the marks. Five tests drive the real post-#4224 wire body rather than a hand-tuned fixture, and were verified to fail without the wiring.
1 parent 07de839 commit eddd4a1

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)