[HOLD App/issues/16660] [$2000] Pinch to zoom on a Macbook browser dynamically changes the app's view and components

[kavimuru]

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

---

Action Performed:

1. When a chat is open, pinch to zoom on either the LHN or the details tab.
   1 (alternate). On a fresh sign in page, zoom in on any part of the site with pinch to zoom.

Expected Result

The page doesn't reorganize or shfit as you zoom in this way, this should be equivalent to magnification and shouldn't change the content ordering or structure of the app.

Actual Result

App re-renders into different various forms as you zoom in breaking your ability to zoom into

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.98-1
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-04-10.at.2.53.22.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @johnmlee101
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681152993219619

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01caf96ff9d8613c10
• Upwork Job ID: 1646608088499531776
• Last Price Increase: 2023-05-02

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[alexpensify]

I'll try to test soon.

[alexpensify]

I tested here is my page before zoom

Here is my page when zooming

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01caf96ff9d8613c10

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @tgolen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Armedin]

This issue seems to happen whenever you zoom in on a web browser. The dimensionsEventListener is registering zoom events as a window dimension change. For instance, if I zoom 3 times and print the window dimension:

[alitoshmatov]

Proposal

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

Pinch to zoom on a Macbook browser dynamically changes the app's view and components

What is the root cause of that problem?

This is caused by how react-native-web handling dimensions.
https://github.com/Expensify/react-native-web/blob/bb2f765614628eeccdd9d2850cb287557a92b6c9/packages/react-native-web/src/exports/Dimensions/index.js#L63-L71
Above you can see it is setting width and height based on window.visualViewport which changes when you zoom.

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

There is an option to make an upstream fix in react-native-web to set window height and width to documentElement.clientHeight which won't change when you zoom and feels more natural to set this measures to window object in dimensions.

What alternative solutions did you explore? (Optional)

[alexpensify]

@fedirjh - early this week, can you share feedback on this new proposal? Thanks!

[fedirjh]

Thank you for sharing your proposal, @alitoshmatov. I agree with your RCA. However, I'd like to point out that the use of viewport dimensions was intentionally added in this commit Expensify/react-native-web@ccfd936 (PR #14392) to address this issue #11463. It seems that you are suggesting reverting these changes, but I don't think that would be ideal unless you also provide a solution for #11463.

[alitoshmatov]

@fedirjh Is it possible that we return one more field from dimensions from react native web, possibly separate viewport and window objects and use them respectively

[fedirjh]

@alitoshmatov Can you please elaborate more ? How would that fix the issue ?

[tgolen]

Thanks for the research into this issue. One thing that isn't clear to me is why this is the expected behavior. Can anyone add some details about that? Is it because that's what happens on normal websites when you pinch to zoom?

[fedirjh]

The portion of the viewport that is currently visible is called the visual viewport. This can be smaller than the layout viewport, such as when the user has pinched-zoomed. The layout viewport remains the same, but the visual viewport became smaller.

@tgolen The MDN documentation on viewport states that the layout viewport remains the same during a pinched zoom gesture, while the visual viewport becomes smaller. However, this commit Expensify/react-native-web@ccfd936 breaks this behavior.

The reason why zooming affects the visualViewport is because the visualViewport represents the visible portion of the webpage as seen by the user in the viewport. When the user pinch to zoom, the visible portion of the webpage changes, and so the visualViewport needs to be updated to reflect the new dimensions and position of the visible viewport.

This behavior is expected because it reflects the user's interaction with the webpage. When the user pinch to zoom on a webpage, they are changing the scale of the content to better suit their needs, such as making the text easier to read or viewing an image in more detail. They do not expect the page to change its layout when they pinch to zoom, as the position of the content may change or even disappear, resulting in a bad UX.

that's what happens on normal websites when you pinch to zoom?

That's correct, even oldDot has this behavior where the website's layout is maintained when you pinch to zoom.

Screen.Recording.2023-04-17.at.7.39.08.PM.mov

[tgolen]

Great, thank you! That is an excellent explanation. Sounds like the next step for this is still waiting on an answer for #17246 (comment)

[alitoshmatov]

@fedirjh I am suggestion to pass visualViewport width and height seperately along window, and screen values:
https://github.com/Expensify/react-native-web/blob/ccfd936f274ca2105745f9edbbb4aad80725e181/packages/react-native-web/src/exports/Dimensions/index.js#L63-L86

And then we can modify here to use visualViewport width:
https://github.com/rawalyogendra/App/blob/6a1ae162886703a772d27081eff4c99a49dbf971/src/components/ScreenWrapper/index.js#L92

But there is a concern which this solution might break philosophy of platform independent code. Since this code visualViewport will only be available in web platforms through react-native-web.

[alexpensify]

Weekly update: On Hold

[alexpensify]

Weekly Update: There should be a PR soon for the GH that put this one on hold

[alexpensify]

Weekly update: On Hold

[alexpensify]

Weekly update: On Hold but we had some GH movement yesterday. 🤞🏼

[alexpensify]

Weekly update: On Hold

[fedirjh]

RNW upgrade PR has been merged, we can remove the hold once it has been deployed to production.

[alexpensify]

Thank you for that update @fedirjh!

[alexpensify]

@fedirjh - I realized that I don't think the PR you mentioned last week is in this GH. Can you please share the link since the one in the title has many PRs associated with it now? Thanks!

[fedirjh]

@alexpensify This is the PR, it was just deployed to production

• Upgrade to react-native-web v0.19.9 #24482

[alexpensify]

It is time! @tgolen and @fedirjh are you OK for me to take this one off hold and move it back to daily? I'm assuming that we will need updated proposals but keep me posted on the next steps. We can prepare accordingly. Thanks!

[fedirjh]

are you OK for me to take this one off hold and move it back to daily?

I think this should be awaiting for the regression period then payments as the bug was fixed.

I'm assuming that we will need updated proposals but keep me posted on the next steps.

@alexpensify The upgrade PR already includes the fix that was proposed in this issue, the fix was pushed and merged upstream and was released in the RNW v 0.19.

• [Dimensions] consistent measurements with window zoom necolas/react-native-web#2520 (comment)

This video demonstrates that the bug was fixed:

CleanShot.2023-11-15.at.19.56.35.mp4

[alitoshmatov]

#24482 is live, which means we can take this issue off hold, finally 🥳

[alexpensify]

Ok, I chatted with @tgolen. Let's keep this one on hold to clear the regression period.

@fedirjh - Sorry, I'm confused are you suggesting that the PR (#24482) for #16660 fixed this issue too? If yes, then we will need to prepare for partial payment here instead of another fix when the HOLD is removed in this GitHub? I want to confirm the required actions since I will be OOO next week. If there is no new fix for this issue, then can find a replacement to take over the payment process in Upwork. Thanks!

[fedirjh]

Sorry, I'm confused are you suggesting that the PR (#24482) for #16660 fixed this issue too?

@alexpensify We addressed this particular issue upstream, and the corresponding fix was included in the release of RNW v0.19. In PR #24482, we upgraded to RNW v0.19, which inherently incorporates the fix. As a result, no further action is needed at this point.

If yes, then we will need to prepare for partial payment here instead of another fix when the HOLD is removed in this GitHub?

The upstream fix was a part of this issue. The process for this issue was :

1. Open upstream PR and fix the issue: [HOLD App/issues/16660] [$2000] Pinch to zoom on a Macbook browser dynamically changes the app's view and components #17246 (comment)
2. Open same PR in our RNW fork: [HOLD App/issues/16660] [$2000] Pinch to zoom on a Macbook browser dynamically changes the app's view and components #17246 (comment)
3. Open a PR in our App to bump the version of our RNW in E/App.

Steps 1 and 2 were successfully completed. Upon completing the final step, the decision was made to discontinue the RNW fork. Instead, we began utilizing the react-native-web package directly within E/App.This transition was handled in #16660 (PR #/24482).

Given that RNW v0.19 has now been merged into our App, the third step is no longer necessary.

[alexpensify]

Got it, we will close after the regression period and prepare for the payment period. Thanks!

[alexpensify]

Reassigning another team member, I'm going OOO until Monday, November 27, and will take it back if it's still open by my return date.

@laurenreidexpensify - Required action from the team:

There will need to be a payment after the regression period closes while I'm offline next week. I haven't been able to write up a payment summary but will need help with that one before the payment date and then pay everyone via Upwork. Thanks!

[laurenreidexpensify]

Ready for payment, regression period has passed as #24482 has been on staging for a week

[laurenreidexpensify]

Payment Summary:

• Contributor @alitoshmatov - $2000, payment will be in Upwork
• C+ @fedirjh - $2000, payment will be in Upwork

[laurenreidexpensify]

Offers sent in Upwork,

@fedirjh is there a regression test that we need to add for this one?

[alitoshmatov]

Accepted the offer

[laurenreidexpensify]

Payment Summary:

• Contributor @alitoshmatov - $2000, payment issued in Upwork
• C+ @fedirjh - $2000, payment issued in Upwork

[laurenreidexpensify]

Outstanding action: waiting for @fedirjh to confirm if we need a regression test

[fedirjh]

BugZero Checklist:

• Link to the PR: Fix: iOS button obstructed by keyboard #14392
• Link to comment: Fix: iOS button obstructed by keyboard #14392 (comment)
• Link to discussion: N/A, regression test is needed for this bug.
• Determine if we should create a regression test for this bug: Yes

---

Regression Test Proposal

1. Open App
2. Open a report -> Open details page of the report
3. zoom in on any part of the site with a pinch-to-zoom gesture.
4. Verify that app layout style does not change.
5. Verify that the zoom works as expected.

• Do we agree 👍 or 👎

[alexpensify]

Thanks for your help here @laurenreidexpensify to complete the payment process!