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

When initializing a WebImage with isAnimating, if the initial value is false, the image never animates #332

Closed
joshuatbrown opened this issue Aug 27, 2024 · 5 comments · Fixed by #333

Comments

@joshuatbrown
Copy link

joshuatbrown commented Aug 27, 2024

Given the following code:

struct MyView: View {
    let url: URL

    @State private var isAnimating = false

    var body: some View {
        ZStack {
            WebImage(url: url, isAnimating: $isAnimating)
            Button {
                 isAnimating = true
            } label: {
               Text(.localizable.gifButton)
            }
        }
    }
}

When I tap the button, the image will never start animating. It looks like the issue is in init(url:scale:options:context:isAnimating:transaction:content:) for WebImage here:

// provide animated image class if the initialized `isAnimating` is true, user can still custom the image class if they want
if isAnimating.wrappedValue {
    if context[.animatedImageClass] == nil {
        context[.animatedImageClass] = SDAnimatedImage.self
    }
}

So if the initial value of isAnimating is false, the context never gets an .animatedImageClass and therefore the image never animates.

This is unexpected -- the init accepts a Binding<Bool>, so if the value changes to true, I'd expect the image to start animating.

Here's my workaround, which I'd prefer not to do:

WebImage(url: url, context: [.animatedImageClass: SDAnimatedImage.self], isAnimating: $isAnimating)

When I do this, I can start animating the image by setting the value of my isAnimating to true.

@dreampiggy
Copy link
Collaborator

dreampiggy commented Aug 28, 2024

You're correct. Currently it relies on the extra information passed to SDWebImage level. So the binding may cause issue.

I've point this out in the comments here in old version:

The binding value should be true when initialized to setup the correct animated image class. If not, you must provide the .animatedImageClass explicitly.

If you want to change this logic, this may cause some issue because the non-animated image may be already cached for the same URL, and you'will never get the animated one. (Animated Image and non-Animated Image is different object in implementation detail)

@dreampiggy
Copy link
Collaborator

dreampiggy commented Aug 28, 2024

The AniamtedImage one seems work fine. Because the custom image class option is provided during updateUIView callback, so it allows for state changes.

@joshuatbrown
Copy link
Author

joshuatbrown commented Aug 28, 2024

@dreampiggy thanks for the explanation! Sorry I missed the doc comments -- that makes sense to me now. At any rate, thanks for the quick fix in #333!

@dreampiggy
Copy link
Collaborator

Released in v3.1.2

@joshuatbrown
Copy link
Author

@dreampiggy thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants