Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dialog] Unable to pinch zoom using trackpad #1554

Open
chesterhow opened this issue Jul 22, 2022 · 7 comments · May be fixed by #3127
Open

[Dialog] Unable to pinch zoom using trackpad #1554

chesterhow opened this issue Jul 22, 2022 · 7 comments · May be fixed by #3127

Comments

@chesterhow
Copy link

chesterhow commented Jul 22, 2022

Bug report

Congrats on the 1.0 release! I see that the allowPinchZoom prop has been removed and now defaults to true. However, there is still an issue with pinch zooming: it is not supported on trackpads.

I understand that allowPinchZoom is a prop for the underlying react-remove-scroll library, and I filed an issue with them a couple of weeks back (theKashey/react-remove-scroll#75). The maintainer has acknowledged the issue but it doesn't seem like there will be a fix anytime soon.

Current Behavior

When a dialog is open (or any primitive that uses react-remove-scroll), I am unable to pinch-and-zoom the webpage using the trackpad.

Expected behavior

I should still be able to pinch-and-zoom using the webpage when the dialog is open.

Reproducible example

This is reproducible in the dialog example in the Radix docs: https://www.radix-ui.com/docs/primitives/components/dialog

Additional context

I am using Dialog to render a full-screen modal of sorts which displays an image. An expected common behavior here would be that I am able to pinch-and-zoom on the image.

@alii
Copy link

alii commented Dec 16, 2022

Able to reproduce this using the Context Menu component in @radix-ui/[email protected]🙂

@JasperAlexander
Copy link

Is there already a solution to this problem?

@benoitgrelard
Copy link
Collaborator

Hi @chesterhow,

Yeah this is strange, we specifically enabled pinchZooming on react-remove-scroll:

<RemoveScroll as={Slot} allowPinchZoom shards={[context.contentRef]}>

We made this change in this PR: #1514 for exactly the reasons you are talking about.

Unfortunately, if the fix is not made upstream, there isn't much we can do at the moment. We did experiment with ditching react-remove-scroll but haven't had the bandwidth to pursue this further.

I would suggest you all continue working with the upstream library to push for a fix which we can then integrate.

🙏

@origamisage
Copy link

origamisage commented Oct 12, 2023

I just ran into this today, the way I got it working is a bit hacky:
Idea is to set body's overflow to hidden when dialog is opened to prevent scrolling behavior of content behind the dialog and then set it back to initial once closed.

  • Change Dialog.Overlay into a normal div
  • Add this to Dialog.Root
onOpenChange={() => {
        if (open) {
          document.body.style.overflow = "initial"
        } else {
          document.body.style.overflow = "hidden"
        }
...
}}

Note: since the state changes asynchronously I put the if statement with a "counter-intuitive" condition before applying setOpen(!open)

@tangye1234
Copy link

I use pnpm, so I can use overrides and pnpm patch to resolve the issue by patch react-remove-scroll.

It is a neat resolution that everyone can take without breaking structures of dependencies.

  1. Make sure you use pnpm.
  2. In your root package.json, override react-remove-scroll package to the latest version 2.5.7:
{
  ...
  "resolutions": {
    "react-remove-scroll": "2.5.7"
  }
}
  1. execute pnpm install to make use of only one version of react-remove-scroll. If you didn't install react-remove-scroll at any place, nor any workspaces yet, just install this package to the workspace that depends on radixui dialog / shadcn before you override package.
  2. execute pnpm patch [email protected], it will tell you to move on to a tmp directory to fix this lib.
  3. find the code at dist/esXXX/SideEffect.js(XXX means multiple directories es5 / es2015/es2019):
    https://github.com/theKashey/react-remove-scroll/blob/bd8d6433319b67eabf350d7ec2fa4f17627af417/src/SideEffect.tsx#L57

and change this condition to

if (('touches' in event && event.touches.length === 2) || (event.type === 'wheel' && event.ctrlKey)) {
...
}

This is because it dose not support pinch zoom action on magic track pad, which event is wheel + ctrlKey
6. just do as it tells you, execute pnpm patch-commit XXXXX

now you can pinch and zoom on the dialog.

@dzucconi
Copy link

A related problem is that this disables other gestures like swipe back, so if you, for instance, have a route that is opened in a dialog, that breaks the gesture for back.

@bon10
Copy link

bon10 commented Sep 17, 2024

@tangye1234

Thank you for your solution!
I've created react-remove-scroll PR with your solution, and it's already been merged and v2.6.0 released.

Therefore, this issue should be solved if the package.json of radix-ui can be updated.
I will create a new Pull Request.

@bon10 bon10 linked a pull request Sep 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants