[HOLD for payment 2023-09-12] Design clean-up and consistency across `expense` reports and requests.

[JmillsExpensify]

We've been making a lot of fast-and-loose changes to requests and reports as part of waves3-wave5, and as a result of adding merchant names for wave4, the end result isn't what we intended. Accordingly, this is a summary of what's broken and needs to be fixed.

1. We're showing right carets for all report and request previews, when instead right carets should be removed from all request and report previews.

2. Similarly, IOU reports are showing the merchant when they should instead be showing the description (because IOUs always have the same merchant, which is Request).

3. When a merchant name exists for an expense / request preview, we only show the merchant.

4. We're missing the X requests logic on report previews in the workspace chat. More specifically, when more than one request exists on the expense report, we replace the merchantName with X requests, where X is the number of requests on the report. We also show up to three receipt images in the report preview. If more than three receipts exist, then we show an overlay in the lower right corner of the right most receipt, +X. This overlay counts the number of additional receipts (e.g. receipts over 3) that exist on the report but can't be seen. Examples for completeness.

[melvin-bot[bot]]

Current assignee @JmillsExpensify is eligible for the NewFeature assigner, not assigning anyone new.

[mountiny]

@koko57 would you be up for this one?

[koko57]

Sorry, I'll be ooo from tomorrow till 10th Sept., so maybe someone else from my team could take it?

[JmillsExpensify]

@mountiny @luacmartins can we get someone else on this one given the OOO? I think we really need to have these items cleaned up ahead of 6th September for the product promotions.

[luacmartins]

asked here

[mountiny]

Also asked if any C+ has time to tackle this https://expensify.slack.com/archives/C02NK2DQWUX/p1693362407056109 I think Callstack might have a bit less availability now ahead of RNEU conference.

[JmillsExpensify]

Nice, thanks ya'll!

[rushatgabhane]

Hi, I can work on this if no one from callstack has bandwidth

[mountiny]

@rushatgabhane Michael should be able to, you could do c+ role here to help with the code review

[rushatgabhane]

Okay, so to be clear I'm just reviewing the PR that Michael will raise?

[rezkiy37]

Hi, I'm Michael (Mykhailo) from Callstack and I would like to work in this issue.

[rushatgabhane]

@rezkiy37 let's do this!

[rezkiy37]

@JmillsExpensify, just to clarify the 4th step. The app already handles cases with one or multiple requests and receipts. I see only one discrepancy associated with the overlay, that counts the number of additional receipts.

https://github.com/Expensify/App/blob/03d13f7c36d94e37d889565b063ed2eed02a32be/src/components/ReportActionItem/ReportPreview.js#L121-L126

Example: multiple and more than 3 receipts

Example: multiple and scanning

Example: single

So, based on this investigation, I think we need to fix the overlay. Am I right? Please correct me if I am wrong 🙂

[mountiny]

We also need to fix this for a normal Cash requests, thats where it was not working and I agree with the overlay issue here the cards are no visible

[rezkiy37]

@mountiny, do you mean this case:

https://github.com/Expensify/App/assets/57314631/6f01e3b2-b4b5-4648-ada1-4e6c24a58259

Should we add a counter under the amount?

[JmillsExpensify]

So, based on this investigation, I think we need to fix the overlay. Am I right? Please correct me if I am wrong 🙂

@rezkiy37 Yes you're exactly right! Thank you for confirming.

[mountiny]

@rezkiy37 Yes correct, there should be the counter same as for the receipts or distance!

[rezkiy37]

@JmillsExpensify, is it possible to provide access for a design for this specific component? I am not sure, that I can do it precisely without colors, dimensions and so on. Thank you!

[mountiny]

I am actually not sure if we want that design

[mountiny]

Asked internally

[melvin-bot[bot]]

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

[shawnborton]

Another thing to consider is the shape of the thumbnail. I wonder if we want to make everything appear rounded, as it feels a bit nicer with our design system? For example, look at this mock that has rounded corners for the thumbnails:

Whereas this doesn't:

Thoughts @dannymcclain?

[rezkiy37]

Proposal

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

The app has several inaccuracies for previewing a description and merchant of expense reports and expense requests.

What is the root cause of that problem?

1. We're showing right carets for all report and request previews, when instead right carets should be removed from all request and report previews.

There is an icon (ArrowRight) of ReportPreview and MoneyRequestPreview.

2. Similarly, IOU reports are showing the merchant when they should instead be showing the description (because IOUs always have the same merchant, which is **Request**).

There is a condition to show/hide the merchant of MoneyRequestPreview. As we can see below, there is not any restrictions if it is an IOU.

https://github.com/Expensify/App/blob/a19a67213c1674b1178a66d23aad6fa097f639fe/src/components/ReportActionItem/MoneyRequestPreview.js#L278

3. When a merchant name exists for an expense / request preview, we only show the merchant.

There is a condition to show/hide the description of MoneyRequestPreview. As we can see below, there is not any restrictions if the merchant exists.

https://github.com/Expensify/App/blob/a19a67213c1674b1178a66d23aad6fa097f639fe/src/components/ReportActionItem/MoneyRequestPreview.js#L288

4. We also show up to three receipt images in the report preview. If more than three receipts exist, then we show an overlay in the lower right corner of the right most receipt, +X.

There is a remaining counter of ReportActionItemImages.

https://github.com/Expensify/App/blob/c20c3870ee46d6788de554be27706e9898638904/src/components/ReportActionItem/ReportActionItemImages.js#L66-L70

However, it uses deprecated/wrong styles. It is centralized, rounded, etc.

5. Distance requests appear to be showing the merchant twice.

There is a condition to use the merchant as a description for distance requests of MoneyRequestPreview. As we can see below.

https://github.com/Expensify/App/blob/a19a67213c1674b1178a66d23aad6fa097f639fe/src/components/ReportActionItem/MoneyRequestPreview.js#L158-L160

6. Normal "Cash" requests are missing the X requests logic.

There is a condition in ReportPreview. It has a restriction to preview counter only for requests with receipts.

https://github.com/Expensify/App/blob/c20c3870ee46d6788de554be27706e9898638904/src/components/ReportActionItem/ReportPreview.js#L209-L215

7. Cards are no visible when they are inside "Only visible for you" message.

That's happens because when a message is a whisper, it has the the same background color as for ReportActionItem (parent) and for ReportPreview (child).

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

1. We're showing right carets for all report and request previews, when instead right carets should be removed from all request and report previews.

Remove icon (ArrowRight) from ReportPreview and MoneyRequestPreview.

2. Similarly, IOU reports are showing the merchant when they should instead be showing the description (because IOUs always have the same merchant, which is **Request**).

Create a variable like shouldShowMerchant (boolean). it should checks:

1. if the merchant exists.
2. if it is not a split bill.
3. if it has a receipt. This step verifies that the merchant is shown only for expense / request preview.

Something like this:

const shouldShowMerchant = !_.isEmpty(requestMerchant) && !props.isBillSplit && hasReceipt;

3. When a merchant name exists for an expense / request preview, we only show the merchant.

Create a variable like shouldShowDescription (boolean). it should checks:

1. if the description exists.
2. if it should show the merchant. This step verifies that the merchant is shown and the app hides the description.

Something like this:

const shouldShowDescription = !_.isEmpty(description) && shouldShowMerchant;

4. We also show up to three receipt images in the report preview. If more than three receipts exist, then we show an overlay in the lower right corner of the right most receipt, +X.

Modify styles.reportActionItemImagesMore to have a proper UI.

5. Distance requests appear to be showing the merchant twice.

Extend the shouldShowMerchant variable from the 2nd step. It should take into account if it is a distance request.

Something like this:

const shouldShowMerchant = !_.isEmpty(requestMerchant) && !props.isBillSplit && (hasReceipt || isDistanceRequest);

Also, we need to remove lines that insert the merchant as a description.

https://github.com/Expensify/App/blob/a19a67213c1674b1178a66d23aad6fa097f639fe/src/components/ReportActionItem/MoneyRequestPreview.js#L158-L160

6. Normal "Cash" requests are missing the X requests logic.

Modify a condition and remove restriction on receipts. It unblocks previewing counter for normal "cash" requests. Also, add a restriction on amount to show counter only when 2 requests at least.

7. Cards are no visible when they are inside "Only visible for you" message.

Pass a property to ReportPreview - isWhisper (boolean). Based on this value, ReportPreview will enable styles.reportPreviewBoxHoverBorder. This "hover" effect looks good in a combination with a whisper.


What alternative solutions did you explore? (Optional)

N/A

[JmillsExpensify]

Another thing to consider is the shape of the thumbnail. I wonder if we want to make everything appear rounded, as it feels a bit nicer with our design system? For example, look at this mock that has rounded corners for the thumbnails:

Great point. I like the rounded corners. Looks more friendly and groovy haha.

[JmillsExpensify]

@JmillsExpensify, is it possible to provide access for a design for this specific component? I am not sure, that I can do it precisely without colors, dimensions and so on. Thank you!

@dannymcclain Do you mind helping with the dimensions, colors, etc. I don't think we have this one in our general design system yet.

[shawnborton]

Gonna start dropping some stuff into here quickly

[dylanexpensify]

@rushatgabhane mind reviewing proposal as soon as you can? 🙇‍♂️

[neil-marcellini]

FYI I fixed the duplicated distance merchant in this PR.

[JmillsExpensify]

Nice. Thanks @neil-marcellini. I'll remove from this issue then.

[mountiny]

@rezkiy37 please feel free to make the PR, you proposal lokos good and we can refine any details in the PR itself

[mountiny]

asking in slack for an update 🚀

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.63-2 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/26603

If no regressions arise, payment will be issued on 2023-09-12. :confetti_ball:

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:

• @rushatgabhane does not require payment (Eligible for Manual Requests)
• @rezkiy37 does not require payment (Contractor)

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

[mountiny]

ready for the payment

[luacmartins]

I think we're just waiting on payment here

[luacmartins]

@JmillsExpensify can you help with payment please?

[luacmartins]

Bump @JmillsExpensify for payment

[shawnborton]

Still waiting for payment?

[shawnborton]

Still waiting for payment

[mountiny]

We only need to pay $500 to @situchan here, otherwise nothing to pay and we can close this.

[JmillsExpensify]

Offer sent to @situchan.

[JmillsExpensify]

Offer accepted and paid.