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

Allow mobile pinch to zoom of website on carousel. #1021

Closed
wants to merge 5 commits into from

Conversation

minervabot
Copy link

Description

Widgets that disrupt ordinary touch gestures on a mobile device are frustrating. For example, users may want to zoom in on a slide slightly or correct an already zoomed-in state using gestures on top of the carousel component. Unfortunately, the carousel acts as a dead zone, neutralizing the pinch-to-zoom gesture if even one finger of the gesture is inside the component.

This patch causes the carousel to only react to single touch gestures, which is essential so pinch to zoom will not simultaneously drag the slide. It also adds pinch-zoom to the touch-action style, enabling global pinch zooming.

Fixes #1020

Type of Change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Connect an iPhone 13 Pro Max to the development web server. Used manual testing for the following:

  • Verified pinch to zoom works with one or both pinching fingers inside the carousel.
  • Verified swiping with two fingers does not work in the carousel.
  • Verified adding and removing a touch in addition to the initial panning does not disrupt the existing swipe gesture inside and outside the carousel.
  • Verified removing all fingers from an incomplete swipe causes the drag to end and the carousel to snap back.
  • Verified removing all fingers from the carousel with one finger still touching the outside, causes the drag to reset.

Ideally, this would include a Cypress test, but I need help figuring out how to simulate compelling touch actions there.

Checklist

  • My code follows the style guidelines of this project (I have run pnpm run lint)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (I have run pnpm run test:ci-with-server/pnpm run test)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have recorded any user-facing fixes or changes with pnpm changeset.
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@vercel
Copy link

vercel bot commented May 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nuka-carousel-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2023 1:42am

@minervabot minervabot changed the title Allow mobile pinch to zoom of website to original on carousel. Allow mobile pinch to zoom of website on carousel. May 20, 2023
@leventdeniz
Copy link

@gksander any update on this?

@minervabot
Copy link
Author

@leventdeniz I ended up using embla-carousel-react which works alright with pinch zoom. This PR does work as well!

@carbonrobot
Copy link
Contributor

Closing as the code is no longer valid in the new codebase, however, we are using the great tests written here as a baseline to ensure we test this and if needed fix this in v8 as soon as possible.

@carbonrobot carbonrobot closed this Apr 5, 2024
@minervabot
Copy link
Author

@carbonrobot I believe the tests were never finished so I would suggest writing new ones. Nobody seemed particularly interested in the PR for me to waste the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pinch to zoom gesture blocked on iPhone
3 participants