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

Support pinch-zoom #645

Closed
3 of 9 tasks
kj opened this issue Dec 3, 2023 · 6 comments
Closed
3 of 9 tasks

Support pinch-zoom #645

kj opened this issue Dec 3, 2023 · 6 comments
Labels
feature request New feature or request resolved This issue is resolved

Comments

@kj
Copy link

kj commented Dec 3, 2023

Feature request is related to

  • embla-carousel (core package)
  • embla-carousel-react
  • embla-carousel-vue
  • embla-carousel-svelte
  • embla-carousel-autoplay
  • embla-carousel-auto-height
  • embla-carousel-class-names
  • embla-carousel-docs (documentation)
  • embla-carousel-docs (generator)

Is your feature request related to an issue?

Pinch zoom isn't working for me on Android (at least in the examples on the website). I noticed in another issue #480 (comment) someone commented that it is working under iOS, so this might only affect Android, although I can't be sure.

Describe the solution you'd like

I'd like to be able to use pinch zoom to zoom in to image details in a slider. I made a PR for Keen-Slider to get this working, although there's not much development activity over there so the PR hasn't gone anywhere.

The PR just checks whether there are two or more fingers touching the surface before dragging, and adds pinch-zoom to touch-action.

@kj kj added the feature request New feature or request label Dec 3, 2023
@davidjerleke
Copy link
Owner

davidjerleke commented Dec 3, 2023

Thank you for your feature request @kj. Embla doesn’t prevent this but the examples all have touch-action set to pan-y for horizontal carousels and pan-x for vertical ones. So the pinch zoom declaration is missing.

This might only be a matter of finding the touch-action declaration in the CSS and adding pinch-zoom to it like so:

.classname {
  touch-action: pan-y pinch-zoom;
}

Let me know how it goes.

Best,
David

@kj
Copy link
Author

kj commented Dec 3, 2023

Hi David, thanks for your quick response. I completely forgot to mention that I was testing on Firefox for Android. I just tried Chrome (actually Cromite) and it works there, even on the example page, which is great! I did actually try earlier with touch-action: pan-y pinch-zoom on the examples page, but that didn't work for me in Firefox, so Firefox must behave differently somehow. I haven't looked at the Embla code too closely, but perhaps it just needs something like that extra check in my Keen-Slider PR to work in Firefox for Android too?

@davidjerleke
Copy link
Owner

@kj I don’t know anything about Keen-Slider but you can add a callback to the Embla watchDrag option to customize the behavior and intercept the drag behavior. In your case, you need to do the following:

  1. The second callback argument is the raw event so check if it’s a touch event. Return true if it’s not a touch event to allow Embla to continue with its default behavior.
  2. Return false if touches has two fingers (it’s a pinch zoom) and otherwise return true to allow Embla to continue with its default behavior.

Best,
David

@kj
Copy link
Author

kj commented Dec 3, 2023

Oh that does sound like it should work. I'll try it out tomorrow probably and report back.

@davidjerleke
Copy link
Owner

@kj do you intend to pursue this?

@kj
Copy link
Author

kj commented Dec 6, 2023

Sorry @davidjerleke I didn't forget about this, I've just been a bit distracted. I tried with a minimal setup, basically this:

<script defer type="module">
  import EmblaCarousel from './embla-carousel.esm.js';

  EmblaCarousel(document.querySelector('.embla'));
</script>

And pinch zoom does work in Firefox for Android with touch-action: pan-y pinch-zoom, or when touch-action is unset. Only with touch-action: pan-y; does it not work. So I'm not sure why it wasn't working for the website examples when I added pinch-zoom, but maybe I missed something else. Either way, it's clearly working fine in the typical case without any workarounds (like the touch count I had to add with Keen-Slider), so I'll close this.

It's great to finally have a lightweight carousel that just does these things right!

@kj kj closed this as completed Dec 6, 2023
@davidjerleke davidjerleke added not planned Won't be investigated unless it gets lots of traction resolved This issue is resolved and removed not planned Won't be investigated unless it gets lots of traction labels Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request resolved This issue is resolved
Projects
None yet
Development

No branches or pull requests

2 participants