Show thumbnail preview while receipt image loads#88575
Conversation
|
@Eskalifer1 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] |
|
Hi, i will review it today! |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e51a2698d4
ℹ️ 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".
|
Hi @sosek108, Can you let me know if this PR is ready for review? If not, please bump me when it’s ready, since it doesn’t seem fully ready yet — I see that not all comments have been addressed, there are no platform-specific videos, and there’s an ESLint error. If it’s ready for review, I’ll get back to it today. Just let me know :) |
marcaaron
left a comment
There was a problem hiding this comment.
Changes LGTM. The PR to fix "OpenApp does not provide thumbnail data for newly-uploaded receipts" is on staging now if we want to test against there to see if that bug has been solved now.
|
@Eskalifer1 I fixed CI issues. edit: I spotted some error on iOS today. So |
Can you recheck this on This is how it behaves on my Nagranie.z.ekranu.2026-05-5.o.12.11.24.mov |
|
Yeah, I know it's in the main branch, but it seems like the changes in this PR have made it more noticeable. If you think it's okay, then we can leave it as is. |
|
I'll try to handle this, but the issue is somewhere else. Nagranie.z.ekranu.2026-05-5.o.12.57.35.mov |
|
After some investigation I'm not able to easily fix this. The thumbnail feature itself is correct — the glitches are entirely from pre-existing remounting instability that causes ImageWithLoading to restart from scratch each time. I'd propose to create new issue for this and handle this separately CC: @marcaaron |
I can reproduce this on Nagranie.z.ekranu.2026-05-5.o.13.48.55.mov |
done. |
|
Well, of course, I didn’t insist on fixing it, since I already know these behavior are in the main branch. I was just curious to hear your thoughts on this, because sometimes certain changes can make bugs (or certain behaviors) that were already there more noticeable. If QA asks about this, we’ll be able to tell them that it’s already in the main branch. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-native.movAndroid: mWeb Chromeandroid-web.moviOS: HybridAppios-native.moviOS: mWeb Safariios-web.movMacOS: Chrome / Safariweb.mov |
|
One last question I have: what do you think about adding a note to the Tests section explaining that this preview appears frequently, but not always, depending on various factors, for example, this is clearly visible in the videos you recorded, especially on Android native, where the low-resolution preview doesn’t always have time to load when opening; I’ve also encountered this, especially on native platforms |
|
@marcaaron (cc: @mountiny) There are two issues that are on main:
From UX perspective they look like related to my changes but deep dive to codebase and the root cause is in other places. I think we should work on these but propose to do them in separate issues. What do you think? |
Right, we should describe these |
I agree, let's see what others say |
|
Should we put this on hold? cc: @marcaaron @sosek108 |
|
Yes. Who is looking into the fix PR? Do we need another issue for that or handling it here? |
|
@marcaaron I think I can look into it, but currently I'm helping with Onyx investigation which is a little bit more urgent for me. I should be able to get back here this week |
|
can you retest this one? #88575 (review) |
|
Hi @sosek108, Yep, it works much more smoothly now! Let's remove “HOLD” from the name and fix the ESLint rules |
|
Hi @Expensify/design, while working on this PR, I noticed a small UI inconsistency related to the trip details section. Specifically, the indentation between “Passenger” and the following element differs depending on whether we are on “Rail details” or “Train details.” The fix itself is very straightforward (just a one-line change). If we decide it’s worth addressing, I’d be happy to help with it — either by working on it together with Melvin as C+, or by opening a small PR myself if that’s preferred. 2026-05-20.17.08.30.mov |
|
Yeah I think we should fix it 👍 working with Melvin seems like a good idea. |
|
@shawnborton Can you please create an issue for that and assign me pls ^) |
|
@Eskalifer1 all checks are passing now! |
|
@marcaaron Can we proceed with merging this? |
Explanation of Change
When a receipt image is loading, the app previously showed only a spinner. This PR renders a low-resolution thumbnail (
thumbnail320) as a placeholder while the full-resolution image loads. The thumbnail itself shows a spinner until it is ready, then the full image replaces it cleanly. If no thumbnail is available the existing spinner-only behavior is preserved.Three files changed:
ImageWithLoading: accepts a newthumbnail320prop and, when loading, overlays anImageusingStyleSheet.absoluteFillwith that URI; the spinner is only shown while the thumbnail itself is still loading.ReceiptImage: threadsthumbnail320through toImageWithLoading.ReportActionItemImage: derivesthumbnail320SourcefromgetThumbnailAndImageURIs(skipping local files) and passes it to bothReceiptImagecall sites.Fixed Issues
$ #85875
PROPOSAL:
Tests
Prequisities: You should start testing on freshly logged in account and the best experience should be in incognito mode.
Spendpage -> TransactionsNote: this fix aims primarily to improve performance for desktop web. It adds the functionality to iOS and Android platform but due to other issues (not related to the fix) the performance gain is not visible. These two issues one and two will be visible while testing.
Offline tests
N/A
QA Steps
Same as tests
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_final.mov
new (May 21)
Nagranie.z.ekranu.2026-05-21.o.08.54.50.mov
Android: mWeb Chrome
Nagranie.z.ekranu.2026-05-5.o.13.59.07.mov
iOS: Native
Nagranie.z.ekranu.2026-04-29.o.13.38.45.mov
new (May 21)
Nagranie.z.ekranu.2026-05-21.o.08.45.54.mov
iOS: mWeb Safari
Nagranie.z.ekranu.2026-05-5.o.13.56.03.mov
MacOS: Chrome / Safari
web_final.mov