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

Display images with new image viewer (DragGesture version) #1407

Closed
wants to merge 14 commits into from

Conversation

joshuatbrown
Copy link
Contributor

@joshuatbrown joshuatbrown commented Aug 16, 2024

Issues covered

#1164

Description

Displays full-screen images with the new image viewer. The gallery view will be covered in #1177.

How to test

  1. Navigate to a post with an image
  2. Tap on it
  3. You're in the new image viewer!
  4. Double-tap to zoom in
  5. Drag around to pan
  6. Double-tap again to zoom out
  7. Swipe down or tap the x to dismiss

Screenshots/Video

Coming soon!

Notes

#1408 and #1413 are the same thing, but using different implementations. The point of having three draft PRs is to decide on the right approach -- each has its own pros and cons.

Pros for this approach

  1. It meets all the requirements, including double-tap to zoom and swipe to dismiss.
  2. It keeps the image from being overscrolled so you don't see the edges of it like you do in Display images with new image viewer (UIScrollView version) #1413. (See the screenshot in the Cons section.)
  3. It's pure SwiftUI.
  4. No ScrollView or UIScrollView needed.

Cons for this approach

  1. Dragging feels slightly sluggish as compared to the ScrollView and UIScrollView implementations.
  2. Double-tapping to zoom will zoom in on the middle of the image, even if you double-tap the top left or bottom right corner. It doesn't zoom where you tapped.
  3. Pinching to zoom was tricky and didn't behave as expected, so it's not implemented here. It's not a requirement right now, but if it is in the future, this approach may not work as well as the others.
  4. Swiping to pan on Mac doesn't work at all. You need to click and drag.
  5. The implementation of the code that keeps the image from being overscrolled is a little complex, but I could probably make it more readable.

@joshuatbrown joshuatbrown marked this pull request as ready for review August 16, 2024 15:00
@joshuatbrown joshuatbrown changed the title Display images with new image viewer Display images with new image viewer (DragGesture version) Aug 16, 2024
Nos/Views/Components/Images/ImageViewer.swift Outdated Show resolved Hide resolved
Nos/Views/Components/Images/ImageViewer.swift Show resolved Hide resolved
Nos/Views/Components/Images/ImageViewer.swift Outdated Show resolved Hide resolved
Nos/Views/LinkPreview.swift Outdated Show resolved Hide resolved
Nos/Views/LinkPreview.swift Outdated Show resolved Hide resolved
Nos/Views/LinkPreview.swift Outdated Show resolved Hide resolved
Nos/Views/LinkPreview.swift Outdated Show resolved Hide resolved
Nos/Views/Components/Images/ImageViewer.swift Outdated Show resolved Hide resolved
Comment on lines +49 to +50
width: lastOffset.width + value.translation.width * 1.5,
height: lastOffset.height + value.translation.height * 1.5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martindsq you said this feels weird, like the image is moving too much. We can adjust these values and see how it works for you:

Suggested change
width: lastOffset.width + value.translation.width * 1.5,
height: lastOffset.height + value.translation.height * 1.5
width: lastOffset.width + value.translation.width,
height: lastOffset.height + value.translation.height

@joshuatbrown
Copy link
Contributor Author

We're moving forward with #1413.

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.

2 participants