Skip to content
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

add formValues and formErrors to /current-name #483

Merged
merged 1 commit into from
Mar 25, 2025
Merged

add formValues and formErrors to /current-name #483

merged 1 commit into from
Mar 25, 2025

Conversation

Fbasham
Copy link
Member

@Fbasham Fbasham commented Mar 25, 2025

Summary

EDIT: I now realize there are more routes that need the implementation...I wonder what other TS errors are making this a pain.

I think we can all agree TS is a pain sometimes. Originally we had partially agreed to remove the persistent error messages/partial validated default form values on language toggle. Instead of completely removing it, why not instead make it work for the route that didn't have this feature? If it's still a flop, let's remove it completely.

Types of changes

What types of changes does this PR introduce?
(check all that apply by placing an x in the relevant boxes)

  • 🛠️ refactoring -- non-breaking change that improves code readability or structure
  • 🐛 bugfix -- non-breaking change that fixes an issue
  • new feature -- non-breaking change that adds functionality
  • 💥 breaking change -- change that causes existing functionality to no longer work as expected
  • 📚 documentation -- changes to documentation only
  • ⚙️ build or tooling -- ex: CI/CD, dependency upgrades

Checklist

Before submitting this PR, ensure that you have completed the following. You can fill these out now, or after creating the PR.
(check all that apply by placing an x in the relevant boxes)

  • code has been linted and formatted locally
  • added or updated tests to verify the changes
  • added adequate logging for new or updated functionality
  • ensured metrics and/or tracing are in place for monitoring and debugging
  • documentation has been updated to reflect the changes (if applicable)
  • linked this PR to a related issue (if applicable)
Linting and formatting
npm run lint:check
npm run format:check
Unit and e2e tests
npm run test
npm run test:e2e

Additional Notes

If this PR introduces significant changes, explain your reasoning and provide any necessary context here. Feel free to include diagrams, screenshots, or alternative approaches you considered.

Screenshots (if applicable)

Provide screenshots or screen-recordings to help reviewers understand the visual impact of your changes, if relevant.

@Fbasham Fbasham enabled auto-merge (squash) March 25, 2025 12:45
@gregory-j-baker
Copy link
Contributor

Adding this to all the routes might be okay with the current implementation (where we store the validated data in the xstate machine).

When I originally tried this I first started by modifying what data was being stored in the machine, and that change quickly snowballed into typescript hell when I tried updating the error stuff.

@Fbasham
Copy link
Member Author

Fbasham commented Mar 25, 2025

Adding this to all the routes might be okay with the current implementation (where we store the validated data in the xstate machine).

When I originally tried this I first started by modifying what data was being stored in the machine, and that change quickly snowballed into typescript hell when I tried updating the error stuff.

Yeah. When I first opened this PR I though this was the only route missing the implementation after looking at the state machine. Didn't look closely into each route though. If I have time I'll look into adding the rest.

@Fbasham Fbasham merged commit 8e4bc9a into main Mar 25, 2025
6 checks passed
@Fbasham Fbasham deleted the fb-name-err branch March 25, 2025 15:59
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