[HOLD for payment 2024-06-28] [$125] [Search v1] No background color placeholder exists for receipt thumbnail

[luacmartins]

Coming from this thread, the receipt thumbnail doesn't have a placeholder background color.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019803a773929eed48
• Upwork Job ID: 1800191951832943611
• Last Price Increase: 2024-06-10
• Automatic offers:
• dukenv0307 | Reviewer | 102688635
• Krishna2323 | Contributor | 102688638

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

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

[shawnborton]

The color should be the same that we use for the empty state thumbnail:

Which probably means we need to consider the hover state too - I think we bump that background color a shade up when the expense or report row is hovered.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~019803a773929eed48

[melvin-bot]

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

[melvin-bot]

Upwork job price has been updated to $125

[Krishna2323]

@shawnborton, can you help me understand the issue? Where do we need the bg, on the container which shows the existing receipt?

[shawnborton]

Take a look at this screenshot, notice how before the receipt loads, there is no background color for the container and thus it looks like a giant blank space:

[Krishna2323]

Thanks a lot, understood

[Krishna2323]

Proposal

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

[Search v1] No background color placeholder exists for receipt thumbnail

What is the root cause of that problem?

No bg is applied to the container.

App/src/components/SelectionList/Search/TransactionListItemRow.tsx

Line 77 in 169109e

<View style={[StyleUtils.getWidthAndHeightStyle(variables.h36, variables.w40), StyleUtils.getBorderRadiusStyle(variables.componentBorderRadiusSmall), styles.overflowHidden]}>

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

Add the same bg used for the thumbnail. We can use isHovered ? styles.activeComponentBG : styles.hoveredComponentBG,, or we can simply use styles.activeComponentBG without checking for hovered state.

What alternative solutions did you explore? (Optional)

[Krishna2323]

@shawnborton, do we need to change the bg on hover? I think we shouldn't do that because the empty receipt placeholder also does not change its bg on hover.

Monosnap.screencast.2024-06-10.21-48-11.mp4

[shawnborton]

Ah okay, then if that's the case, let's just use that same exact BG color from your screenshot.

[TheGithubDev]

Proposal

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

The problem is that no background color placeholder exists for the receipt thumbnail in the Search tab, resulting in a visually inconsistent user experience when the receipt thumbnail is not loaded.

What is the root cause of that problem?

The root cause of this problem is that no background color is applied to the container of the receipt thumbnail. The relevant code snippet is:

<View style={[StyleUtils.getWidthAndHeightStyle(variables.h36, variables.w40), StyleUtils.getBorderRadiusStyle(variables.componentBorderRadiusSmall), styles.overflowHidden]}>

The container lacks a background color style, which causes it to appear blank when the thumbnail is not loaded.

What changes do you think we should make in order to solve the problem?
We should add a background color to the container of the receipt thumbnail. The background color can be applied by modifying the style to include a background color from the existing styles. Here are the specific changes:

1. Add Background Color Style:
   • Apply a background color style to the container of the receipt thumbnail. This can be done by using a background color from the existing styles, such as styles.activeComponentBG or styles.hoveredComponentBG.

Example of how the changes might look:

<View style={[
    StyleUtils.getWidthAndHeightStyle(variables.h36, variables.w40), 
    StyleUtils.getBorderRadiusStyle(variables.componentBorderRadiusSmall), 
    styles.overflowHidden,
    styles.activeComponentBG // Added background color
]}>

Alternatively, if you want to handle the hovered state:

<View style={[
    StyleUtils.getWidthAndHeightStyle(variables.h36, variables.w40), 
    StyleUtils.getBorderRadiusStyle(variables.componentBorderRadiusSmall), 
    styles.overflowHidden,
    isHovered ? styles.activeComponentBG : styles.hoveredComponentBG // Conditional background color based on hover state
]}>

What alternative solutions did you explore?

1. Using a Default Image Placeholder: Instead of a solid background color, a default image or icon could be used as a placeholder. This would provide a more visually appealing placeholder but might require additional assets and logic to handle the image.

2. Implementing a Loading Spinner: Adding a loading spinner or skeleton loader within the container until the thumbnail is fully loaded. This approach enhances user feedback but could complicate the loading logic.

3. CSS Gradients: Using CSS gradients as a background to provide a more visually interesting placeholder. This approach could improve aesthetics but might not be consistent with the rest of the app's design language.

After considering these alternatives, applying a simple background color using styles.activeComponentBG could be deemed the most straightforward and consistent solution, ensuring a seamless user experience without introducing additional complexity.

Please note that the above explanation outlines only the technical approach I plan to take. If my proposal gets accepted, I will submit the finalized solution as mentioned.

Thanks!

[dukenv0307]

Let's go with @Krishna2323's proposal. We need to add clear steps on the PR phase (try with offline mode)

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[luacmartins]

I think the proposal looks good, but @Krishna2323 please note that we removed the hover state from the receipt component and are now just using theme.border color for the background, we should use that.

[melvin-bot]

📣 @dukenv0307 🎉 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]

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

[dukenv0307]

@Krishna2323 Waiting for your PR

[Krishna2323]

@dukenv0307, PR 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.4.85-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: [Search v1] No background color placeholder exists for receipt thumbnail. #43631

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @Krishna2323 requires payment automatic offer (Contributor)
• @dukenv0307 requires payment automatic offer (Reviewer)

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

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

[trjExpensify]

I don't think we need a regression test for this, and it was a cheeky bug in a new feature release.

Payment summary as follows:

• $125 to @Krishna2323 for the fix
• $125 to @dukenv0307 for C+

Paid both, closing!