Skip to content

Bank account label in Workspace > Workflows > Make or track payments is too small #40204

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

Closed
1 of 6 tasks
m-natarajan opened this issue Apr 13, 2024 · 6 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@m-natarajan
Copy link

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: 1.4.62-2
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: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712858899464959

Action Performed:

  1. Go to Workspace Settings > Workflows and enable the "Make or track payments" toggle

Expected Result:

When there is no bank account added, the label for "Bank account" should be 15px in size

Actual Result:

The label is too small and is 13px in size

Workaround:

unknonw

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

Add any screenshot/video evidence

CleanShot 2024-04-11 at 20 06 20@2x

Recording.5.mp4

View all open jobs on GitHub

@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 13, 2024
Copy link

melvin-bot bot commented Apr 13, 2024

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Krishna2323
Copy link
Contributor

Proposal

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

Bank account label in Workspace > Workflows > Make or track payments is too small

What is the root cause of that problem?

When hasVBA is true we are using textLabelSupportingNormal which has fontSizeLabel: getValueUsingPixelRatio(13, 19) for font size. The hasVBA is true because we even if we have reimburser login so we need to check for bankDisplayName also.

titleStyle={hasVBA ? styles.textLabelSupportingNormal : styles.textLabelSupportingEmptyValue}

const hasVBA = !!policy?.achAccount;

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

We should also check for bankDisplayName for applying styles.

titleStyle={hasVBA && bankDisplayName ? styles.textLabelSupportingNormal : styles.textLabelSupportingEmptyValue}

What alternative solutions did you explore? (Optional)

@allgandalf
Copy link
Contributor

Proposal

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

Incorrect font size for Connect Bank Account text.

What is the root cause of that problem?

Currently, when we click on connect bank account we set hasVBA to true as we set the field reimburser inside of ACHAccount to the current policy owner email, but if we navigate back without connecting the bank account, then even though the bank account is not connected, yet hasVBA would be set to true as we have set the value of reimbursed:

type ACHAccount = {
bankAccountID: number;
accountNumber: string;
routingNumber: string;
addressName: string;
bankName: string;
reimburser: string;
};

Now, this will set the style of the text to styles.textLabelSupportingNormal which has 13px as font size, but the text is still connect bank account and hence wrong style would be applied over here

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

We should add a extra boolean condition like we do when we check which one to display between Connect Bank Account and Bank Account in the same component we check the condition as follows for text, so we can apply the same condition for style as well:

title={
hasVBA && policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES
? translate('common.bankAccount')
: translate('workflowsPage.connectBankAccount')
}

Update :

<MenuItem
titleStyle={hasVBA ? styles.textLabelSupportingNormal : styles.textLabelSupportingEmptyValue}

To:

 <MenuItem
      titleStyle={ hasVBA && policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? styles.textLabelSupportingNormal : styles.textLabelSupportingEmptyValue}
                            

What alternative solutions did you explore? (Optional)

@ahmedGaber93
Copy link
Contributor

This issue will fix here #39947, it has the same root cause. The style condition is correct hasVBA ? 13 : 15 but the issue is the hasVBA declaration is not correct after disable and then enable the "Make or track payments" for the first time, and this will fix here #39947.

Tested in last main.

Screen.Recording.2024-04-13.at.9.26.42.PM.mov

@ikevin127
Copy link
Contributor

I can confirm that this issue will be fixed as part of the already open PR #40182 from issue:

Specifically the following lines of code:

  1. Line 89: Fixed hasVBA check.
  2. Line 90: Added shouldShowBankAccount variable.
  3. Line 191: Passed titleStyle correctly based on shouldShowBankAccount.

@slafortune
Copy link
Contributor

Thanks @ikevin127 , in that case, let's close this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

6 participants