[HOLD for payment 2023-09-27] [$250] Web – Split Bill – Difference in padding among amount and description.

[izarutskaya]

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

---

Action Performed:

1. Go to any chat and split a bill.
2. Notice the difference in padding among amount and description. Amount is relatively above then description.

Expected Result:

Padding should be consistent among amount and description.

Actual Result:

Padding is not consistent among amount and description.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.57-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

Notes/Photos/Videos: Any additional supporting documentation

Bug.3.mp4

Expensify/Expensify Issue URL:

Issue reported by: @usmantariq96

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692690646431509

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0137a2de3122624eed
• Upwork Job ID: 1699562237383294976
• Last Price Increase: 2023-09-06
• Automatic offers:
• namhihi237 | Contributor | 26568205

[namhihi237]

Proposal

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

Padding should be consistent among amount and description.

What is the root cause of that problem?

We using mt1 for the description, and in an amount for each, we missing for this.

App/src/components/ReportActionItem/MoneyRequestPreview.js

Line 281 in c029dc5

{!_.isEmpty(description) && <Text style={[styles.mt1, styles.colorMuted]}>{description}</Text>}

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

We should add mt1 here

 <Text style={[styles.textLabel, styles.colorMuted, styles.ml1, styles.mt1]}>

What alternative solutions did you explore? (Optional)

Or we can remove mt1 here and here and add mt1 here:

App/src/components/ReportActionItem/MoneyRequestPreview.js

Line 276 in c029dc5

<View style={[styles.flexRow]}>

[melvin-bot]

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

[s-alves10]

Proposal

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

Split bill preview has inconsistency in padding among amount and description

What is the root cause of that problem?

Description has style styles.mt1 but amount doesn't

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

I suggest to add styles.mt1 to the parent here

App/src/components/ReportActionItem/MoneyRequestPreview.js

Line 278 in 17025aa

<View style={[styles.flexRow]}>

and remove all mt1 in its children

What alternative solutions did you explore? (Optional)

[stephanieelliott]

I agree the padding is obviously off. I think it should be fixed but considering how minor this is, adjusting the price to $250.

[melvin-bot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[PiyushChandra17]

Proposal

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

Padding is not consistent among amount and description.

What is the root cause of that problem?

We are using styles.ml1 while styling the amount each text

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

• Instead of using styles.ml1, we should add styles.mt1 on styling the amount each text (just to be consistent on both the description and amount text)

• In MoneyRequestPreview.js, line number: 286, add styles.mt1

<Text style={[styles.textLabel, styles.colorMuted, styles.mt1]}>

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0137a2de3122624eed

[melvin-bot]

Current assignee @stephanieelliott is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[stephanieelliott]

Forgot to add External label, @robertKozik looks like we have a proposal #26210 (comment)

[munsuyog]

Proposal

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

Inconsistent padding between amount and description.

What is the root cause of that problem?

styles.ml1 is being used for styling.

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

In MoneyRequestPreview.js, add styles.mt1

<Text style={[styles.textLabel, styles.colorMuted, styles.mt1]}>

What alternative solutions did you explore? (Optional).

[robertKozik]

Thanks for pinging @stephanieelliott !

Thank you all for your proposals 🙇🏼
It seems that each of you has correctly pinpointed the root cause of the issue. Additionally, the proposed solutions were quite similar. I'm inclined to choose @namhihi237's proposal, as it was the quickest and first to introduce this particular solution. However, I believe we should proceed with your alternative solution to avoid any style duplications on children nodes.

Selected proposal: Proposal
Contributor: @namhihi237

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[PiyushChandra17]

@stephanieelliott How about my proposal ? I think it's slightly different from what's @namhihi237 proposed

[aldo-expensify]

@PiyushChandra17 thanks for the proposal, but to me it looks almost the same as @namhihi237's proposal, except that you are dropping the left margin (styles.ml1) which I'm not sure is something we want.

[melvin-bot]

📣 @namhihi237 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @usmantariq96 We're missing your Upwork ID to automatically send you an offer for the Reporter role.
Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @namhihi237 got assigned: 2023-09-08 17:06:07 Z
• when the PR got merged: 2023-09-18 04:49:29 UTC
• days elapsed: 5

On to the next one 🚀

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

• fix padding description and amount #27088

If no regressions arise, payment will be issued on 2023-09-27. 🎊

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:

• @namhihi237 requires payment offer (Contributor)
• @robertKozik does not require payment (Contractor)
• @usmantariq96 requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@robertKozik] The PR that introduced the bug has been identified. Link to the PR:
• [@robertKozik] 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:
• [@robertKozik] 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:
• [@robertKozik] Determine if we should create a regression test for this bug.
• [@robertKozik] 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.
• [@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

@stephanieelliott, @namhihi237, @robertKozik, @aldo-expensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[aldo-expensify]

ping @robertKozik for the checklist

[robertKozik]

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:

• [@robertKozik] The PR that introduced the bug has been identified. Link to the PR: It's hard to find exact PR as the lint commit was applied for these styles, considering that It could be forgotten during initial implementation of component I've stopped looking for it
• [@robertKozik] 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: N/A
• [@robertKozik] 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: N/A
• [@robertKozik] Determine if we should create a regression test for this bug. I think we don't need any regression tests for it as it is purely visual bug
• [@robertKozik] 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. N/A

@aldo-expensify checklist done. I propose not to create regression tests for this case, as it is purely visual bug

[stephanieelliott]

Summarizing payment on this issue:

• Issue reporter: @usmantariq96 $250 (paid via Upwork)
• Contributor: @namhihi237 $250 (paid via Upwork)
• Contributor+: @robertKozik, no payment due

Upwork job is here, no bonus onpayout