Display images with new image viewer (UIScrollView version)

[joshuatbrown]

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

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-08-20.at.11.33.49.mp4

Mac	iPad

Notes

#1407 and #1408 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.

Update: Martin and I like the approach in this PR -- the UIScrollView -- since it feels the way we'd expect on both iPhone and Mac. I'd still love to get your thoughts, @mplorentz.

Pros for this approach

1. Dragging around on the image feels great. The image moves around quickly and predictably.
2. The implementation is fairly straightforward, though it does require the ZoomableContainer and all its subtypes.
3. Pinching to zoom is implemented and works as expected! It's not a requirement now, but it's nice to have.
4. Double-tapping to zoom will zoom in on the location you tapped. For the other implementations, double-tapping anywhere just zooms to the middle of the image.
5. Works as expected on Mac. You can swipe on a mouse or trackpad, or click and drag.

Cons for this approach

1. Swipe down to dismiss only works when zoomed out. Maybe this can be solved.
2. On a portrait image, if you double-tap to zoom, you can scroll past the top or bottom of the image. Here's an example:

[mplorentz]

I like this approach best as well! I didn't review the code in depth yet, but as a user this feels the best to me. Here are a few thoughts I had which playing with it. Do with them what you will:

• I love that this has pinch-to-zoom
• I wish the background around the image was black, not our app bg color. Black is more neutral and doesn't clash with the colors in the image the way dark purple does.
• The Mac/iPad view feels clunky to me. There are two sets of margins: one around the modal presentation and another around the image. Also there is no presentation animation, and personally I miss the swipe to dismiss even when I'm using a mouse. I think it would feel better to do a push transition rather than a modal presentation for the bigger size classes.

[joshuatbrown]

• I wish the background around the image was black, not our app bg color. Black is more neutral and doesn't clash with the colors in the image the way dark purple does.

100% agreed. I followed the design, which was actually a new purple that I had to add, but I like black better and can just do that.

• The Mac/iPad view feels clunky to me. There are two sets of margins: one around the modal presentation and another around the image. Also there is no presentation animation, and personally I miss the swipe to dismiss even when I'm using a mouse. I think it would feel better to do a push transition rather than a modal presentation for the bigger size classes.

Agreed. We can eliminate a margin on Mac/iPad and fix the animation if we use a fullScreenCover instead of a sheet, but we'll lose swipe to dismiss on iPad. If we want to make that tradeoff, that's ok with me, but it'll move us further from the stated requirements so we may want to check to be sure. Personally, I don't like the idea of a push rather than a modal for this -- I like the way Ivory works on Mac, for example, but I realize this may just be personal preference. Here's what Ivory looks like:

[martindsq]

@joshuatbrown @mplorentz I'm fine with changing the background color to black. But I'd like to see that discussion happening in #design or in private conversation with our designer before moving on with that change.

[martindsq]

I like this PR very much, it feels good to interact with it. It now looks even better after controlling how much the user can pan after zooming in. I'm marking this as request changes because I just want to see that check-in with our designer before merging this.

[joshuatbrown]

@martindsq Linda and Sebastian both agreed to a black background in Slack.