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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6350706_1705942135356.1000009543.mp4

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

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

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

[melvin-bot]

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

[melvin-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.

App/src/components/Icon/index.tsx

Lines 92 to 100 in cc64de9

	<ImageSVG
	src={src}
	width={iconWidth}
	height={iconHeight}
	fill={fill}
	hovered={hovered}
	pressed={pressed}
	contentFit={contentFit}
	/>

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.

App/src/components/ImageSVG/index.tsx

Line 10 in cc64de9

additionalProps.fill = fill;

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.

App/src/components/ImageSVG/index.native.tsx

Line 7 in cc64de9

const tintColorProp = fill ? {tintColor: fill} : {};

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.

App/assets/images/eReceiptIcon.svg

Line 1 in 0efabc2

<svg xmlns="http://www.w3.org/2000/svg" fill="currentColor" viewBox="0 0 72 100"><style type="text/css">.st0{fill-rule:evenodd;clip-rule:evenodd}</style><path d="M0 95.6235V4.3765C0 2.49065 2.35447 1.61234 3.6053 3.03159L7.45184 7.39606C8.27143 8.32601 9.72857 8.32601 10.5482 7.39606L16.4518 0.69746C17.2714 -0.232485 18.7286 -0.232487 19.5482 0.697458L25.4518 7.39606C26.2714 8.32601 27.7286 8.32601 28.5482 7.39606L34.4518 0.69746C35.2714 -0.232485 36.7286 -0.232487 37.5482 0.697458L43.4518 7.39606C44.2714 8.32601 45.7286 8.32601 46.5482 7.39606L52.4518 0.69746C53.2714 -0.232485 54.7286 -0.232487 55.5482 0.697458L61.4518 7.39606C62.2714 8.32601 63.7286 8.32601 64.5482 7.39606L68.3947 3.03159C69.6455 1.61235 72 2.49065 72 4.3765V95.6235C72 97.5093 69.6455 98.3877 68.3947 96.9684L64.5482 92.6039C63.7286 91.674 62.2714 91.674 61.4518 92.6039L55.5482 99.3025C54.7286 100.232 53.2714 100.232 52.4518 99.3025L46.5482 92.6039C45.7286 91.674 44.2714 91.674 43.4518 92.6039L37.5482 99.3025C36.7286 100.232 35.2714 100.232 34.4518 99.3025L28.5482 92.6039C27.7286 91.674 26.2714 91.674 25.4518 92.6039L19.5482 99.3025C18.7286 100.232 17.2714 100.232 16.4518 99.3025L10.5482 92.6039C9.72857 91.674 8.27143 91.674 7.45184 92.6039L3.6053 96.9684C2.35448 98.3877 0 97.5093 0 95.6235Z" class="st0"/></svg>

<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.

App/src/components/EReceiptThumbnail.tsx

Line 100 in cc64de9

fill={secondaryColor}

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.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-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]

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

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

• fix: EReceipt thumbnail icon is shown in black color #35217

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

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]

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: Expo image #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: Expo image #30905 (comment)
• 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]

@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!