App: [HOLD for payment 2024-01-26] [$500] Wallet - `Fix the errors` button is non-functional in Additional details

BugZero Checklist:

• Link to the PR: https://github.com/Expensify/App/pull/32196
• Link to comment: Author already aware of it and has fixed it.
• Link to discussion: N/A
• Determine if we should create a regression test for this bug:

1. Open Expensify App
2. Login with expensifail account
3. Go to Settings > Wallet > Enable Wallet
4. Go through plaid verification flow
5. Follow the flow until Idology questions
6. Don't fill anything and press Save & continue
7. Verify that an Error appears without fix the errors alert

Oh Melvin. Was trying to get it to assign @kowczarz as the contributor, but doesn’t seem to want to do it. Not sure if that is important since @kowczarz is part of an expert agency. Either way, @kowczarz feel free to raise the PR and we can figure out how to wrangle Melvin if need be.

Thanks @kowczarz that looks good.

Let’s proceed with this proposal

🎀 👀 🎀 C+ reviewed

@fedirjh currently the isAlertVisible is calculated the following way:

isAlertVisible={_.size(errors) > 0 || Boolean(errorMessage) || !_.isEmpty(formState.errorFields)}

After update I would like to calculate it that way:

isAlertVisible={((_.size(errors) > 0 || !_.isEmpty(formState.errorFields)) && !shouldHideFixErrorsAlert) || Boolean(errorMessage)}

So If we will want to display something more specific than fix the errors, it will show the alert.

Proposal

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

The app displays a redundant alert with fix the errors button even tough there is no way to focus proper input.

What is the root cause of that problem?

We have no possibility to decide wether or not the fix the errors alert should be displayed.

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

In the FormProvider we can add a property shouldHideFixErrorsAlert that will be passed down the tree to FormWrapper and there we make calculation of isAlertVisible depending on it, so we can easily hide the alert when needed.

Sounds good, in that case I will prepare a new proposal, where I will add a prop that disables the redundant alert.

@kowczarz gotcha - thanks for showing me that!

Proposal

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

The app displays fix the errors button even tough in case of radio button there is no way to focus proper input.

What is the root cause of that problem?

Currently we have no option to add custom message, or disable the button in error alert.

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

We can add a prop to FormAlertWrapper and pass it down the component tree from FormProvider.

Just for the context there is a thread we discussed the new layout of the errors https://github.com/Expensify/App/pull/31401#discussion_r1396510915

As @dubielzyk-expensify suggested we can add a prop that will disable the fix the errors button it will look that way:

I agree that one error message seems better in that case.

Thanks for that!

The problem is that clicking ‘Fix the errors’ in this case doesn’t trigger anything (it feels broken)

I’d ideally think we should only have 1 error message here, but if we have to have two then let’s just remove the link in “fix the errors” as like you say it doesn’t do anything. The errors are self-explanatory by themselves anyways.

Sorry for the late reply. In this instance, I think we shouldn’t select anything so if they click next immediately, it’s fine to throw an error. We’d want them to make an explicit choice, hence why a default selection is no bueno 😃

Hey @fedirjh! that was intentional UI change, here you can see the discusson https://github.com/Expensify/App/pull/31401#discussion_r1396510915

@mountiny good point, we didn’t consider what should be the behaviour of the fix button. As soon as we will figure it out I can prepare a PR with update in the FORMS.md guidelines and an optional behaviour change of the button.