[$250] Padding is different in the expense report header and transaction thread

[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.4.61-0
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: @paultsimura
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712438375235479

Action Performed:

1. Open any the expense report
2. Observe the header styling
3. Open the expense and go to transaction thread

Expected Result:

Both should be same

Actual Result:

different styling (padding) in the header of an expense report & transaction thread (now it's especially visible after the combined view was introduced)

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

Add any screenshot/video evidence

Recording.2955.mp4

Screen.Recording.2024-04-06.at.23.17.44.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fbb293b8872b8543
• Upwork Job ID: 1778856398639214592
• Last Price Increase: 2024-04-12
• Automatic offers:
• paultsimura | Reviewer | 0
• bernhardoj | Contributor | 0

[melvin-bot]

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[bernhardoj]

Proposal

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

There are extra padding in the header of a single transaction page header.

What is the root cause of that problem?

Inside the header (MoneyReportHeader), we use AvatarWithDisplayName. If shouldShowSubscriptAvatar is true and it's not an expense request (transaction thread), we add an extra margin left.

App/src/components/AvatarWithDisplayName.tsx

Line 131 in 38ced97

<View style={[styles.flex1, styles.flexColumn, shouldShowSubscriptAvatar && !isExpenseRequest ? styles.ml4 : {}]}>

Both conditions are satisfied when opening a single transaction page, so we see an extra spacing between the avatar and the display name.

This also happens in the anonymous report footer.

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

Completely remove the extra margin left.

App/src/components/AvatarWithDisplayName.tsx

Line 131 in 38ced97

<View style={[styles.flex1, styles.flexColumn, shouldShowSubscriptAvatar && !isExpenseRequest ? styles.ml4 : {}]}>

[allgandalf]

Proposal

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

We see extra padding for moneyRequestview header

What is the root cause of that problem?

We add extra padding in report header if the current request is not a expense report and shouldShowSubscriptAvatar is true:

App/src/components/AvatarWithDisplayName.tsx

Line 131 in 38ced97

<View style={[styles.flex1, styles.flexColumn, shouldShowSubscriptAvatar && !isExpenseRequest ? styles.ml4 : {}]}>

This gives us the extra padding which we see in money requests.

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

We should update the check such that it will also check if the report is not IOU Request, this will solve the extra padding problem and also maintaining the original intended functionality :

<View style={[styles.flex1, styles.flexColumn, shouldShowSubscriptAvatar && !isExpenseRequest && !isIOURequest ? styles.ml4 : {}]}> 

App/src/libs/ReportUtils.ts

Line 673 in 61dbbdf

function isIOUReport(reportOrID: OnyxEntry<Report> | string | EmptyObject): boolean {

What alternative solutions did you explore? (Optional)

[shawnborton]

Can we try to figure out what caused this? This wasn't broken about a week ago, so I am pretty convinced that we introduced a regression somewhere and I would love to track it down.

[bernhardoj]

This is the condition that adds the extra spacing.

shouldShowSubscriptAvatar && !isExpenseRequest ? styles.ml4

I'm pretty sure this started to happen when we introduced the one-transaction report.

In one-transaction report, we show the subscript avatar

App/src/libs/ReportUtils.ts

Lines 4950 to 4952 in 4e13dad

	if (isExpenseReport(report) && isOneTransactionReport(report?.reportID ?? '')) {
	return true;
	}

but it's not an expense request, so the spacing is added.

[shawnborton]

Got it, cc @NikkiWines for that.

But just curious - are there any conditions where we'd want a gap that large? I don't think so, right? Just wondering if we can get rid of that entirely.

[paultsimura]

@sonialiap could you please assign me as C+ here as I've reported the bug and have been following the discussion since the beginning?

[melvin-bot]

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

[NikkiWines]

I can confirm this an external bug, assigning @paultsimura as the C+

[melvin-bot]

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

[melvin-bot]

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

[NikkiWines]

But just curious - are there any conditions where we'd want a gap that large? I don't think so, right? Just wondering if we can get rid of that entirely.

Agreed here, I think we can standardize on the spacing for the report and request headers 👍

[paultsimura]

@grgia I see you added the extra left margin in this commit – would be awesome if you could remember what issue this margin was fixing (the commit message says "Fix expense request"), because I'm struggling to figure this out 🤔

[paultsimura]

So far @bernhardoj's proposal looks good to me.

Maybe Georgia will provide some cases I'm missing, but I think we can address it from within the PR.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 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 📖

[sonialiap]

@grgia bump on this question #39868 (comment) 😊

[grgia]

@sonialiap @paultsimura we can remove that line, at the time it was required for the headers

[paultsimura]

we can remove that line

Cool, thanks for the confirmation.
@bernhardoj feel free to proceed with the PR 🚀

[bernhardoj]

PR is ready

cc: @paultsimura

[paultsimura]

This was deployed to production on April 22, due payment 2024-04-29

[sonialiap]

Payment summary:

• @paultsimura $250 - paid ✔️
• @bernhardoj $250 - paid ✔️