-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$250] Bank account - App does not trigger required field validation when no state is selected #54572
Comments
Triggered auto assignment to @anmurali ( |
Edited by proposal-police: This proposal was edited at 2024-12-25 18:59:32 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Bank account - App does not trigger required field validation when no state is selected What is the root cause of that problem?We run validate onBlur but we are not bluring on modal close here App/src/components/PushRowWithModal/index.tsx Lines 52 to 57 in 12e0941
What changes do you think we should make in order to solve the problem?We can apply similar fix as we did in here by blurring on modal close
we can also add a new prop of What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
Add this in PushRowWithModal.
App/src/components/PushRowWithModal/index.tsx Lines 53 to 55 in 12e0941
Add the following line at line:
Introduce a new flag parameter, such as What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.App does not trigger required field validation when no state is selected What is the root cause of that problem?We are missing the addition of onBlur to validate the form when the modal is closed or when navigating back to trigger form validation
onBlur to trigger onValidate: App/src/components/Form/FormProvider.tsx Lines 349 to 374 in 08f8314
What changes do you think we should make in order to solve the problem?To resolve this issue, we applied the previous solution. In this solution, 1 Create new state const [valueChanged, setValueChanged] = useState(false);
Update to: const handleModalClose = () => {
if (!value || valueChanged) { // add this line
onBlur?.(); // add this line
} // add this line
setIsModalVisible(false);
};
Update to: const handleOptionChange = (optionValue: string) => {
onInputChange(optionValue);
setValueChanged(optionValue !== value); // add this line
if (stateInputIDToReset) {
onInputChange('', stateInputIDToReset);
}
}; POCScreen.Recording.2024-12-31.at.00.02.29.movTest branchWhat specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
@anmurali Eep! 4 days overdue now. Issues have feelings too... |
Job added to Upwork: https://www.upwork.com/jobs/~021874219392873534463 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale ( |
Note for reviewer: The solution should cover the comment below: if not, the regression will be encountered (The error message is displayed for a while after user selecting any option). See that comment in PR |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@akinwale have you reviewed the proposals here? Let's pick one asap so we can move forward. |
@anmurali I'll review the proposals today. |
Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.0.78-2
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
App should trigger required field validation when we focus / open state field and focus away (as in PR #33926)
Actual Result:
App does not trigger required field validation when we focus / open state field and focus away.
Workaround:
Unknown
Platforms:
Screenshots/Videos
bug.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @akinwaleThe text was updated successfully, but these errors were encountered: