[HOLD for payment 2023-11-22] [$500] Inconsistency in the request money preview text size

[m-natarajan]

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.3.90-1
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: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1698087869176119

Action Performed:

1. Open a chat
2. Go to the request money
3. Enter a specific amount and send the request money
4. Click on the requested money
5. Check the preview text size

Expected Result:

The request money preview text size should be the same in both the chat and the review sections

Actual Result:

There is an inconsistency in the request money preview text size

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

20231024000522.mp4

Android: mWeb Chrome

20231024000545.mp4

iOS: Native

Screen.Recording.2023-10-23.at.11.58.16.PM.mov

RPReplay-Final1698099846.MP4

iOS: mWeb Safari

Screen.Recording.2023-10-24.at.12.06.11.AM.mov

MacOS: Chrome / Safari

2023-10-24.03-27-22.mp4

MacOS: Desktop

Screen.Recording.2023-10-24.at.3.29.03.AM.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a260f6acb4caa392
• Upwork Job ID: 1716855328239075328
• Last Price Increase: 2023-10-31
• Automatic offers:
• fedirjh | Reviewer | 27543446
• dukenv0307 | Contributor | 27543447

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a260f6acb4caa392

[melvin-bot]

Triggered auto assignment to @lschurr (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)

[wlegolas]

Proposal

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

Inconsistency in the request money preview text size

What is the root cause of that problem?

In the MoneyRequestPreview component the information has the styles applied by "moneyRequestPreviewAmount" styles, this style doesn't have the font-size and line-height values has the text in the ReportPreview has.

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

We should change the text in the MoneyRequestPreview component to use the style "textHeadline" instead of "moneyRequestPreviewAmount", and remove the fontSize that is calculate for small devices.

What alternative solutions did you explore? (Optional)

N/A

POC

poc-issue-30275.mov

[dukenv0307]

Proposal

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

Inconsistency in the request money preview text size

What is the root cause of that problem?

The amount text of ReportPreview has style styles.textHeadline which has the constant fontSize style

App/src/components/ReportActionItem/ReportPreview.js

Line 221 in b72dfd4

<Text style={styles.textHeadline}>{getDisplayAmount()}</Text>

But in MoneyRequestPreivew the amount text has the fontSize based on the screen width

App/src/components/ReportActionItem/MoneyRequestPreview.js

Line 279 in b72dfd4

StyleUtils.getAmountFontSizeAndLineHeight(variables.fontSizeXLarge, variables.lineHeightXXLarge, isSmallScreenWidth, windowWidth),

App/src/styles/StyleUtils.ts

Line 1263 in b72dfd4

if (isSmallScreenWidth) {

That makes the inconsistency font size of the amount text in small screen width

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

In ReportPreview we should get the fontSize of amount text in the same way we do in MoneyRequestPreivew

App/src/components/ReportActionItem/MoneyRequestPreview.js

Line 279 in b72dfd4

StyleUtils.getAmountFontSizeAndLineHeight(variables.fontSizeXLarge, variables.lineHeightXXLarge, isSmallScreenWidth, windowWidth),

What alternative solutions did you explore? (Optional)

We can update the amount text font size in MoneyRequestPreivew to the constant fontSize which is the same with
ReportPreview by removing this responsive fontSize and use styles.textHeadline

App/src/components/ReportActionItem/MoneyRequestPreview.js

Line 279 in b72dfd4

StyleUtils.getAmountFontSizeAndLineHeight(variables.fontSizeXLarge, variables.lineHeightXXLarge, isSmallScreenWidth, windowWidth),

We will only need to update the fontSize adjustable for SplitBill which will be based on the length of the amount and the participant of the split bill and the screen is small screen with. We will update this on getAmountFontSizeAndLineHeight function and the font size can be done in the PR.

[yh-0218]

Proposal

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

Inconsistency in the request money preview text size

What is the root cause of that problem?

We have different style between two.

App/src/components/ReportActionItem/ReportPreview.js

Line 221 in f8fc21d

<Text style={styles.textHeadline}>{getDisplayAmount()}</Text>

App/src/components/ReportActionItem/MoneyRequestPreview.js

Lines 277 to 279 in f8fc21d

	style={[
	styles.moneyRequestPreviewAmount,
	StyleUtils.getAmountFontSizeAndLineHeight(variables.fontSizeXLarge, variables.lineHeightXXLarge, isSmallScreenWidth, windowWidth),

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

We need update ReportPreview like MoneyRequestPreview

App/src/components/ReportActionItem/ReportPreview.js

Line 221 in f8fc21d

<Text style={styles.textHeadline}>{getDisplayAmount()}</Text>

<Text
         style={[
                  styles.moneyRequestPreviewAmount,
                  StyleUtils.getAmountFontSizeAndLineHeight(variables.fontSizeXLarge, variables.lineHeightXXLarge, isSmallScreenWidth, windowWidth)
         ]}
>
      {getDisplayAmount()}
</Text>

What alternative solutions did you explore? (Optional)

[fedirjh]

The request money preview text size should be the same in both the chat and the review sections

@lschurr I am not sure about the expected result. I think there is a difference between the two previews :

1. The report preview inside the parent report is an IOU/Expense preview.
2. The preview inside the IOU/Expense report is a money request preview.

So I think having a different style for each preview makes sense. cc @Expensify/design Should we unify the styles for both previews?

[shawnborton]

I agree, these should be consistent. @JmillsExpensify @trjExpensify is this a regression from somewhere?

[fedirjh]

@shawnborton What should be expected? Here is the difference between the two styles:

• The style of the money request depends on the screen width.

• The IOU preview looks to have a fixed style.

• IOU Preview style

• Money request preview

Screen.Recording.2023-10-26.at.4.59.42.PM.mov

[shawnborton]

I think the preview card should have a consistent max-width (so they all look the same width on desktop), but it's okay if it goes 100% width on smaller screens

[fedirjh]

@shawnborton All cards have the same width, but the issue lies in the text size on the cards.

CleanShot.2023-10-26.at.18.05.31.mp4

[shawnborton]

Interesting, I don't know how we ended up with a responsive text size like that but I think we should revert that to stay static. I guess the idea might have been to make the text dynamically smaller in case there were a lot of avatars? @youssef-lr any ideas here?

[youssef-lr]

@shawnborton looks like this change came from this PR to fix the large amounts being cut off in the split preview..

[melvin-bot]

@lschurr, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick!

[fedirjh]

I guess the idea might have been to make the text dynamically smaller in case there were a lot of avatars

@shawnborton Coming from #24321 (comment) (Related Slack), The expected is :

When entering 8+ numerals (ex: 12,345,678, 123,456,789, etc), we should expect the text to dynamically decrease in size to allow the characters to fit.

Should we create a different style for the split preview, adjusting the text size based on responsiveness, while keeping the text size static for the other money previews?

[trjExpensify]

I think they should all be consistent, personally.

[dubielzyk-expensify]

Covering for Shawn while he's OOO: Agree that they should all be consistent. If the numbers can't fit, we should shrink to make them fit (but also this is quite rare), but otherwise keep the same static size.

[fedirjh]

@wlegolas @dukenv0307 @yh-0218 Could you please update your proposals to follow the expected behavior in #30275 (comment) ?

[dukenv0307]

@fedirjh So we will make both text has the static font-size right?

[dukenv0307]

My alternative solution have already covered this case #30275 (comment)

[lschurr]

Alrighty, I've assigned @dukenv0307

[dukenv0307]

@fedirjh The PR is ready for review.

[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.3.99-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix inconsistency font size of amount text between report preview and split bill #30960

If no regressions arise, payment will be issued on 2023-11-22. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @fedirjh requires payment offer (Reviewer)
• @dukenv0307 requires payment offer (Contributor)
• @ayazhussain79 requires payment

[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.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[lschurr]

@fedirjh could you work through the checklist for this one?

[fedirjh]

BugZero Checklist:

• Link to the PR: [24321] Large amounts are no longer cut off on smaller widths. #26280
• Link to comment: [24321] Large amounts are no longer cut off on smaller widths. #26280 (comment)
• Link to discussion: N/A Nothing to add to the checklist.
• Determine if we should create a regression test for this bug: Let's have a regression text for the amount text size

---

Regression Test Proposal

1. Open the app and navigate to the report with a user.
2. Create a money request with same user.
3. Create a split request with same user.
4. Verify that you see both the money request preview and the split request preview.
5. Verify that the font size of the amount text is consistent between the money request preview and the split bill preview.

• Do we agree 👍 or 👎

[lschurr]

Payment summary:

• Bug reporter: @ayazhussain79 $50
• Contributor: @dukenv0307 $500
• Contributor+: @fedirjh $500

[lschurr]

@ayazalavi could you apply to the job in Upwork? https://www.upwork.com/jobs/~01a260f6acb4caa392

[ayazhussain79]

@lschurr You've mentioned the wrong person. I've applied on Upwork. Thank you.

[lschurr]

Sorry about that! GH auto-populated. Thanks!

[lschurr]

Updated the payment summary with the correct GH handle.

[ayazhussain79]

Offer accept, Thank you

[lschurr]

Sorry about the delay! These are all paid now.