Add form reset button variable to defineInstrument, button revealed in instrument viewer and odc - #1518
Open
david-roper wants to merge 6 commits into
Open
Conversation
…ust preventResetValuesOnReset to include button
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: let an instrument opt into a form reset button
Adds
resetButtonto thedefineInstrumentform API. An author writesresetButton: true, andevery surface that renders the instrument shows a button that clears the subject's answers.
Motivation
libui's
Formhas always supported a reset button, but the instrument API never exposed it, so noauthored instrument could offer one.
FormInstrumentis a deliberately curated subset of libui'sprops — it also omits
readOnly,revalidateOnBlurandfieldsFooter— so surfacing one meansdeclaring it in the type, mirroring it in the schema, and forwarding it at the single mount point.
What changed
Six files, in dependency order:
packages/runtime-core/src/types/instrument.form.tsresetButton?: booleanonFormInstrumentpackages/schemas/src/instrument/instrument.form.tsresetButton: z.boolean().optional()on$$FormInstrumentpackages/react-core/.../FormContent/FormContent.tsxpackages/instrument-library/.../DNP_ENHANCED_DEMOGRAPHICS_QUESTIONNAIRE/index.tsresetButton: true— the worked examplepackages/instrument-guidelines/AGENTS.mdtesting/+ two test filesThe one subtlety worth reviewing
FormContentpreviously passedpreventResetValuesOnResetunconditionally. libui's reset is: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:
The invariant: a reset button renders if and only if
preventResetValuesOnResetis off. Never abutton 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 isresetBtnand is not ours to rename. The twomeet on one adapter line in
FormContent, which is the only coupling — the component enumeratesprops and never spreads, so nothing leaks through implicitly.
This cannot drift silently:
FormPropsindexes only[key: `data-${string}`], so passing thewrong name is an excess-property error. Verified deliberately:
If libui ever renames its prop,
tscfails on that one line rather than the button quietlydisappearing.
Behaviour
resetButtonunsetresetButton: trueThe 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 formunmounts 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_QUESTIONNAIREis the only instrument that opts in. It is the longestform in the library — roughly twenty fields — which is where starting over beats clearing by hand.
Testing
pnpm lintpnpm testpnpm test:e2e@smoke)New coverage:
runtime-core, checked bytscinlint) — pinsresetButtonasboolean | undefinedand confirms it is optional, so instruments predating it still satisfy thetype.
packages/schemas) — that$FormInstrumentpreservesresetButtonrather thanstripping it, that omitting it still parses, and that a non-boolean is rejected.
testing/src/specs/form-reset.spec.ts, 3 tests) — the button clears a filled form; aninstrument 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:
$$FormInstrument→ 2 schema tests fail; restoring it → 8 pass.preventResetValuesOnResetto unconditional → the e2e fails withExpected: "" / Received: "3"— button rendered, field unchanged.The schema test matters more than it looks: Zod strips undeclared keys,
apps/webvalidates only indevelopment (
validate: import.meta.env.DEV) and the playground always does. Omitting that one linewould have dropped the flag in exactly the places an author tests their instrument, while leaving it
working in production.
Reviewer notes
runtime-core(lib/+dist/) andruntime/v1/dist. Instruments import through/runtime/v1/..., sotscfails on a fresh cloneuntil
pnpm buildruns. Nothing to commit.packages/instrument-guidelinesis a published npm package whosebininstalls it into externalinstrument repos as their
AGENTS.md. Its version tracks the root, andincrement-version.shrewrites every path, so this documentation ships with the next ordinary release — no separate
action.
instrument-completion.spec.tsalready drives the demographics instrument, so the existing suiteexercises the changed instrument. It passes.
Out of scope
#1517
While tracing
preventResetValuesOnResetI found and reproduced a pre-existing state-bleed inskipProgressseries: the form is not remounted between items, so answers carry forward. Filedseparately as #. It is unrelated to this feature and its fix belongs in
SeriesInstrumentRendereror
useInterpretedInstrument, neither of which this PR touches.One interaction is worth knowing while reviewing: because this PR makes
preventResetValuesOnResetconditional, an instrument declaring
resetButton: truebecomes incidentally immune to that bug —reset()now empties its form after each submit. That is a side effect, not a fix, and # saysso explicitly.
Also out of scope: any confirmation dialog on Reset, which would require an upstream libui change.
Closes issue #1391