Skip to content

Add form reset button variable to defineInstrument, button revealed in instrument viewer and odc - #1518

Open
david-roper wants to merge 6 commits into
DouglasNeuroInformatics:mainfrom
david-roper:form-reset-btn
Open

Add form reset button variable to defineInstrument, button revealed in instrument viewer and odc#1518
david-roper wants to merge 6 commits into
DouglasNeuroInformatics:mainfrom
david-roper:form-reset-btn

Conversation

@david-roper

Copy link
Copy Markdown
Collaborator

feat: let an instrument opt into a form reset button

Adds resetButton to the defineInstrument form API. An author writes resetButton: true, and
every surface that renders the instrument shows a button that clears the subject's answers.

Motivation

libui's Form has always supported a reset button, but the instrument API never exposed it, so no
authored instrument could offer one. FormInstrument is a deliberately curated subset of libui's
props — it also omits readOnly, revalidateOnBlur and fieldsFooter — so surfacing one means
declaring it in the type, mirroring it in the schema, and forwarding it at the single mount point.

What changed

Six files, in dependency order:

File Change
packages/runtime-core/src/types/instrument.form.ts resetButton?: boolean on FormInstrument
packages/schemas/src/instrument/instrument.form.ts resetButton: z.boolean().optional() on $$FormInstrument
packages/react-core/.../FormContent/FormContent.tsx forwards it to libui
packages/instrument-library/.../DNP_ENHANCED_DEMOGRAPHICS_QUESTIONNAIRE/index.ts resetButton: true — the worked example
packages/instrument-guidelines/AGENTS.md the published authoring spec
testing/ + two test files coverage

The one subtlety worth reviewing

FormContent previously passed preventResetValuesOnReset unconditionally. libui's reset is:

const reset = () => {
  setRootErrors([]);                                    // always
  setErrors({});                                        // always
  if (!preventResetValuesOnReset) { setValues({}); }    // ← the only line that flag gates
};

So simply adding the button would have produced a Reset that clears validation errors and leaves
every answer in place
— visibly broken. The two props are one switch, and this PR ties them
together:

preventResetValuesOnReset={!instrument.resetButton}
resetBtn={instrument.resetButton}

The invariant: a reset button renders if and only if preventResetValuesOnReset is off. Never a
button that cannot clear; never silent clear-suppression when a button is offered.

Instruments that do not opt in are byte-identical to before.

Why the names differ

The authoring API is resetButton; libui's prop is resetBtn and is not ours to rename. The two
meet on one adapter line in FormContent, which is the only coupling — the component enumerates
props and never spreads, so nothing leaks through implicitly.

This cannot drift silently: FormProps indexes only [key: `data-${string}`], so passing the
wrong name is an excess-property error. Verified deliberately:

error TS2322: Property 'resetButton' does not exist on type
  'IntrinsicAttributes & FormProps<...>'. Did you mean 'resetBtn'?

If libui ever renames its prop, tsc fails on that one line rather than the button quietly
disappearing.

Behaviour

resetButton unset resetButton: true
Reset button not rendered rendered
Clicking it n/a clears every answer, immediately
After a successful submit values retained values cleared

The post-submit difference is not observable in the standard flow: the scalar renderer advances to
the summary (setIndex(2)) and the series renderer drops to its progress screen, so the form
unmounts either way. It is documented in the guidelines regardless, because it is real.

The button has no confirmation step and no undo — one click and the subject loses everything
entered. That is libui's behaviour, not something introduced here, and the guidelines now say so
explicitly, so authors opt in knowingly. It applies on every surface, including the patient-facing
gateway: the author opted in, and they know their instrument better than the platform does.

DNP_ENHANCED_DEMOGRAPHICS_QUESTIONNAIRE is the only instrument that opts in. It is the longest
form in the library — roughly twenty fields — which is where starting over beats clearing by hand.

Testing

Suite Result
pnpm lint 33/33 tasks
pnpm test 751 passed, 1 skipped
pnpm test:e2e 154/154 passed (Chromium, plus Firefox @smoke)

New coverage:

  • Type-level (runtime-core, checked by tsc in lint) — pins resetButton as
    boolean | undefined and confirms it is optional, so instruments predating it still satisfy the
    type.
  • Schema (packages/schemas) — that $FormInstrument preserves resetButton rather than
    stripping it, that omitting it still parses, and that a non-boolean is rejected.
  • E2E (testing/src/specs/form-reset.spec.ts, 3 tests) — the button clears a filled form; an
    instrument that does not opt in has no button; and a form still submits normally after a reset.

Both guards were checked against the failure they exist for

Rather than trusting that the new tests pass, I confirmed each one fails when its protection is
removed:

  • Deleting the line from $$FormInstrument2 schema tests fail; restoring it → 8 pass.
  • Reverting preventResetValuesOnReset to unconditional → the e2e fails with
    Expected: "" / Received: "3" — button rendered, field unchanged.

The schema test matters more than it looks: Zod strips undeclared keys, apps/web validates only in
development (validate: import.meta.env.DEV) and the playground always does. Omitting that one line
would have dropped the flag in exactly the places an author tests their instrument, while leaving it
working in production.

Reviewer notes

  • Two build artifacts must be regenerated, both gitignored: runtime-core (lib/ + dist/) and
    runtime/v1/dist. Instruments import through /runtime/v1/..., so tsc fails on a fresh clone
    until pnpm build runs. Nothing to commit.
  • packages/instrument-guidelines is a published npm package whose bin installs it into external
    instrument repos as their AGENTS.md. Its version tracks the root, and increment-version.sh
    rewrites every path, so this documentation ships with the next ordinary release — no separate
    action.
  • instrument-completion.spec.ts already drives the demographics instrument, so the existing suite
    exercises the changed instrument. It passes.
  • No new dependencies, no version bumps, no schema migration.

Out of scope

#1517

While tracing preventResetValuesOnReset I found and reproduced a pre-existing state-bleed in
skipProgress series: the form is not remounted between items, so answers carry forward. Filed
separately as #. It is unrelated to this feature and its fix belongs in SeriesInstrumentRenderer
or useInterpretedInstrument, neither of which this PR touches.

One interaction is worth knowing while reviewing: because this PR makes preventResetValuesOnReset
conditional, an instrument declaring resetButton: true becomes incidentally immune to that bug —
reset() now empties its form after each submit. That is a side effect, not a fix, and # says
so explicitly.

Also out of scope: any confirmation dialog on Reset, which would require an upstream libui change.

Closes issue #1391

@david-roper
david-roper requested a review from joshunrau as a code owner August 28, 2026 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant