UI/PhotoPreview #330

nagavcindriqim · 2024-09-09T13:19:45Z

A UI component you can use to present an image or set of images in full-screen mode.

Requirements

you should be able to inject an array of images
image preview is presented full-screen
you can zoom and pan the image
you can rotate the screen vertically and/or horizontally
you can navigate to the next screen (if multiple are supplied)
swipe down to dismiss it interactively
written in SwiftUI

Closes #206

Sources/UI/SwiftUI/View Modifiers/PhotoPreviewModifier.swift

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreviewConfiguration.swift

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreviewItem.swift

borut-t

I like it. I would also like to see a preview of it in the Storbybook app. Can you do that within this PR please.

Sources/UI/SwiftUI/View Modifiers/PhotoPreviewModifier.swift

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift

borut-t · 2024-09-23T06:51:49Z

I've previewed the component in the Storybook app. Here is my observation:

there is some flickering going on and it appears to be happening only for the remote images
remote image aspect ratio is not respected
could we go with a TabView and PageTabViewStyle() instead?

Screen.Recording.2024-09-23.at.08.45.45.mov

nagavcindriqim · 2024-09-23T08:19:45Z

I've previewed the component in the Storybook app. Here is my observation:

there is some flickering going on and it appears to be happening only for the remote images

remote image aspect ratio is not respected

could we go with a TabView and PageTabViewStyle() instead?

Screen.Recording.2024-09-23.at.08.45.45.mov

@borut-t I've made some changes to loading remote images. The issue persisted only when Kingfisher was not available.
Unfortunately we can not use TabView since it limits us on the scrolling functionality.

borut-t · 2024-09-23T09:08:05Z

I've previewed the component in the Storybook app. Here is my observation:

there is some flickering going on and it appears to be happening only for the remote images

remote image aspect ratio is not respected

could we go with a TabView and PageTabViewStyle() instead?

Screen.Recording.2024-09-23.at.08.45.45.mov

@borut-t I've made some changes to loading remote images. The issue persisted only when Kingfisher was not available. Unfortunately we can not use TabView since it limits us on the scrolling functionality.

There is still some glitching going on...
https://github.com/user-attachments/assets/3b516597-0d31-42bf-a181-f149d4b0ff20

Regards to the TabView, I think this can be done with it.

nagavcindriqim · 2024-09-23T12:35:23Z

I've previewed the component in the Storybook app. Here is my observation:

there is some flickering going on and it appears to be happening only for the remote images

remote image aspect ratio is not respected

could we go with a TabView and PageTabViewStyle() instead?

Screen.Recording.2024-09-23.at.08.45.45.mov

@borut-t I've made some changes to loading remote images. The issue persisted only when Kingfisher was not available. Unfortunately we can not use TabView since it limits us on the scrolling functionality.

There is still some glitching going on... https://github.com/user-attachments/assets/3b516597-0d31-42bf-a181-f149d4b0ff20

Regards to the TabView, I think this can be done with it.

@borut-t the glitch should now be fixed 🙏

borut-t · 2024-09-23T12:45:13Z

I've previewed the component in the Storybook app. Here is my observation:

there is some flickering going on and it appears to be happening only for the remote images

remote image aspect ratio is not respected

could we go with a TabView and PageTabViewStyle() instead?

Screen.Recording.2024-09-23.at.08.45.45.mov

@borut-t I've made some changes to loading remote images. The issue persisted only when Kingfisher was not available. Unfortunately we can not use TabView since it limits us on the scrolling functionality.

There is still some glitching going on... https://github.com/user-attachments/assets/3b516597-0d31-42bf-a181-f149d4b0ff20
Regards to the TabView, I think this can be done with it.

@borut-t the glitch should now be fixed 🙏

And they truly are. Two things:

progress loader looks a bit funny, can we use the default one?
i can zoom out the image too far. Can we limit until the image fit's the screen? (e.g., it should not be smaller than the screen width/height)

nagavcindriqim · 2024-09-23T13:45:29Z

I've previewed the component in the Storybook app. Here is my observation:

there is some flickering going on and it appears to be happening only for the remote images

remote image aspect ratio is not respected

could we go with a TabView and PageTabViewStyle() instead?

Screen.Recording.2024-09-23.at.08.45.45.mov

@borut-t I've made some changes to loading remote images. The issue persisted only when Kingfisher was not available. Unfortunately we can not use TabView since it limits us on the scrolling functionality.

There is still some glitching going on... https://github.com/user-attachments/assets/3b516597-0d31-42bf-a181-f149d4b0ff20
Regards to the TabView, I think this can be done with it.

@borut-t the glitch should now be fixed 🙏

And they truly are. Two things:

progress loader looks a bit funny, can we use the default one?

i can zoom out the image too far. Can we limit until the image fit's the screen? (e.g., it should not be smaller than the screen width/height)

@borut-t we can't use the default one since we are using .drawingGroup() modifier.
zooming out is now limited to fit the screen 👌

dejanskledar · 2025-02-24T10:22:59Z

@nagavcindriqim @borut-t can we wrap this up? Any major issues still present?

markom-povio

Looking good! Can you add some more documentation in the code (in the Configuration, for example)?

nagavcindriqim · 2025-02-24T20:20:11Z

@nagavcindriqim @borut-t can we wrap this up? Any major issues still present?

@dejanskledar major issues should be fixed with this commit 👌

borut-t

Really nice progress 👌

I've added a few comments about the code itself.
And have also tested out the implementation on the real device and it still feels a bit clunky when zooming in. Especially when you zoom-in and out and want to continue scrolling.

I would de-prioritise this ticket for now and we can come back latter...

borut-t · 2025-02-25T13:36:34Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift

+
+// MARK: - ViewBuilders
+@available(iOS 16.0, *)
+extension PhotoPreview {


We don't want builders to be exposed, do we?

borut-t · 2025-02-25T13:36:52Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift

+
+// MARK: - Helpers
+@available(iOS 16.0, *)
+extension PhotoPreview {


Can we tighten access?

borut-t · 2025-02-25T13:37:06Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift

+
+// MARK: - Gestures
+@available(iOS 16.0, *)
+extension PhotoPreview {


Private access?

borut-t · 2025-02-25T13:37:28Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview+ItemView.swift

+#if canImport(Kingfisher)
+import Kingfisher
+#endif


Let's remove this in favour or RemoteImage.

borut-t · 2025-02-25T13:37:47Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview+ItemView.swift

+  var remoteImageView: some View {
+    #if canImport(Kingfisher)
+    KFImage(item.url)
+      .resizable()
+      .placeholder {
+        (item.placeholder ?? Image(uiImage: UIImage()))
+          .resizable()
+      }
+      .scaledToFit()
+    #else
+    AsyncImage(url: item.url) { image in
+      image
+        .resizable()
+        .scaledToFit()
+    } placeholder: {
+      ProgressView()
+        .tint(.white)
+        .controlSize(.large)
+    }
+#endif
+  }


Can we replace with RemoteImage?

borut-t · 2025-02-25T13:38:00Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview+ItemView.swift

+
+// MARK: - Helper methods
+@available(iOS 16.0, *)
+extension PhotoPreview.ItemView {


Private access?

borut-t · 2025-02-25T13:38:09Z

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview+ItemView.swift

+
+// MARK: - Gestures
+@available(iOS 16.0, *)
+extension PhotoPreview.ItemView {


Private access?

nagavcindriqim added the needs review label Sep 9, 2024

nagavcindriqim self-assigned this Sep 9, 2024

nagavcindriqim requested a review from a team September 9, 2024 13:20

tonikocjan reviewed Sep 10, 2024

View reviewed changes

nagavcindriqim requested a review from tonikocjan September 10, 2024 07:39

borut-t requested changes Sep 18, 2024

View reviewed changes

Sources/UI/SwiftUI/View Modifiers/PhotoPreviewModifier.swift Outdated Show resolved Hide resolved

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift Outdated Show resolved Hide resolved

Sources/UI/SwiftUI/Views/PhotoPreview/PhotoPreview.swift Show resolved Hide resolved

borut-t added the requested changes label Sep 18, 2024

nagavcindriqim requested a review from borut-t September 19, 2024 11:54

nagavcindriqim force-pushed the feature/photo-preview branch 2 times, most recently from 6f1017f to 960df31 Compare September 26, 2024 11:04

nagavcindriqim added work in progress and removed needs review labels Sep 26, 2024

markom-povio approved these changes Feb 24, 2025

View reviewed changes

nagavcindriqim added 10 commits February 24, 2025 20:52

Model.

58832c7

Introduce PhotoPReviewItemView.

f09651e

Introduce PhotoPreview.

b1e939d

Swipe down to dismiss.

8fb1630

Modifier.

2bc6c0f

Ensure new index within bounds.

45fed44

Limit zoom in.

64fe166

Animate vertical out of bounds offset.

45d89f7

Scale to fit remote image.

c579015

Configuration.

22e3bd6

nagavcindriqim added 20 commits February 24, 2025 20:52

Dismiss button.

513349c

Background opacity.

e4e6e7d

README.

b6ee364

Address PR comments.

52ca616

Support from iOS 15.

f701d4d

Storybook.

b3c1647

Introduce PhotoPreviewImageLoader.

343c73f

Load remote images.

323a3a0

Drawing group.

c7b9821

Show loader.

54cf561

Drag outside image bounds.

5acfb14

Disable zooming out.

f87b186

Animate loading.

5a248d0

Files under PhotoPreview namespace.

15aa2d9

Clear presentation background.

c86ed47

Hide close button when dismissing.

91480d7

File naming.

853e5f6

Fix drag gesture.

43ef534

Switch to TabView mid work commit.

cf1afe2

Fix gestures & cleanup.

bf622d1

nagavcindriqim force-pushed the feature/photo-preview branch from b24be4f to bf622d1 Compare February 24, 2025 20:17

nagavcindriqim added needs review and removed work in progress labels Feb 24, 2025

borut-t reviewed Feb 25, 2025

View reviewed changes

borut-t added work in progress and removed requested changes needs review labels Feb 25, 2025

borut-t unassigned nagavcindriqim Feb 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI/PhotoPreview #330

UI/PhotoPreview #330

nagavcindriqim commented Sep 9, 2024 •

edited

Loading

borut-t left a comment

borut-t commented Sep 23, 2024

nagavcindriqim commented Sep 23, 2024

borut-t commented Sep 23, 2024

nagavcindriqim commented Sep 23, 2024

borut-t commented Sep 23, 2024

nagavcindriqim commented Sep 23, 2024

dejanskledar commented Feb 24, 2025

markom-povio left a comment

nagavcindriqim commented Feb 24, 2025

borut-t left a comment

borut-t Feb 25, 2025

borut-t Feb 25, 2025

borut-t Feb 25, 2025

borut-t Feb 25, 2025

borut-t Feb 25, 2025

borut-t Feb 25, 2025

borut-t Feb 25, 2025

UI/PhotoPreview #330

Are you sure you want to change the base?

UI/PhotoPreview #330

Conversation

nagavcindriqim commented Sep 9, 2024 • edited Loading

borut-t left a comment

Choose a reason for hiding this comment

borut-t commented Sep 23, 2024

nagavcindriqim commented Sep 23, 2024

borut-t commented Sep 23, 2024

nagavcindriqim commented Sep 23, 2024

borut-t commented Sep 23, 2024

nagavcindriqim commented Sep 23, 2024

dejanskledar commented Feb 24, 2025

markom-povio left a comment

Choose a reason for hiding this comment

nagavcindriqim commented Feb 24, 2025

borut-t left a comment

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

borut-t Feb 25, 2025

Choose a reason for hiding this comment

nagavcindriqim commented Sep 9, 2024 •

edited

Loading