Defer image loading and use original receipt image - #97313
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🚧 mountiny 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! 🧪🧪
|
|
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 |
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. |
|
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. |
|
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 |
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) |
|
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 |
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. App/src/components/ImageWithLoading.tsx Lines 138 to 143 in 1c596a1 We can simplify it and shows whenever the higher-res image is loading. Here is how it looks (with the dim): web.mp4 |
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. |
This is an existing behavior. Maybe it was intentionally designed like that?
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. |
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.) |
|
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. |
No strong feelings. Fine with following up. Might wanna do some prototyping for this and have clearer instructions 👍 |
Explanation of Change
Fixed Issues
$ #96999
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 stepssectionAvatar, 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