From 4e5df663899143b40040785954a55a7ee8a0b718 Mon Sep 17 00:00:00 2001 From: Yashas Date: Mon, 7 Sep 2026 16:57:20 +0530 Subject: [PATCH] form: say what is missing, in the field's own name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submitting the new-lead form with a blank Product Line produced: 400 The workflow rejected this submission. validation failed for activity zk-act-init-agent: product_line_4(required) That asks an operator to know what a field_s_id is, that `_4` is a disambiguating suffix and not part of a name, that "required" is a rule rather than a value, and that 400 means them and not us. All four are ours to know. Two changes, and the first means the second is rarely reached. The form now checks required fields before it sends anything. It marks each empty one — on the label, because somebody scanning a thirteen-field form is looking for the NAME they missed, not for a red box — scrolls the first into view, and says "Product Line is required". No round trip, and nothing to decode. The marking clears as the field is filled, since a field still flagged after being corrected teaches people to ignore the flagging. If the workflow does reject a submission on fields — it validates more than this form can know about — describeValidation now renders the reply in the form's own labels. It parses every `key(reason)` pair, maps the reason to a sentence, and resolves the key through the schema, including the suffix rule the prefill seeding uses. A key the schema does not carry keeps its raw name: a wrong label is worse than an ugly one, because the operator goes looking for a field that is not there. The status code is dropped from that surface. "400" is the first thing read and the least useful thing shown; it stays on the errors an operator genuinely cannot fix by typing. --- src/api/errors.js | 58 +++++++++++++++++++++++ src/components/ActivityForm.css | 27 +++++++++++ src/components/ActivityForm.jsx | 82 +++++++++++++++++++++++++++++++-- 3 files changed, 162 insertions(+), 5 deletions(-) diff --git a/src/api/errors.js b/src/api/errors.js index 1b0a531..066c591 100644 --- a/src/api/errors.js +++ b/src/api/errors.js @@ -70,3 +70,61 @@ export function describeError(err) { return { kind: 'unknown', title: raw || 'Something went wrong.', detail: null } } + +/** + * Turns the workflow's field-validation reply into something a person can act + * on, using the form's own labels. + * + * The server answers with the machine keys and a bare reason: + * + * validation failed for activity zk-act-init-agent: product_line_4(required) + * + * Shown raw, that asks an operator to know what a field_s_id is, that `_4` is a + * disambiguating suffix rather than part of a name, and that "required" is a + * rule and not a value. All three are ours to know, not theirs. + * + * `fields` is the schema array, so the reply is rendered in the same words the + * form used a line above it — "Product Line", not product_line_4. A key the + * schema does not carry keeps its raw name: a wrong label is worse than an + * ugly one, because the operator goes looking for a field that is not there. + * + * Returns null when the message is not a field-validation reply, so callers + * can fall through to describeError. + */ +export function describeValidation(rawMessage, fields = []) { + const raw = String(rawMessage ?? '') + if (!/validation failed/i.test(raw)) return null + + const label = (key) => + fields.find((f) => f.id === key)?.name ?? + // Same suffix rule the prefill seeding uses: the activity key may carry a + // numeric tail the global name does not. + fields.find((f) => String(f.id).replace(/_\d+$/, '') === key)?.name ?? + key + + const reasons = { + required: (n) => `${n} is required`, + unknown: (n) => `${n} is not a field this form accepts`, + enum: (n) => `${n} is not one of the available choices`, + invalid: (n) => `${n} is not valid`, + type: (n) => `${n} is the wrong kind of value`, + } + + const items = [] + const re = /([A-Za-z0-9_.]+)\s*\(([^)]+)\)/g + let m + while ((m = re.exec(raw)) !== null) { + const name = label(m[1]) + const why = String(m[2]).toLowerCase().trim() + const key = Object.keys(reasons).find((k) => why.includes(k)) + items.push(key ? reasons[key](name) : `${name}: ${why}`) + } + + if (!items.length) return null + return { + kind: 'validation', + // One problem reads as a sentence; several read as a list. + title: items.length === 1 ? items[0] : 'Some details are missing or not accepted.', + items, + } +} diff --git a/src/components/ActivityForm.css b/src/components/ActivityForm.css index 5ff988c..2f7bd4b 100644 --- a/src/components/ActivityForm.css +++ b/src/components/ActivityForm.css @@ -322,3 +322,30 @@ border-radius: var(--r-md); padding: 8px 11px; } + +/* A field the form is waiting on. Marked on the label rather than the input so + the name is highlighted too — an operator scanning a wide form is looking for + the LABEL they missed, not for a red box. */ +.af__field.is-missing > span { color: var(--zk-danger-ink); } +.af__field.is-missing input, +.af__field.is-missing select, +.af__field.is-missing textarea { + border-color: var(--zk-danger-line); + background: var(--zk-danger-tint); +} +.af__field.is-missing input:focus, +.af__field.is-missing select:focus, +.af__field.is-missing textarea:focus { + border-color: var(--zk-danger); + outline-color: var(--zk-danger); +} + +/* The form talking about itself, or relaying a field-level refusal. Distinct + from af__err, which is for a failure the operator cannot fix by typing. */ +.af__err--soft { + background: var(--zk-danger-tint); + border: 1px solid var(--zk-danger-line); + color: var(--zk-danger-ink); +} +.af__err--soft ul { margin: 6px 0 0; padding-left: 18px; } +.af__err--soft li { margin: 2px 0; } diff --git a/src/components/ActivityForm.jsx b/src/components/ActivityForm.jsx index df1bfe1..5872860 100644 --- a/src/components/ActivityForm.jsx +++ b/src/components/ActivityForm.jsx @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react' import { useZino } from '../api/provider.jsx' import { fieldApplies } from '../api/config.js' -import { describeError } from '../api/errors.js' +import { describeError, describeValidation } from '../api/errors.js' import FileField from './FileField.jsx' import './ActivityForm.css' @@ -37,6 +37,10 @@ export default function ActivityForm({ activityUid, instanceId, lead, onDone, on const [schema, setSchema] = useState(null) const [values, setValues] = useState({}) const [error, setError] = useState(null) + // Which required fields were empty on the last attempt. Kept separate from + // `error`, because this is the form talking about itself rather than the + // server refusing something. + const [missing, setMissing] = useState([]) const [busy, setBusy] = useState(false) useEffect(() => { @@ -78,7 +82,12 @@ export default function ActivityForm({ activityUid, instanceId, lead, onDone, on return () => { dead = true } }, [client, activityUid, instanceId]) - function set(id, v) { setValues((p) => ({ ...p, [id]: v })) } + function set(id, v) { + setValues((p) => ({ ...p, [id]: v })) + // Clear the complaint as soon as the field is filled. Leaving a red field + // marked after it has been corrected teaches people to ignore the marking. + setMissing((p) => (p.includes(id) ? p.filter((x) => x !== id) : p)) + } // Computed before the early returns below use it, and before submit: a field // that was never offered must never be sent. @@ -97,8 +106,34 @@ export default function ActivityForm({ activityUid, instanceId, lead, onDone, on const hidesMandatory = schema ? schema.fields.some((f) => f.mandatory && !fieldApplies(f.id, lead)) : false const fields = applicable.length && !hidesMandatory ? applicable : (schema?.fields ?? []) + function isEmpty(v) { + return v === undefined || v === null || v === '' || (Array.isArray(v) && v.length === 0) + } + async function submit(e) { e.preventDefault() + + // Check here rather than letting the workflow do it. The server's answer is + // correct and unreadable — "product_line_4(required)" — and it costs a round + // trip to be told something this form already knew. id_gen is issued + // server-side and is never the operator's to fill. + const gaps = fields.filter((f) => f.mandatory && f.data_type !== 'id_gen' && isEmpty(values[f.id])) + if (gaps.length) { + setMissing(gaps.map((f) => f.id)) + setError(null) + // Put the first offender on screen. On a form this wide the empty field + // is often above the fold and the message below it. Scrolling to the + // LABEL rather than focusing an input works for every field type, + // including the file and OCR widgets that render no input at all. + requestAnimationFrame(() => { + document + .querySelector('.af__field.is-missing') + ?.scrollIntoView({ behavior: 'smooth', block: 'center' }) + }) + return + } + + setMissing([]) setBusy(true); setError(null) // Send only fields the activity defines. A submission is schema-validated // and an unknown field is fatal, so empties are dropped rather than sent. @@ -136,7 +171,12 @@ export default function ActivityForm({ activityUid, instanceId, lead, onDone, on const opts = f.properties?.options || [] const v = values[f.id] ?? (f.data_type === 'multiselect' ? [] : '') return ( -