From b2445b0325915c47b687bcf7b60c0569abc387b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 18:16:48 +0000 Subject: [PATCH 1/4] feat: progressive validation in the compute form A new job opened the Compute tab on four red 'The field is required' errors - complaints about things the reader had not been given a chance to do yet. The form validates live, so every required field reported itself on first paint. Errors are now shown only for fields the reader has actually edited, tracked from the field id RJSF passes to onChange. A new showAllErrors prop (default off) reveals everything at once for the cases where the whole form must answer for itself - submit, and the preflight check coming in phase 2. Note the RJSForm remount keyed on showAllErrors: RJSF only re-validates when its schema or form data change, so without it the toggle left the previous validation result on screen. Demo: 'Start empty' reproduces the state a new job starts in, and the 'Show all errors' switch flips between the two modes. LOAD JSON now remounts the form too - ComputeForm derives its data in the constructor, so loading JSON previously did nothing to the form. SOF-8023 phase 1.1. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DK8KomMescJvMQNSEfeRR8 --- src/components/Compute.jsx | 4 ++ src/components/ComputeForm.tsx | 52 ++++++++++++++++++-- src/standalone/index.tsx | 37 ++++++++++++++ src/utils/touchedFields.ts | 60 +++++++++++++++++++++++ tests/touchedFields.tests.ts | 89 ++++++++++++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 5 deletions(-) create mode 100644 src/utils/touchedFields.ts create mode 100644 tests/touchedFields.tests.ts diff --git a/src/components/Compute.jsx b/src/components/Compute.jsx index 799850e..60db8b4 100644 --- a/src/components/Compute.jsx +++ b/src/components/Compute.jsx @@ -88,6 +88,7 @@ class Compute extends React.Component { showAdvancedOptions, accountUsers, isAccountUsersLoading, + showAllErrors, } = this.props; return ( @@ -126,6 +127,7 @@ class Compute extends React.Component { showAdvancedOptions={showAdvancedOptions} accountUsers={accountUsers} isAccountUsersLoading={isAccountUsersLoading} + showAllErrors={showAllErrors} /> )} {this.showStatusTrack && ( @@ -150,6 +152,8 @@ Compute.propTypes = { showStatusTrack: PropTypes.bool, showAdvancedOptions: PropTypes.bool, accountUsers: PropTypes.array, + /** Reveal every validation error, not just those for fields already touched. */ + showAllErrors: PropTypes.bool, }; Compute.defaultProps = { diff --git a/src/components/ComputeForm.tsx b/src/components/ComputeForm.tsx index b988010..c531d22 100644 --- a/src/components/ComputeForm.tsx +++ b/src/components/ComputeForm.tsx @@ -28,6 +28,7 @@ import omitBy from "lodash/omitBy"; import React from "react"; import { getComputeSchema, getComputeValidator } from "../validators"; +import { shouldShowFieldError, withTouchedField } from "../utils/touchedFields"; import Notify from "./Notify"; import QueuesTable from "./QueuesTable"; @@ -309,10 +310,19 @@ interface ComputeFormProps { onUpdate: (s: string) => void; appName?: string; pathForClusters?: string; + /** + * Reveals every validation error at once, including for fields the reader + * has not touched. Off by default: a form the reader has not filled in yet + * should not open by listing everything wrong with it. Turn it on when the + * whole form has to answer for itself — on submit, or from a preflight check. + */ + showAllErrors?: boolean; } interface ComputeFormState { formData: any; + /** Form-data keys the reader has edited; see `utils/touchedFields`. */ + touchedFields: ReadonlySet; } export class ComputeForm extends React.Component { @@ -334,6 +344,7 @@ export class ComputeForm extends React.Component(), }; this.handleFormUpdate = this.handleFormUpdate.bind(this); this.onNotifyUpdate = this.onNotifyUpdate.bind(this); @@ -344,10 +355,21 @@ export class ComputeForm extends React.Component }) { - this.setState({ formData }, () => { - this.updateForm(); - }); + /** + * `fieldId` is RJSF's id for the field that changed. It is what makes + * progressive validation possible: errors stay hidden until the reader has + * been to the field in question. + */ + handleFormUpdate({ formData }: { formData: Record }, fieldId?: string) { + this.setState( + (previousState) => ({ + formData, + touchedFields: withTouchedField(previousState.touchedFields, fieldId), + }), + () => { + this.updateForm(); + }, + ); } onNotifyUpdate(notify: Record) { @@ -404,9 +426,20 @@ export class ComputeForm extends React.Component) => { const { params } = obj; const { name, message } = this.getErrorMessage(obj); + const fieldName = params.missingProperty || name; + + // The form validates live, so without this every required field + // reports itself on first paint — before the reader has had a chance + // to fill anything in. + if (!shouldShowFieldError({ fieldName, touchedFields, showAllErrors })) { + return; + } if (params.missingProperty) { errors[params.missingProperty]?.addError("The field is required"); @@ -459,6 +492,7 @@ export class ComputeForm extends React.Component this.handleFormUpdate(event)} + onChange={(event: any, fieldId?: string) => + this.handleFormUpdate(event, fieldId) + } showErrorList={false} customValidate={this.customValidate} liveValidate diff --git a/src/standalone/index.tsx b/src/standalone/index.tsx index bf55121..290f72d 100644 --- a/src/standalone/index.tsx +++ b/src/standalone/index.tsx @@ -177,6 +177,23 @@ function App() { const [compute, setCompute] = useState(defaultComputeConfig); const [jsonInput, setJsonInput] = useState(JSON.stringify(defaultComputeConfig, null, 2)); const [jsonError, setJsonError] = useState(""); + const [showAllErrors, setShowAllErrors] = useState(false); + // Remount key: clearing the form has to reset the touched-field state too, + // otherwise the previous session's touches keep their errors on screen. + const [computeInstanceKey, setComputeInstanceKey] = useState(0); + + /** + * The state a brand-new job actually starts in. Worth one click in the demo: + * it is the case progressive validation exists for, and the one where the + * form used to open on a wall of "The field is required". + */ + const loadEmptyCompute = useCallback(() => { + const emptyCompute = { cluster: {}, arguments: {} }; + setCompute(emptyCompute as typeof defaultComputeConfig); + setJsonInput(JSON.stringify(emptyCompute, null, 2)); + setJsonError(""); + setComputeInstanceKey((previousKey) => previousKey + 1); + }, []); const mockJob = useMemo(() => { return { @@ -205,6 +222,10 @@ function App() { const parsed = JSON.parse(jsonInput); setCompute(parsed); setJsonError(""); + // ComputeForm derives its form data once, in its constructor, so a new + // `compute` prop alone is invisible to it — loading JSON did nothing to + // the form without this remount. + setComputeInstanceKey((previousKey) => previousKey + 1); } catch (e: any) { setJsonError(e.message); } @@ -256,6 +277,20 @@ function App() { } label="Editable" /> + {/* Progressive validation: an untouched form stays quiet until + something downstream (submit, preflight) demands the full picture. */} + setShowAllErrors(e.target.checked)} + /> + } + label="Show all errors" + /> + @@ -264,6 +299,7 @@ function App() { {/* Left Pane: Interactive Form */} diff --git a/src/utils/touchedFields.ts b/src/utils/touchedFields.ts new file mode 100644 index 0000000..ba2686d --- /dev/null +++ b/src/utils/touchedFields.ts @@ -0,0 +1,60 @@ +/** + * Progressive validation state for the compute form. + * + * The form validates live, which used to mean a brand-new job opened on a wall + * of "The field is required" — an error message about something the reader has + * not had a chance to do yet. These helpers narrow that to fields the reader has + * actually interacted with, while still allowing every error to be revealed at + * once when something downstream (submit preflight) needs the full picture. + */ + +/** RJSF prefixes generated field ids with this; `root_nodes` → `nodes`. */ +const RJSF_FIELD_ID_PREFIX = "root_"; + +/** + * Maps an RJSF field id to the key used in form data and in the validation + * error object. The compute form flattens its data, so a nested property + * arrives as a single dotted key (`root_cluster.fqdn` → `cluster.fqdn`). + * + * Returns null for ids that are not field ids (the form root itself, or an + * absent id) — callers treat that as "nothing became touched". + */ +export function fieldNameFromFieldId(fieldId?: string | null): string | null { + if (!fieldId || !fieldId.startsWith(RJSF_FIELD_ID_PREFIX)) return null; + + return fieldId.slice(RJSF_FIELD_ID_PREFIX.length) || null; +} + +/** + * Returns the touched set including `fieldId`. The same set instance is + * returned when nothing changed, so callers can skip a re-render. + */ +export function withTouchedField( + touchedFields: ReadonlySet, + fieldId?: string | null, +): ReadonlySet { + const fieldName = fieldNameFromFieldId(fieldId); + + if (!fieldName || touchedFields.has(fieldName)) return touchedFields; + + return new Set(touchedFields).add(fieldName); +} + +export interface FieldErrorVisibilityOptions { + /** Form-data key the error belongs to, e.g. "nodes" or "cluster.fqdn". */ + fieldName?: string | null; + touchedFields: ReadonlySet; + /** Set once the whole form must own up — submit, preflight, an explicit check. */ + showAllErrors: boolean; +} + +export function shouldShowFieldError({ + fieldName, + touchedFields, + showAllErrors, +}: FieldErrorVisibilityOptions): boolean { + if (showAllErrors) return true; + if (!fieldName) return false; + + return touchedFields.has(fieldName); +} diff --git a/tests/touchedFields.tests.ts b/tests/touchedFields.tests.ts new file mode 100644 index 0000000..96b7cb0 --- /dev/null +++ b/tests/touchedFields.tests.ts @@ -0,0 +1,89 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { + fieldNameFromFieldId, + shouldShowFieldError, + withTouchedField, +} from "../src/utils/touchedFields"; + +describe("fieldNameFromFieldId", () => { + it("strips RJSF's root prefix", () => { + assert.equal(fieldNameFromFieldId("root_nodes"), "nodes"); + }); + + it("keeps dotted keys intact, since compute form data is flattened", () => { + assert.equal(fieldNameFromFieldId("root_cluster.fqdn"), "cluster.fqdn"); + }); + + it("returns null for the form root, an unprefixed id, or no id at all", () => { + assert.equal(fieldNameFromFieldId("root_"), null); + assert.equal(fieldNameFromFieldId("nodes"), null); + assert.equal(fieldNameFromFieldId(undefined), null); + assert.equal(fieldNameFromFieldId(null), null); + }); +}); + +describe("withTouchedField", () => { + it("adds the field a change came from", () => { + const touched = withTouchedField(new Set(), "root_ppn"); + assert.deepEqual([...touched], ["ppn"]); + }); + + it("accumulates across changes", () => { + let touched: ReadonlySet = new Set(); + touched = withTouchedField(touched, "root_nodes"); + touched = withTouchedField(touched, "root_cluster.fqdn"); + assert.deepEqual([...touched].sort(), ["cluster.fqdn", "nodes"]); + }); + + it("returns the same set when nothing new was touched, so callers can skip work", () => { + const touched = new Set(["nodes"]); + assert.equal(withTouchedField(touched, "root_nodes"), touched); + assert.equal(withTouchedField(touched, undefined), touched); + }); + + it("does not mutate the set it was given", () => { + const touched = new Set(["nodes"]); + withTouchedField(touched, "root_ppn"); + assert.deepEqual([...touched], ["nodes"]); + }); +}); + +describe("shouldShowFieldError", () => { + const touchedFields = new Set(["nodes"]); + + it("hides errors for fields the reader has not been to", () => { + // The point of the whole module: a new job must not open on a wall of + // "The field is required". + assert.equal( + shouldShowFieldError({ fieldName: "ppn", touchedFields, showAllErrors: false }), + false, + ); + }); + + it("shows errors once the field has been touched", () => { + assert.equal( + shouldShowFieldError({ fieldName: "nodes", touchedFields, showAllErrors: false }), + true, + ); + }); + + it("shows everything when the whole form has to answer for itself", () => { + assert.equal( + shouldShowFieldError({ fieldName: "ppn", touchedFields, showAllErrors: true }), + true, + ); + assert.equal( + shouldShowFieldError({ fieldName: null, touchedFields, showAllErrors: true }), + true, + ); + }); + + it("stays quiet for errors it cannot attribute to a field", () => { + assert.equal( + shouldShowFieldError({ fieldName: undefined, touchedFields, showAllErrors: false }), + false, + ); + }); +}); From b60568bbef2920bcd4d7aeec59cc75cd2a8fa044 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 20:28:31 +0000 Subject: [PATCH 2/4] build: regenerate dist for progressive compute validation dist/ is tracked in this repo and the published package ships it ("files": ["/dist", "/src"]). There is no prepublishOnly build here, so a stale dist is what consumers would get - and dist/utils/touchedFields.js was missing entirely, which ComputeForm.js imports. The husky pre-commit hook does this automatically (npm run transpile && git add dist/), but only once hooks are installed; this repo has no 'prepare: husky install' script, so a fresh clone commits without it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DK8KomMescJvMQNSEfeRR8 --- dist/components/Compute.d.ts | 1 + dist/components/Compute.js | 6 +++-- dist/components/ComputeForm.d.ts | 16 ++++++++++++- dist/components/ComputeForm.js | 33 +++++++++++++++++++++---- dist/utils/touchedFields.d.ts | 31 ++++++++++++++++++++++++ dist/utils/touchedFields.js | 41 ++++++++++++++++++++++++++++++++ 6 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 dist/utils/touchedFields.d.ts create mode 100644 dist/utils/touchedFields.js diff --git a/dist/components/Compute.d.ts b/dist/components/Compute.d.ts index 4386aca..0b195c4 100644 --- a/dist/components/Compute.d.ts +++ b/dist/components/Compute.d.ts @@ -28,6 +28,7 @@ declare namespace Compute { let showStatusTrack: PropTypes.Requireable; let showAdvancedOptions: PropTypes.Requireable; let accountUsers: PropTypes.Requireable; + let showAllErrors: PropTypes.Requireable; } namespace defaultProps { let editable_1: boolean; diff --git a/dist/components/Compute.js b/dist/components/Compute.js index 6b37021..dbc9d43 100644 --- a/dist/components/Compute.js +++ b/dist/components/Compute.js @@ -65,8 +65,8 @@ class Compute extends React.Component { return Boolean(showStatusTrack && job.statusTrack && job.statusTrack.length); } render() { - const { className, showHeader, isLoading, adjustable, editable, showComputeForm, compute, user, account, clusters, onUpdate, job, showAdvancedOptions, accountUsers, isAccountUsersLoading, } = this.props; - return (_jsxs("div", { className: setClass(className, "wizard-step", "compute-step"), children: [showHeader ? (_jsxs(EntityHeaderContainer, { children: [_jsx(EntityHeader, { name: "Compute", subtitle: "Runtime configuration parameters", icon: "pages.compute", isLoading: isLoading, editable: false, adjustable: true, isDescriptionEditorHidden: true }), adjustable || editable ? (_jsx(AutoSetActionContainer, { children: _jsx(Dropdown, { className: "pull-right", actions: this.getDropdownAction(), children: _jsx(DropdownButton, { children: _jsx(IconByName, { name: "shapes.dots.vertical" }) }) }) })) : null] })) : null, showComputeForm && (_jsx(ComputeForm, { editable: editable, compute: compute, user: user, account: account, clusters: clusters, onUpdate: onUpdate, appName: job.workflow.usedApplicationNames[0], showAdvancedOptions: showAdvancedOptions, accountUsers: accountUsers, isAccountUsersLoading: isAccountUsersLoading })), this.showStatusTrack && (_jsx(Box, { p: 2, children: _jsx(StatusTrackTable, { entity: job }) }))] })); + const { className, showHeader, isLoading, adjustable, editable, showComputeForm, compute, user, account, clusters, onUpdate, job, showAdvancedOptions, accountUsers, isAccountUsersLoading, showAllErrors, } = this.props; + return (_jsxs("div", { className: setClass(className, "wizard-step", "compute-step"), children: [showHeader ? (_jsxs(EntityHeaderContainer, { children: [_jsx(EntityHeader, { name: "Compute", subtitle: "Runtime configuration parameters", icon: "pages.compute", isLoading: isLoading, editable: false, adjustable: true, isDescriptionEditorHidden: true }), adjustable || editable ? (_jsx(AutoSetActionContainer, { children: _jsx(Dropdown, { className: "pull-right", actions: this.getDropdownAction(), children: _jsx(DropdownButton, { children: _jsx(IconByName, { name: "shapes.dots.vertical" }) }) }) })) : null] })) : null, showComputeForm && (_jsx(ComputeForm, { editable: editable, compute: compute, user: user, account: account, clusters: clusters, onUpdate: onUpdate, appName: job.workflow.usedApplicationNames[0], showAdvancedOptions: showAdvancedOptions, accountUsers: accountUsers, isAccountUsersLoading: isAccountUsersLoading, showAllErrors: showAllErrors })), this.showStatusTrack && (_jsx(Box, { p: 2, children: _jsx(StatusTrackTable, { entity: job }) }))] })); } } Compute.propTypes = { @@ -81,6 +81,8 @@ Compute.propTypes = { showStatusTrack: PropTypes.bool, showAdvancedOptions: PropTypes.bool, accountUsers: PropTypes.array, + /** Reveal every validation error, not just those for fields already touched. */ + showAllErrors: PropTypes.bool, }; Compute.defaultProps = { editable: true, diff --git a/dist/components/ComputeForm.d.ts b/dist/components/ComputeForm.d.ts index bae6671..82689cc 100644 --- a/dist/components/ComputeForm.d.ts +++ b/dist/components/ComputeForm.d.ts @@ -27,9 +27,18 @@ interface ComputeFormProps { onUpdate: (s: string) => void; appName?: string; pathForClusters?: string; + /** + * Reveals every validation error at once, including for fields the reader + * has not touched. Off by default: a form the reader has not filled in yet + * should not open by listing everything wrong with it. Turn it on when the + * whole form has to answer for itself — on submit, or from a preflight check. + */ + showAllErrors?: boolean; } interface ComputeFormState { formData: any; + /** Form-data keys the reader has edited; see `utils/touchedFields`. */ + touchedFields: ReadonlySet; } export declare class ComputeForm extends React.Component { computeUiSchema: UISchema; @@ -37,9 +46,14 @@ export declare class ComputeForm extends React.Component