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

Fixes for the zoom behaviour and overlay height in iOS Safari #434

Merged

Conversation

eych
Copy link
Contributor

@eych eych commented Aug 1, 2023

Description

Address a few issues I've discovered when using the library in my project.

  1. Implements the style fix for On iOS Safari overlay doesn't cover all of the screen's height because of height: 100svh style #433 as suggested by @rpearce
  2. Allows to zoom on the already zoomed image with ctrl + wheel and pinch to zoom gestures on desktop
  3. Triggers unzoom on touchmove instead of touchend. That is much closer to the behaviour on Medium (https://medium.design/image-zoom-on-medium-24d146fc0c20). Handling unzoom on touchend leads to nothing happening with the image while I touch-scroll until I lift my finger up.

Plus, renames master to main in CONTRIBUTING.md; otherwise, the instructions don't fully work.

Testing

  1. Clone this project and go to the project directory
  2. Run npm ci && npm start
  3. Go to http://localhost:6006 (or equivalent local IP if you use a mobile device as suggested by Storybook on start)
  4. See if the behaviour described above is working as intended

@rpearce
Copy link
Owner

rpearce commented Aug 1, 2023

Wow! This PR looks like a great addition. Thank you for taking the time to do this PR and update the docs; it really helps.

I need to test it across my devices, and I will do so as soon as I can (probably tonight).

@rpearce rpearce self-requested a review August 2, 2023 01:05
@rpearce rpearce self-assigned this Aug 2, 2023
Copy link
Owner

@rpearce rpearce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it works well, and you've now got me interested in making the mobile pinch-to-zoom experience better. Thanks for your contribution!

@rpearce rpearce merged commit 336416f into rpearce:main Aug 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants