Skip to content

Defer image loading and use original receipt image - #97313

Draft
bernhardoj wants to merge 2 commits into
Expensify:mainfrom
bernhardoj:feat/deferred-image-loading
Draft

Defer image loading and use original receipt image#97313
bernhardoj wants to merge 2 commits into
Expensify:mainfrom
bernhardoj:feat/deferred-image-loading

Conversation

@bernhardoj

Copy link
Copy Markdown
Contributor

Explanation of Change

Fixed Issues

$ #96999
PROPOSAL:

Tests

  • Verify that no errors appear in the JS console

Offline tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

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

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/components/DeferredImageWithLoading.tsx 100.00% <100.00%> (ø)
src/components/ImageWithLoading.tsx 74.28% <ø> (ø)
src/components/ImageWithSizeCalculation.tsx 100.00% <100.00%> (ø)
...ponents/ReportActionItem/ReportActionItemImage.tsx 72.13% <ø> (ø)
src/components/ReceiptImage/index.tsx 68.88% <0.00%> (ø)
... and 14 files with indirect coverage changes

@github-actions

Copy link
Copy Markdown
Contributor

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions

Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #97313.

Android 🤖 iOS 🍎
⏩ SKIPPED ⏩ ⏩ SKIPPED ⏩
The build for Android was skipped The build for iOS was skipped
Web 🕸️
https://97313.pr-testing.expensify.com
Web

👀 View the workflow run that generated this build 👀

@dubielzyk-expensify

Copy link
Copy Markdown
Contributor

I'm unsure if I'm testing it right, but it looks blurry then it loads in the unpixelated one is that correct?

It doesn't feel horrible or anything. I wonder if we could make it feel slightly better by applying a blur effect until it's fully loaded or even making it grayscale or something. Maybe it's a dumb idea, but I wonder if on worse connections that it'll be slow and they'll think we're loading in a really bad resolution then it suddenly swaps to the hires one. Curious what @Expensify/design thinks

@bernhardoj

bernhardoj commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

but it looks blurry then it loads in the unpixelated one is that correct

Yes, the image switching is actually an existing behavior. But in this PR, we switch from the low-res to the full-res. In prod, we switch from low-res to a higher one, but still blurry. Another new behavior in this PR is that no image is shown until the navigation transition is done.

@dannymcclain

Copy link
Copy Markdown
Contributor

Yeah I'm not sure... For some of my receipts it took many seconds to swap to the higher res version, so it looked like the receipt was just blurry. And it seems like the height of the receipt preview area changes once the higher res version loads in? I wonder if it would be better to just spinner the whole thing until we can load the good version? It's not awful by any means, but it doesn't feel super great to me.

But also if this is essentially an upgrade of current behavior I guess we can proceed and keep trying to make it better.

@mountiny

Copy link
Copy Markdown
Contributor

I think the delay is sort of inevitable as if the high resolution receipt is large, then its just taking a while to download couple megabytes and hence the delay. Only thing we could do is to show some loading feedback instead of just a blury larger low res image until the larger res loads

@dannymcclain

Copy link
Copy Markdown
Contributor

Only thing we could do is to show some loading feedback instead of just a blury larger low res image until the larger res loads

Yeah that could definitely help. Even if it's a little unobtrusive thing that still let's you see the low res image in the background (so not totally blocking, but just gives you some affordance)

@dubielzyk-expensify

Copy link
Copy Markdown
Contributor

Yeah, that's kinda what I was getting at. I'm aware the delay is by design, but I think the transition and general "waiting state" can be better. Do we have a way to actually understand when the new image is loaded in? If so, I wonder if we could dim the receipt with a spinner on top and then un-dim with the high resolutiojn receipt

@bernhardoj

Copy link
Copy Markdown
Contributor Author

Do we have a way to actually understand when the new image is loaded in? If so, I wonder if we could dim the receipt with a spinner on top and then un-dim with the high resolutiojn receipt

Yes, we do. We actually already have a loading, but it's only shown after 200ms and if the thumbnail (the lower res) is still loading.

{isLoading && (!previewUri || isThumbnailLoading) && !isImageCached && !isOffline && (
<LoadingIndicator
iconSize={loadingIconSize}
style={[styles.opacity1, styles.bgTransparent, loadingIndicatorStyles]}
/>
)}

We can simplify it and shows whenever the higher-res image is loading.

Here is how it looks (with the dim):

web.mp4

@dannymcclain

Copy link
Copy Markdown
Contributor

Here is how it looks (with the dim):

The last few giant ones in your video still seem a bit weird*, but the first few with normal images actually felt pretty good to me.

*Why is the receipt preview area so massive? I thought it would only ever be as tall as the viewport available. Also I didn't see the spinner on those—is that just because it's out of view because the preview area is so massive?

Does seem like this could be a good solution though because it at least sets users expectations a bit better than just a blurry image.

@bernhardoj

Copy link
Copy Markdown
Contributor Author

*Why is the receipt preview area so massive? I thought it would only ever be as tall as the viewport available.

This is an existing behavior. Maybe it was intentionally designed like that?

Also I didn't see the spinner on those—is that just because it's out of view because the preview area is so massive?

Yes, it's at the center of the image and since the image is really long, we need to scroll to the middle of the image to see it.

@dannymcclain

dannymcclain commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

*Why is the receipt preview area so massive? I thought it would only ever be as tall as the viewport available.

This is an existing behavior. Maybe it was intentionally designed like that?

This doesn't sound right to me. @Expensify/design what do y'all think? (Even so, this sounds like a separate issue from the one we're focusing on here.)

@shawnborton

Copy link
Copy Markdown
Contributor

Hmm I think we did design it that way at one point (to cap the max-height) but then we got some weirdness with how to display the receipt in terms of using something similar to background: cover, etc. I guess the reality is that most receipts are pretty standard size and we don't run into this often. I'd be down to do nothing here or follow up and do something with these edge cases if you feel strongly!

Everything else seems to be looking nice to me though.

@dubielzyk-expensify

Copy link
Copy Markdown
Contributor

I'd be down to do nothing here or follow up and do something with these edge cases if you feel strongly!

No strong feelings. Fine with following up. Might wanna do some prototyping for this and have clearer instructions 👍

@bernhardoj

Copy link
Copy Markdown
Contributor Author

Cool. @mountiny, so should we go with that approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants