The "I accept the Expensify Terms of Service" checkbox not checked when retuning from step 2 #20871

Activity

1. kbecciv

   added

   DailyKSv2KSv2

   BugSomething is broken. Auto assigns a BugZero manager.Something is broken. Auto assigns a BugZero manager.

   on Jun 15, 2023
2. melvin-bot commented on Jun 15, 2023

   

   melvin-botBot

   on Jun 15, 2023

   More actions

   Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
3. melvin-bot

   assigned

   slafortune

   on Jun 15, 2023
4. melvin-bot commented on Jun 15, 2023

   

   melvin-botBot

   on Jun 15, 2023

   More actions

   Bug0 Triage Checklist (Main S/O)

   • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
   • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
     • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
   • This bug is reproducible using the reproduction steps in the OP. S/O
     • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
     • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
   • This issue is filled out as thoroughly and clearly as possible
     • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
   • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync
5. allroundexperts commented on Jun 15, 2023

   

   allroundexperts

   on Jun 15, 2023

   Contributor

   More actions

   Proposal

   Please re-state the problem that we are trying to solve in this issue.

   The checkbox values are not retained when returning back to the previous step.

   What is the root cause of that problem?

   The root cause of this issue is that we're using two props to determine if the checkbox is checked. We're using isChecked and value prop. The value prop is set by the Form here.
   For the case mentioned in this bug, the value is set as true by the Form but the isChecked prop is false. On the first mount, the state value is true initially but after this effect runs, the state value becomes false, thus causing the checkbox to be unchecked.

   What changes do you think we should make in order to solve the problem?

   The simplest way to fix this is to change defaultProp value of the value to be undefined instead of false. Then in the effect here, we can add the following:

   if (props.value  !==  undefined) return;
   

   What alternative solutions did you explore? (Optional)

   We can also remove isChecked prop altogether and just use value everywhere. Likewise, we can also change the Form component, such that for checkboxes, it sets isChecked prop instead of the value prop here.
6. hungvu193 commented on Jun 16, 2023

   

   hungvu193

   on Jun 16, 2023

   Last edited by hungvu193

   Contributor

   More actions

   Proposal

   Please re-state the problem that we are trying to solve in this issue.

   The "I accept the Expensify Terms of Service" checkbox not checked when retuning from step 2

   What is the root cause of that problem?

   I agree with Sibtain's proposal about the root cause of this issue is about the code inside CheckboxWithLabel:
   

   App/src/components/CheckboxWithLabel.js

   Lines 92 to 94 in bd0e843

   	useEffect(() => {
   	setIsChecked(props.isChecked);
   	}, [props.isChecked]);

   What changes do you think we should make in order to solve the problem?

   Since isChecked is a state, it won't re-calculated when the props.value change, we should do it dynamically
   

   App/src/components/CheckboxWithLabel.js

   Line 84 in bd0e843

   const [isChecked, setIsChecked] = useState(_.find([props.value, props.defaultValue, props.isChecked], (value) => _.isBoolean(value)));

   So our useEffect inside CheckboxWithLabel will look like this:

   // we should update the checkbox value when props.value is changed, because our isChecked state isn't re-calculated
       useEffect(() => {
           setIsChecked(props.isChecked || props.value);
       }, [props.isChecked,  props.value]);

   What alternative solutions did you explore? (Optional)

   N/A
7. bernhardoj commented on Jun 17, 2023

   

   bernhardoj

   on Jun 17, 2023

   Contributor

   More actions

   It's fixed in this PR #20091

   👍React with 👍2Reacted by Stevie LaFortune and Hans Vu
8. melvin-bot

   added

   Overdue

   on Jun 19, 2023
9. luacmartins

   self-assigned this

   on Jun 19, 2023
10. luacmartins commented on Jun 19, 2023

    

    luacmartins

    on Jun 19, 2023

    Contributor

    More actions

    I agree has already been fixed in my PR. Closing.
11. luacmartins

    closed this as completedon Jun 19, 2023
12. melvin-bot

    removed

    Overdue

    on Jun 19, 2023