Create receipt-wrangler-space-cowgirl.svg#95374
Conversation
|
@abzokhattab @iwiznia One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7694964c05
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,209 @@ | |||
| <svg width="400" height="242" viewBox="0 0 400 242" fill="none" xmlns="http://www.w3.org/2000/svg"> | |||
There was a problem hiding this comment.
Register the new illustration before using it
If this asset is meant to power the Submit modal, adding only the SVG leaves it unreachable through the app's illustration loading path: repo-wide product-illustration imports are centralized in src/components/Icon/chunks/illustrations.chunk.ts, and the lazy illustration hooks resolve only names present in that Illustrations map. Without adding this file to that registry and switching the modal to the new key, users will continue to see the existing/placeholder art rather than this illustration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The Codex bot is correct that the raw SVG isn't reachable on its own — product illustrations are only resolvable if they're registered in the Illustrations map in illustrations.chunk.ts, which is what the lazy illustration hooks (e.g. useMemoizedLazyIllustrations) look up by key.
The self-contained fix for this asset-only PR is to register it:
- Add the import alongside the other
product-illustrationsimports:import ReceiptWranglerSpaceCowgirl from '@assets/images/product-illustrations/receipt-wrangler-space-cowgirl.svg';
- Add
ReceiptWranglerSpaceCowgirl,to the exportedIllustrationsmap (alphabetical, nearBigVault).
The bot's second point — "switch the modal to the new key" — is out of scope here: the Submit modal from #87912 isn't part of this PR (this branch only touches the SVG), and there's no consumer of this illustration in the codebase yet. That wiring belongs in the modal PR. Registering the key now just makes the asset available for it.
There was a problem hiding this comment.
Done — registered the illustration in illustrations.chunk.ts (commit ef53ccf), so it's now resolvable by key:
- Added the import:
import ReceiptWranglerSpaceCowgirl from '@assets/images/product-illustrations/receipt-wrangler-space-cowgirl.svg'; - Added
ReceiptWranglerSpaceCowgirlto the exportedIllustrationsmap.
oxfmt, lint-changed (ESLint), and typecheck-tsgo all pass locally.
As noted before, I did not wire it into the Submit modal — that modal (#87912) isn't part of this branch and has no consumer here yet, so that switch belongs in the modal PR. This just makes the key available for it.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
… chunk Co-authored-by: Garrett Knight <garrettmknight@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🚧 garrettmknight has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Explanation of Change
Adding illustration for the Submit modal
Fixed Issues
$#87912
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari