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

KFImage doesn't allow to configure Image as a View (SwiftUI) #2016

Closed
3 tasks done
omar-belhaouss opened this issue Jan 5, 2023 · 3 comments
Closed
3 tasks done

KFImage doesn't allow to configure Image as a View (SwiftUI) #2016

omar-belhaouss opened this issue Jan 5, 2023 · 3 comments

Comments

@omar-belhaouss
Copy link

omar-belhaouss commented Jan 5, 2023

Check List

Thanks for considering to open an issue. Before you submit your issue, please confirm these boxes are checked.

Issue Description

What

I want to apply a scaledToFill() modifier to the loaded Image and a scaledToFit() to the placeholder View but it is not possible because the KFImage.configure function requires to return an Image instead of a View.

public func configure(_ block: @escaping (HoldingView) -> HoldingView) -> Self

(Where HoldingView is an Image for KFImage)

If we take the AsyncImage as an example, it allows to configure the Image and returns some View instead

Reproduce

Here is a code that reproduce the issue:

import SwiftUI
import Kingfisher

struct ImageWrapper: View {
    var url: URL
    
    var body: some View {
        KFImage(url)
            .resizable()
            .configure({ image in
                image
                    //.scaledToFill() // ⛔️ NOT ALLOWED
            })
            .placeholder {
                Image(systemName: "arrow.triangle.2.circlepath")
                    .resizable()
                    .scaledToFit()
            }
    }
}

struct ContentView: View {
    var body: some View {
        VStack {
            // Image with correct URL (1000x1000)
            image(from: URL(string: "https://i.picsum.photos/id/106/1000/1000.jpg?hmac=iWNYUAKllk_GfeYH2QJNaFV7rimXMzblGILOLol7jPQ")!)
            
            // Image with bad URL so the placeholder will be visible
            image(from: URL(string: "https://i.picsum.photos/id/106/1000/1000.jpg?hmac=TEST")!)
        }
    }
    
    @ViewBuilder
    private func image(from url: URL) -> some View {
        ImageWrapper(url: url)
            .scaledToFill() // ⛔️ If I do that, it overrides the `scaledToFit()` of the placeholder
            .frame(width: 200, height: 100)
            .clipped()
    }
}

Other Comment

I am working on a project that support iOS 14+

@onevcat
Copy link
Owner

onevcat commented Jan 9, 2023

Basically, when using a placeholder, Kingfisher wraps it in a ZStack like this:

ZStack {
    Image(systemName: "arrow.triangle.2.circlepath")
        .resizable()
        .scaledToFit()
}
.scaledToFill()
.frame(width: 200, height: 100)
.clipped()

So the issue here comes from the way SwiftUI calculate its child view size. As an image it will just ignore the proposal size when a clip applies outside. Maybe you can try to also pass in the size to the placeholder, like this:

ZStack {
    Image(systemName: "arrow.triangle.2.circlepath")
        .resizable()
        .scaledToFit()
        .frame(width: 200, height: 100)
}
.scaledToFill()
.frame(width: 200, height: 100)
.clipped()

So in your example, try:

  KFImage(url)
      .resizable()
-      .configure({ image in
-          image
-              //.scaledToFill() // ⛔️ NOT ALLOWED
-      })
      .placeholder {
          Image(systemName: "arrow.triangle.2.circlepath")
              .resizable()
              .scaledToFit()
+            .frame(width: 200, height: 100)
      }
     .scaledToFill()
     .frame(width: 200, height: 100)
     .clipped()

I will also see if it is possible to provide a more generic Image -> some View method as configuration. But it requires more effect and may take some time.

@omar-belhaouss
Copy link
Author

Indeed, it works perfectly if I pass the size as you suggested 🎉
Thank you!

I let you close that issue if needed (or maybe we keep it open for that Image -> some View subject)

@onevcat
Copy link
Owner

onevcat commented Jan 31, 2023

Image -> some View configuration block is under implementation and I will try to give it a tag soon.

Please allow me to close it for now. : ]

Ref: #2027

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

No branches or pull requests

2 participants