[HOLD for payment 2024-03-14] [$250] iOS - Image - Maximum zoom is applied after zooming out

[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.44-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4345659
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:

Pre-requisite: the user must be logged in

1. Go to any chat
2. Send an image
3. Tap on the image preview
4. Zoom in until you can not zoom more
5. Double tap on the image to zoom out
6. Try to zoom in with 2 fingers

Expected Result:

The image should be zoomed in a bit

Actual Result:

The maximum zoom is applied to the image

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

Bug6393482_1708996519041.Rpreplay_Final1708986936.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013893bd4f5612ab60
• Upwork Job ID: 1762299502083756032
• Last Price Increase: 2024-02-27
• Automatic offers:
• paultsimura | Reviewer | 0
• andreasnw | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~013893bd4f5612ab60

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Auto-assign attempt failed, all eligible assignees are OOO.

[lanitochka17]

We think that this bug might ### be related to #vip-vsp
CC @quinthar

[youssef-lr]

cc @andreasnw @eVoloshchak I think this is related to #35931

[andreasnw]

hi @youssef-lr ,

I don't believe this bug is a regression as it can also be seen in the older version v1.4.43-2 (before my pr).

simulator-screen-recording-iphone-14-2024-02-27-at-121536_FOn1Ddi4.mp4

This seems to be a separate issue. We can fix it by setting pinchScale.value to 1 whenever a double tap occurs. I'm happy to take care of this if you assign it to me.

POC after resetting pinchScale.value to 1:

simulator-screen-recording-iphone-14-2024-02-27-at-121828_hmVPSRAM.mp4

[youssef-lr]

Hmm seems like this is not a deploy blocker then, I can reproduce on the previous version which is in production now. I'll quickly try to find the offending PR and get back to you @andreasnw.

[shahinyan11]

Proposal

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

iOS - Image - Maximum zoom is applied after zooming out

What is the root cause of that problem?

We do not reset pinchScale.value after double tap in reset function . Which causes setting the maximum value for newZoomScale on this line

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

1. Add following code pinchScale.value = withSpring(1, SPRING_CONFIG); here .
2. Add following code pinchScale.value = 1 here

What alternative solutions did you explore? (Optional)

[paultsimura]

The proposal by @shahinyan11 looks good to me.
🎀👀🎀 C+ reviewed

P.S. Let's add pinchScale.value = 1 here as well for the non-animated reset:

App/src/components/MultiGestureCanvas/index.tsx

Lines 131 to 137 in 04c8305

	offsetX.value = 0;
	offsetY.value = 0;
	panTranslateX.value = 0;
	panTranslateY.value = 0;
	pinchTranslateX.value = 0;
	pinchTranslateY.value = 0;
	zoomScale.value = 1;

[melvin-bot]

Current assignee @youssef-lr is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[shahinyan11]

P.S. Let's add pinchScale.value = 1 here as well for the non-animated reset:

I was updating my proposal to add this change too when I saw your comment

[andreasnw]

Wait, didn't @youssef-lr said hes going back to me @paultsimura. Even @shahinyan11 proposal uses the exact same solution as mine? 🙃

[youssef-lr]

Sorry folks but @andreasnw has still described the root cause and offered the correct fix in their comment. It was a reply to me so it didn't have the proper format. I think it's fair to have this assigned to them still. Also, given the simple change, I'm adjusting the price slightly.

[melvin-bot]

Upwork job price has been updated to $250

[melvin-bot]

📣 @paultsimura 🎉 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]

📣 @andreasnw 🎉 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 📖

[shahinyan11]

Sorry folks but @andreasnw has still described the root cause and offered the correct fix in their comment. It was a reply to me so it didn't have the proper format. I think it's fair to have this assigned to them still. Also, given the simple change, I'm adjusting the price slightly.

@youssef-lr I criticize your decision. I think it shouldn't matter there is a simple or complex change. There is a rule and I think we should follow it. As an contributor, I have not seen any proposal with clearly marked changes to understand is it worth to waste additional time to investigate and create proposal.
There are a lot of places where the pinchScale.value used. Where is the guarantee that @andreasnw proposed the change in exactly the right place ?

[andreasnw]

PR ready for review #37692

[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.48-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: resetting pinchScale.value to 1 when double tap performed #37692

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

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

• @paultsimura requires payment automatic offer (Reviewer)
• @andreasnw requires payment automatic offer (Contributor)

[paultsimura]

• The PR that introduced the bug has been identified. Link to the PR: Simplify native attachment gallery paging context and improve code #36534
• 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: https://github.com/Expensify/App/pull/36534/files#r1524549613
• 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: N/A
• 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.

Regression Test Proposal

iOS / Android:

1. Send an image attachment
2. Open the attachment and zoom in to the max using the pinch gesture
3. Double-tap the image to zoom out
4. Start zooming in again using the pinch gesture
5. Verify the image is being zoomed gradually

Do we agree 👍 or 👎

[melvin-bot]

Issue is ready for payment but no BZ is assigned. @slafortune you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

[slafortune]

Testrail added here - https://github.com/Expensify/Expensify/issues/379000
@paultsimura - Paid
@andreasnw - Paid