[HOLD for payment 2024-01-18] [ON HOLD for #25397][$500] No required field validation on no value select on new dropdowns like state (validation works on older dropdown)

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.3.89.5
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1698055983660749

Action Performed:

1. Open the app
2. Open settings->workspaces->any workspace->bank account->connect manually
3. Fill in details in step 1 and continue
4. Open company type dropdown (older style dropdown), close without any value selection and focus on different field
5. Observe that required field validation is triggered
6. Try similarly on any other text fields in step 2 and observe that on focus out, required field validation will be triggered
7. Open state field and go back without selection (new dropdown)
8. Observe that no validation is triggered
9. Click on Save & continue and observe that now state field will show required field validation

Expected Result:

App should trigger required field validation when we focus / open any field and focus away

Actual Result:

App does not trigger required field validation when we open state fields in workspace connect bank account or any such new dropdown field throughout the app, close the field and focus on another field

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Android: Native

Android.native.no.required.validation.on.state.mp4

Android: mWeb Chrome

Android.chrome.no.required.validation.on.state.mp4

iOS: Native

ios.native.no.required.validation.mov

iOS: mWeb Safari

ios.safari.no.required.validation.state.mov

MacOS: Chrome / Safari

mac.chrome.no.required.validation.mov

windows.chrome.focus.back.does.not.trigger.validation.mp4

MacOS: Desktop

mac.desktop.no.required.validation.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0131d5a852872ad378
• Upwork Job ID: 1716523176886263808
• Last Price Increase: 2024-01-01
• Automatic offers:
• fedirjh | Reviewer | 28076905
• DylanDylann | Contributor | 28076907

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0131d5a852872ad378

[melvin-bot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)

[abdel-h66]

Proposal

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

Touched fields in the no not show the validation error

What is the root cause of that problem?

This type of fields, in this MenuItem with a modal, does not follow the conventions of the normal input where we have blur and focus events handled by default. We don't have a way at the moment to set it to touched. And trigger the validation.

In src/components/Form.js the onblur is defined to run the following

 setTouchedInput(inputID);

if (props.shouldValidateOnBlur) {
    onValidate(inputValues, !hasServerError);
}

This event will never occur

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

And then should explicitly call the onBlur to trigger the validation during the closing of the modal of the <StatePicker />.
By updating.

    const hidePickerModal = () => {
        onBlur();
        setIsPickerVisible(false);
    };

Calling the onTouched is not necessary, since the onBlur does that for us. But we can call it during the opening of the modal if we want to have a more consistent behavior.

 const showPickerModal = () => {
        setIsPickerVisible(true);
        onTouched()
    };

Monosnap.screencast.2023-10-23.21-17-42.mp4

What alternative solutions did you explore? (Optional)

N/A

[DylanDylann]

Proposal

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

• No required field validation on no value select on new dropdowns like state (validation works on older dropdown)

What is the root cause of that problem?

• With the StatePicker component, blurring from it does not trigger the onBlur callback function that we have handled in Form component like others input
  

  App/src/components/Form.js

  Lines 349 to 356 in 1c85ebe

  	onBlur: (event) => {
  	// Only run validation when user proactively blurs the input.
  	if (Visibility.isVisible() && Visibility.hasFocus()) {
  	// We delay the validation in order to prevent Checkbox loss of focus when
  	// the user are focusing a TextInput and proceeds to toggle a CheckBox in
  	// web and mobile web platforms.
  	setTimeout(() => {
  	setTouchedInput(inputID);

• Because the setTouchedInput(inputID) is not called, so it will not display error.

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

• We need to call setTouchedInput(inputID) manually with this type of input by:

1. In Form component, expose the formRef with method setTouchedInput like:

useImperativeHandle(ref, () => ({
    setTouchedInput
    },
  }));

2. In StatePicker component, we import the above ref, then once we click on MenuItem, call the setTouchedInput:

• Also we should apply this logic to others similar inputs

What alternative solutions did you explore? (Optional)

• Now, the Form Refactor is done. So we just need to update StatePicker to

+ function StatePicker({value, errorText, onInputChange, forwardedRef, label, ...rest}) {
    const hidePickerModal = () => {
+      rest.onBlur && rest.onBlur();
        setIsPickerVisible(false);
    };
    return (
        <View>
            <MenuItemWithTopDescription
                ...
                onPress={() => {
+                  rest.onPress && rest.onPress();
                    showPickerModal();
                }}
            />
            ...
        </View>
    );
}

Result

Screencast.from.22-11-2023.10.12.22.webm

[johncschuster]

What are your thoughts on the above proposal, @fedirjh?

[melvin-bot]

@johncschuster, @fedirjh Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[fedirjh]

@abdel-h66 @DylanDylann I have some concerns about the proposals because we are in the process of updating the Form.js component and refactoring old pages that use Form.js. Tracking issue is here:

• [HOLD for payment 2024-01-18] [HOLD for payment 2023-10-23] Refactor Form to a custom hook #25397 (comment)

If this bug persists after the refactor, we might need to consider a different solution than the one currently proposed.

I believe the best course of action is to hold this until the pages are updated to use the new version of the Form? Curious about your thoughts @johncschuster.

[johncschuster]

I trust your judgment, @fedirjh! Let's put this on hold for now and come back to it after the refactor.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[fedirjh]

Form refactor is still in progress.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.24-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: No required field validation state picker #33926

If no regressions arise, payment will be issued on 2024-01-18. 🎊

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment automatic offer (Reviewer)
• @DylanDylann requires payment automatic offer (Contributor)
• @dhanashree-sawant requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@johncschuster / @muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[muttmuure]

@fedirjh @DylanDylann paid

[muttmuure]

$50 contract extended to @dhanashree-sawant

[fedirjh]

BugZero Checklist:

• Link to the PR: 17548 Personal details push to page #21049
• Link to comment: 17548 Personal details push to page #21049 (comment)
• Link to discussion: N/A This should have been caught during the review.
• Determine if we should create a regression test for this bug: Yes

1. Open the app
2. Open settings -> workspaces -> any workspace -> bank account -> connect manually
3. Fill in details in step 1 and continue
4. Open state field and go back without selection
5. Verify that validation is triggered and an error message is displayed

[johncschuster]

@muttmuure it looks like I tagged you in when I went OOO for the holidays. I'm going to remove your assignment and take this off your plate. Thanks for pushing this forward!

[melvin-bot]

@nkuoch, @johncschuster, @fedirjh, @DylanDylann Huh... This is 4 days overdue. Who can take care of this?

[johncschuster]

I've issued payment to @dhanashree-sawant and will work on the regression test steps issue today.