[$1000] 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
• [x] 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

https://user-images.githubusercontent.com/43996225/231004704-3ffa4eb5-bcb9-456f-83c7-555e63e266b5.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-04-27

[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:

[fedirjh]

@Armedin I kindly request that you follow our proposal template when submitting proposals. The template can be found here : PROPOSAL TEMPLATE.

[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 https://github.com/Expensify/react-native-web/commit/ccfd936f274ca2105745f9edbbb4aad80725e181 (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.

https://user-images.githubusercontent.com/36869046/232587056-7aeaba5c-1445-4f56-a674-807358c7e235.mov

[tgolen]

Great, thank you! That is an excellent explanation. Sounds like the next step for this is still waiting on an answer for https://github.com/Expensify/App/issues/17246#issuecomment-1510892091

[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.

[fedirjh]

@alitoshmatov I see your point. If I understand correctly, you're referring to the dimensions of the documentElement, because we're currently passing the visualViewport dimensions as the window dimensions.

I am suggestion to pass visualViewport width and height seperately along window, and screen values:

What if we pass the height of the visualViewport and the width of the documentElement ? Would that address both issues ?

[alitoshmatov]

@fedirjh Yes in previous comment i meant window=documentElement, screen, and visualViewport 3 seperate fields.

What if we pass the height of the visualViewport and the width of the documentElement ? Would that address both issues ?

Well it seems like there are some components which is styled based on window.height and they are adopting to smaller screen when zoomed, which is not desirable.

[fedirjh]

Yes in previous comment i meant window=documentElement, screen, and visualViewport 3 seperate fields.

@alitoshmatov Thank you for the update. However, I believe this solution would contradict the philosophy of platform-independent code. In my opinion, any solution that deviates from reverting this commit https://github.com/Expensify/react-native-web/commit/ccfd936f274ca2105745f9edbbb4aad80725e181 would be a workaround.

[alitoshmatov]

You are right, but I think https://github.com/Expensify/react-native-web/commit/ccfd936f274ca2105745f9edbbb4aad80725e181 itself is a workaround solution in expense of this issue. We should really reconsider this solution and https://github.com/Expensify/App/pull/14392 .

[tgolen]

I agree with you both. For this issue, let's make sure we revert the previous workaround and have a solution which fixes both problems 👍

[MelvinBot]

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

[alexpensify]

@fedirjh - are we good with the proposal now and can we carry on to the next steps? Thanks for the update.

[fedirjh]

@alexpensify We are still waiting for proposal that fixes both problems, we agreed that we should revert the previous workaround and have a solution which fixes https://github.com/Expensify/App/pull/14392

[alexpensify]

Whoops, I jumped the start line here. Thank you for clarifying!

[MelvinBot]

@tgolen @alexpensify @fedirjh this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[tgolen]

Still waiting for proposals