Update Thumbnails in expense details and report preview#87056
Update Thumbnails in expense details and report preview#87056sosek108 wants to merge 2 commits intoExpensify:mainfrom
Conversation
|
@ikevin127 Please 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] |
melvin-bot
bot
requested review from
joekaufmanexpensify
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0020220fd
ℹ️ 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".
| const oldReceipt = transaction?.receipt ?? {}; | ||
| const receiptOptimistic = { | ||
| source, | ||
| localSource: null, |
There was a problem hiding this comment.
Setting receiptOptimistic.localSource to null causes ReportActionItemImage to treat localSource as defined (localSource !== undefined) and overwrite a string image with null, which yields an empty resolved image source during optimistic updates. In the rotate/crop replaceReceipt flow this can make the receipt preview disappear (notably in wide RHP where shouldUseThumbnailImage is false) until the server response arrives, and it remains user-visible on slow/offline connections.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — this is a real issue. Here's the trace:
replaceReceipt sets localSource: null in receiptOptimistic (correct Onyx semantics — you need explicit null to clear a key on merge). But ReportActionItemImage.tsx line 140 uses a strict !== undefined guard:
const effectiveImage = localSource !== undefined && typeof image === 'string' ? localSource : image;When localSource = null:
null !== undefined→true← the problem- condition passes →
effectiveImage = null - Line 142:
tryResolveUrlFromApiRoot(null ?? '')→ resolves to an empty string
So during the optimistic update after rotate/crop, the receipt image goes blank until the server responds. On slow or offline connections this is user-visible.
Line 139 is fine — null ?? thumbnail correctly falls back via nullish coalescing. Only line 140 needs fixing.
Fix needed in ReportActionItemImage.tsx line 140:
// before
const effectiveImage = localSource !== undefined && typeof image === 'string' ? localSource : image;
// after
const effectiveImage = localSource != null && typeof image === 'string' ? localSource : image;!= null (loose equality) catches both null and undefined as "no local source", which is the correct semantic. The localSource: null in the optimistic data is right — it just needs a corresponding update to the consumer.
|
🚧 @Beamanator has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Please also fix #87029 (another blocker) as well |
|
Looks like Melvin is suggesting basically the same change to fix that blocker too - see here: #87029 (comment) |
|
Closing in favor of #87091 |
3 participants
Explanation of Change
The PR #86512 introduced a localSource field on
transaction.receiptto preserve the local file URI when a scan expense is created, preventing the server's remote URL from causing an unnecessary image reload. ReportActionItemImage renderslocalSource ?? source, so the locally available image is shown immediately without re-downloading. The regression fix addslocalSource: nullto receiptOptimistic in replaceReceipt, ensuring that when a receipt is rotated or cropped the stale local URI is cleared and the freshly updated image is displayed correctly.Fixed Issues
$ #87051
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)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Nagranie.z.ekranu.2026-04-3.o.09.45.04.mov
iOS: mWeb Safari
MacOS: Chrome / Safari