[HOLD for payment 2024-03-29] [$500] Android – Image is reduced for a moment when quickly slide forward/back images using arrows

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.35-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4265112
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Launch app
2. Log in
3. Navigate to a conversation that has 5 or more image attachments in it's history
4. Click on one of the attachments to open the preview
5. Click on the forward or back arrow to navigate through the conversation attachments quickly

Expected Result:

The image size is normal when quickly slide forward/back images using the arrows

Actual Result:

The image is reduced for a moment when quickly slide forward/back images using the arrows

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6364145_1706825041063.Attachments.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019fd6243b644a1821
• Upwork Job ID: 1753212539309678592
• Last Price Increase: 2024-02-23
• Automatic offers:
• abdulrahuman5196 | Reviewer | 0
• tienifr | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~019fd6243b644a1821

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

[melvin-bot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[tienifr]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The image is reduced for a moment when quickly slide forward/back images using the arrows

What is the root cause of that problem?

In here and here, we're showing the image even though the size was not determined, we use the DEFAULT_IMAGE_DIMENSION as the default in that case, which leads to the image being small for a short time while it loads, then after the image dimensions are loaded, it will show in full size.

What changes do you think we should make in order to solve the problem?

In here and here, make sure the image is not visible while its size is being loaded, we can use opacity: 0, visibilityHidden or similar. While the image is still loading the dimensions, the LoadingIndicator will show here so the UX is smooth.

So in here

style={[contentSize ?? DEFAULT_IMAGE_DIMENSION, !contentSize && {opacity: 0}]}

here

style={[fallbackSize, !contentSize && {opacity: 0}]}

(There're a couple more conditions to determine the fallbackSize here, if we want to make sure to account for that condition, we can return undefined there, then in here if fallbackSize is undefined, fallback to DEFAULT_IMAGE_DIMENSION , and set {opacity: 0} just like we did above in this case)

If we're worried about the onLoad not being triggered if the image is not visible (although when I test on Android, iOS it works fine), we can use Image.getSize inside Image component for native, to determine the sizes rather than relying on Image's onLoad.

What alternative solutions did you explore? (Optional)

Do not render the image altogether if the size is not determined, meanwhile the Image.getSize can be used to populate the contentSize instead

[abdulrahuman5196]

Will work on review today

[johncschuster]

Bumping for Melvin

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[abdulrahuman5196]

Sorry for the delay. Working on review now

[abdulrahuman5196]

@tienifr From the proposal here #35615 (comment),
To summarize, we show the image before the image size is determined by the system, causing it to show in a fallback size. The suggestion is to not show the image and show the loading indicator.

@johncschuster This expectation seems fine IMO, But I would like to confirm the expectation.

[abdulrahuman5196]

Followup question to @tienifr
Is there a way to re-use the image sizes which we got previously?

[tienifr]

Followup question to @tienifr
Is there a way to re-use the image sizes which we got previously?

@abdulrahuman5196 Yes, we can re-use it by not setting the contentSize (and subsequently the cachedImageDimensions cache value) to undefined here when the current lightbox becomes invisible. But this won't fix the issue fully because the issue will still happen when the image is rendered the first time. I agree though that we can surely consider it as a further optimization.

[johncschuster]

@abdulrahuman5196 / @tienifr To make sure I understand:

The suggestion is to not show the image and show the loading indicator.

Are you suggesting that we shouldn't show an image at all when a user is in image preview mode and swiping through images, and instead show a loading indicator?

[tienifr]

@johncschuster no, let me clarify

When swiping to the next image, currently it will show a small image in the center of the screen, with a loading indicator, then after the image is fully loaded, it will show with correct size.

The suggestion is to not show the small image initially. Instead, it will only show loading indicator while the image is loading (which is brief), then after the image is loaded and the size is known, the full image will be shown with the correct size from the start (rather than showing image with incorrect size first then change to correct size as currently)

[abdulrahuman5196]

Are you suggesting that we shouldn't show an image at all when a user is in image preview mode and swiping through images, and instead show a loading indicator?

Not a suggestion. I would put it as a question instead.

@tienifr proposal here #35615 (comment) targets to do, also explained in detail here #35615 (comment). My question would be, Is it good enough from UX perspective? or should we try out for more optimized fix(which would be reach out for other proposals)?

[melvin-bot]

Current assignee @abdulrahuman5196 is eligible for the External assigner, not assigning anyone new.

[iwiznia]

@tienifr bump

[abdulrahuman5196]

Hmmmm but is the proposal from @tienifr making the LoadingIndicator show? I see you said:

the LoadingIndicator will show here so the UX is smooth.

So, maybe how this works is like this?

* Image is loading (loading indicator shows)

* Image loaded, but dimensions not determined yet (loading indicator is not there and image is neither as its opacity is 0)

* Image loaded, dimensions computed (image shows properly)

@iwiznia I don't exactly understand the question here. But as far as the usecase goes, we will show the image only after the image is loaded and the dimensions are computed. Until then loader will be shown.
Hope it clarifies.

[tienifr]

So, maybe how this works is like this?

@iwiznia There're only 2 phases instead of 3:

1. Image is not fully loaded, dimensions not determined yet (loading indicator shows)
2. Image loaded, dimensions computed (image shows properly)

The only difference compared to the current flow is that in step 1:

• Currently: loading indicator shows and the image with the wrong size is shown briefly
• After the change: only loading indicator will show

[iwiznia]

Currently: loading indicator shows and the image with the wrong size is shown briefly

Ahhh got it!

[melvin-bot]

📣 @abdulrahuman5196 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @tienifr 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[tienifr]

PR ready for review #37739.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.55-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: Image shrinks while loading in carousel #37739

If no regressions arise, payment will be issued on 2024-03-29. 🎊

For reference, here are some details about the assignees on this issue:

• @abdulrahuman5196 requires payment automatic offer (Reviewer)
• @tienifr requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@abdulrahuman5196] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@johncschuster] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[tienifr]

@johncschuster @abdulrahuman5196 Please handle payments. Thanks!

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression.

Determine if we should create a regression test for this bug.

Yes.

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

1. Send many images to a report (about 5)
2. Press an image
3. Quickly press the arrows to navigate between those images in carousel
4. Verify that while image is still loading, we only shows the loading indicator

@johncschuster

[johncschuster]

Payment has been issued and regression test issue has been created. Closing! 🎉