Skip to content

Fix form skip logic #349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 9, 2025
Merged

Fix form skip logic #349

merged 8 commits into from
Apr 9, 2025

Conversation

buckhalt
Copy link
Member

@buckhalt buckhalt commented Apr 7, 2025

Bug fix: Stale data from form submission impacting skip logic

Issue:

Interview navigation functions (moveForward and moveBackward) were using stale values of nextValidStageIndex and previousValidStageIndex due to closures. When these indexes were updated during form submission in the beforeNext handler, the navigation functions weren't receiving the updated values. This resulted in behavior where if form data impacted skip logic, the skip logic wasn't working correctly.

Solution:

  • Implemented using useRef to store index values as refs
  • Added useEffect hooks to update these refs whenever indexes change
  • Modified moveForward and moveBackward to use current ref values for updating session progress and stage instead of closure-captured variables

Other changes:

This investigation also revealed a circular deps issue and duplicate selectors (two duplicate versions of getStageIndex). Fixed this by moving some selectors (getActiveSession, getStageIndex) to a shared file and importing from there.

Copy link

vercel bot commented Apr 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fresco-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 9, 2025 8:47am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
barf ⬜️ Ignored (Inspect) Visit Preview Apr 9, 2025 8:47am

@jthrilly jthrilly changed the base branch from main to next April 8, 2025 08:15
@jthrilly jthrilly changed the base branch from next to main April 8, 2025 08:15
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Copy link
Member

@jthrilly jthrilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Only thing for the future would be to remove some of the unnecessary comments, but nbd.

@jthrilly jthrilly merged commit 12f1ca9 into main Apr 9, 2025
6 checks passed
buckhalt added a commit that referenced this pull request Apr 9, 2025
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.

2 participants