[HOLD for payment 2024-02-07] [$500] Android - Center icon in EReceipt for Expensify transactions is blank

[lanitochka17]

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.29-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: Applause - Internal Team Slack conversation:

Action Performed:

1. Log in to account with Expensify Card transaction [email protected]
2. Go to workspace chat with the transaction https://staging.new.expensify.com/r/4612388815642526
3. Navigate to transaction details page
4. Open Ereceipt

Expected Result:

The center icon is not blank

Actual Result:

The center icon is blank

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/b8f07032-b293-4f86-808b-31071ff96cae

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01df6f886be1e32b04
• Upwork Job ID: 1749504709300957184
• Last Price Increase: 2024-01-22
• Automatic offers:
• jjcoffee | Reviewer | 28120700
• aswin-s | Contributor | 28120701

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[lanitochka17]

We think that this bug might be related to #wave5. CC @dylanexpensify

[aswin-s]

Proposal

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

Center icon in EReceipt for Expensify transactions is black in color on Android

What is the root cause of that problem?

This looks like a regression introduced by the expo-image PR. Earlier SVG images were directly imported into the component using react-native-svg. After above PR Icon component was updated to use ImageSvg component to display svg images.

https://github.com/Expensify/App/blob/cc64de9ff6cb556927821ab84c9ddf491be91437/src/components/Icon/index.tsx#L92-L100

For web platform the fill prop supplied to ImageSVG is directly passed down to the SVG component. We are still using react-native-svg on web platform.

https://github.com/Expensify/App/blob/cc64de9ff6cb556927821ab84c9ddf491be91437/src/components/ImageSVG/index.tsx#L10

On native platform since svg is treated just like any other image format by expo-image, fill prop cannot be supplied directly. Instead fill color is applied to tintColor prop and applied to the expo image component.

https://github.com/Expensify/App/blob/cc64de9ff6cb556927821ab84c9ddf491be91437/src/components/ImageSVG/index.native.tsx#L7

The icon used in EReceiptThumbnail center icon is eReceiptIcon.svg. It has fillColor set to currentColor. Expo image library seems to have an issue with getting currentColor prop on Android. Instead it uses black color as fill. Any tintColor applied on top of black doesn't seem to work.

https://github.com/Expensify/App/blob/0efabc2b40fe845b13faa94ec508a6693167ede9/assets/images/eReceiptIcon.svg#L1

<svg xmlns="http://www.w3.org/2000/svg" fill="currentColor" ... />

This is the reason EReceipt icon looks blank.

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

Remove fill="currentColor" from the svg element.

Fill color for the receipt icon is already supplied via fill prop from EReceiptThumbnail component. So it works seemlessly on both web and native platforms. So fill="currentColor" can be safely removed.

https://github.com/Expensify/App/blob/cc64de9ff6cb556927821ab84c9ddf491be91437/src/components/EReceiptThumbnail.tsx#L100

What alternative solutions did you explore? (Optional)

None

[jjcoffee]

@aswin-s Thanks for the proposal! I think your solution would not solve the issue though, if you look at the original PR that implemented EReceipts, the icon should have a colour.

[aswin-s]

@jjcoffee expo-image was introduced after this PR. EReceiptThumbnail uses Icon component to display the image. Icon component now usesImageSVG component to display SVG images. Looks like this is a regression from expo-image integration. I tested it both on web and android and the above change fixes the image on both platforms.

[jjcoffee]

@aswin-s Thanks! I agree the fix works, normally we would require details as to why/how it works too (hence my confusion with thinking it would display without any colour). Are you able to update your proposal with that extra detail?

[aswin-s]

@jjcoffee Updated my proposal with additional details.

[jjcoffee]

@aswin-s Sorry, I meant details on why the solution is a good one (how it works, where does the fill come from if we remove it from there).

[aswin-s]

@jjcoffee Added more details on how ImageSVG works on both web and native platforms and why currentColor is not required anymore. Let me know if anything needs further clarification.

[jjcoffee]

Sweet, thanks for adding that! Happy to go with @aswin-s's proposal, as essentially we don't need the fill in the SVG anymore.

:ribbon::eyes::ribbon: C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @jjcoffee 🎉 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[bot]]

📣 @aswin-s 🎉 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[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.4.34-1 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/35217

If no regressions arise, payment will be issued on 2024-02-07. :confetti_ball:

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

• @aswin-s requires payment automatic offer (Contributor)
• @jjcoffee requires payment automatic offer (Reviewer)

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

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

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/30905
• 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: https://github.com/Expensify/App/pull/30905#issuecomment-1927206517
• 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 - just missed in testing
• Determine if we should create a regression test for this bug. Yes, thought something similar may already exist
• 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.

Regression Test Proposal

1. Create an ECard transaction
2. Navigate to the transaction detail page
3. Verify that the icon at the centre of the EReceipt has a coloured fill that matches the background image

Do we agree 👍 or 👎

[johncschuster]

Payment has been issued! I'll get the regression test steps updated shortly.

[melvin-bot[bot]]

@johncschuster, @aswin-s, @jjcoffee, @marcaaron Eep! 4 days overdue now. Issues have feelings too...

[marcaaron]

This one looks like it can be closed now?

[johncschuster]

Yep! I left it open so I would get the Regression Test steps done, but it is otherwise complete!