fix(workflow,logic): auto-save on card switch discards edits - #9849
Conversation
Editing a step or logic block that already exists, then clicking another card, silently discarded the edits. No error was shown. The auto-save effect decides whether to save by reading formState.isDirty. RHF's formState is a Proxy that only starts tracking a flag once something reads it during render — that read is the subscription. Both components read isDirty only inside the effect, which runs after render, so nothing ever subscribed and the value stayed at its initial false. The effect then took the "nothing changed" branch and switched without saving. New blocks were unaffected because isCreatingState short-circuits the check before it reaches isDirty, which is why this went unnoticed: the natural thing to test is creating a step and clicking away, and that works. Reading isDirty during render subscribes properly, so it reflects real edits by the time the effect runs. The dependency array stays [pendingSwitchTo]: the edit re-renders once subscribed, and the card click re-renders again, so the effect always closes over a current value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes a React Hook Form (RHF) subscription issue where edits to existing step/logic blocks were lost when switching cards because formState.isDirty was only read inside an effect (post-render), so RHF never subscribed to updates.
Changes:
- Subscribe to
formState.isDirtyduring render in step and logic editors. - Use the render-subscribed
isDirtyvalue in the auto-save-on-switch branch decision.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/EditStepBlock/EditStepBlock.tsx | Reads isDirty during render and uses it to decide whether to auto-save on card switch. |
| apps/frontend/src/features/admin-form/create/logic/components/LogicContent/EditLogicBlock/EditLogicBlock.tsx | Mirrors the same render-time isDirty subscription fix for logic blocks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Read during render, not inside the effect below: RHF's formState is a Proxy | ||
| // that only tracks a flag once something reads it while rendering. Read only | ||
| // from the effect it stays false, and edits were discarded on switch. | ||
| const { isDirty } = formMethods.formState |
| // existing step that wasn't touched can switch directly without a | ||
| // redundant save. | ||
| if (!isCreatingState && !formMethods.formState.isDirty) { | ||
| if (!isCreatingState && !isDirty) { |
| // Read during render, not inside the effect below: RHF's formState is a Proxy | ||
| // that only tracks a flag once something reads it while rendering. Read only | ||
| // from the effect it stays false, and edits were discarded on switch. | ||
| const { isDirty } = formMethods.formState |
| // An existing block that wasn't touched can switch directly without a | ||
| // redundant save. | ||
| if (!isCreatingState && !formMethods.formState.isDirty) { | ||
| if (!isCreatingState && !isDirty) { |
|
|
||
| // Read during render, not inside the effect below: RHF's formState is a Proxy | ||
| // that only tracks a flag once something reads it while rendering. Read only | ||
| // from the effect it stays false, and edits were discarded on switch. |
The old phrasing garden-paths on "Read only from the effect". Same explanation, split so the conditional clause reads as one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Re the Copilot review: its own summary notes the agentic review timed out, so it only saw the diff hunks. The four Lint on both changed files is clean: 0 errors, 1 pre-existing Worth noting the disable is load-bearing rather than incidental: the effect fires only on The grammar comment was fair — "Read only from the effect" garden-paths. Reworded in 6402e6c. |
Problem
Editing a step or logic block that already exists, then clicking another card, silently discards the edits. No error is shown. Found while rebasing an approval-reframe branch onto the click-to-edit work.
Not behind a flag:
editable-cards-mrf-logicwas dropped in8caec073a, andInactiveStepBlock'sonClickis unconditional. Limited so far only by recency — #9781 and #9786 merged today.Cause
The auto-save effect decides whether to save by reading
formState.isDirty. RHF'sformStateis a Proxy that only starts tracking a flag once something reads it during render — that read is the subscription. Both components readisDirtyonly inside the effect, which runs after render, so nothing ever subscribed and the value stayed at its initialfalse. The effect took the "nothing changed" branch and switched without saving.New blocks were unaffected because
isCreatingStateshort-circuits before reachingisDirty. That's why this went unnoticed: the natural thing to test is creating a step and clicking away, and that path works.Verified empirically with a throwaway harness reproducing the component shape (parent
useForm, children destructuringformState: { errors }, aController-driven edit, an effect keyed on a pending-switch prop):isDirtyseen by effectfalsevalue === 'edited'truevalue === 'edited'The right-hand column is the control, proving the edit registered in both arms.
Solution
Read
isDirtyduring render so RHF subscribes, and use that variable in the effect. Two lines, one per component.Dependency array stays
[pendingSwitchTo]: the edit re-renders once subscribed, and the card click re-renders again, so the effect always closes over a current value. AddingisDirtyto the deps would re-run the effect mid-switch on every keystroke, leaning entirely onhasSubmittedForPendingSwitchto prevent double submits.Tests
Notes
composeStorieshits theworkflow_typedouble-registration quirk the stories file already documents, and grew several times larger than the two-line fix. Left as the manual TCs above.🤖 Generated with Claude Code