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

[CMDCT-3032] Fix text fields not saving in nested choices if not last field in choicelist #11514

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

ntsummers1
Copy link
Contributor

@ntsummers1 ntsummers1 commented Nov 8, 2023

Description

Important Note!

While reviewing this, you'll see my videos with the Before and After. The before captures a state where the user is tabbing off the checkbox's child to another choice, and then moving to a different page before coming back. THIS IS NOT FIXED. I've tried friends. I really have. We need state management something fierce. That being said, what is fixed are all of the scenarios in the after column. Which are a heck of a lot better then before. Go ahead and repeat those on main and you'll see the difference. Nested fields were getting cleared for no reason when still on the same page. This solves that. For going to a different page though and coming back? I can't do it with out some real edge case spagehetti that would require so many tests. I tried writing those tests too. You don't want those tests and all those branches. It introduces too much complexity and variables. Revisit this when we have more time and zustand can be properly enacted.


If a user was to tab to a different checked answer when filling out a child choice, it would clear that child choice. This fixes it so other fields that were filled out won't be changed!

This really just removes code that didn't need to be there any longer and fixes the value being sent to what the form has it stored as.

Before

281254377-86266114-a6bf-400e-aec8-8eeb56927dad.mov

After

Screen.Recording.2023-11-14.at.11.06.35.AM.mov
Screen.Recording.2023-11-07.at.11.28.56.PM.mov
Screen.Recording.2023-11-07.at.11.57.18.PM.mov
Screen.Recording.2023-11-08.at.12.02.42.AM.mov

Related ticket(s)

CMDCT-3032


How to test

Create a Report
Navigate to C1.III.1 Uses of encounter data
Check Other Specify, fill out the text box
tab off to the next choice
click anywhere that is not in the choicelist (like to another page)
Come back and see its still there!

Go into B.X.7a Changes in Provider circumstances: Monitoring Plans
Play around and make sure its sending and filling out correctly as you clear and check things

Open MLR
Checkout the overlay section too!


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary

@ntsummers1 ntsummers1 changed the title [CMDCT-3032] - Fix text fields not saving in nested choices if not last field in choicelist [CMDCT-3032] Fix text fields not saving in nested choices if not last field in choicelist Nov 8, 2023
@ntsummers1 ntsummers1 added the ready for review Ready for all the reviews! label Nov 8, 2023
Copy link
Contributor

@gmrabian gmrabian left a comment

Choose a reason for hiding this comment

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

fix did not work for me locally 😱

Copy link

codeclimate bot commented Nov 14, 2023

Code Climate has analyzed commit a3b1f0f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 66.6% (90% is the threshold).

This pull request will bring the total coverage in the repository to 95.7% (-0.2% change).

View more on Code Climate.

@ntsummers1 ntsummers1 force-pushed the nested-child-fields-fix branch from a3b1f0f to 25eb6e1 Compare November 14, 2023 17:30
@ntsummers1 ntsummers1 requested a review from gmrabian November 14, 2023 17:37
@BearHanded BearHanded merged commit 501a5dd into main Nov 16, 2023
@BearHanded BearHanded deleted the nested-child-fields-fix branch November 16, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Ready for all the reviews!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants