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 won't load image if EmptyView is a placeholder #1972

Closed
3 tasks done
damian-rzeszot opened this issue Jul 28, 2022 · 4 comments · Fixed by #1973
Closed
3 tasks done

KFImage won't load image if EmptyView is a placeholder #1972

damian-rzeszot opened this issue Jul 28, 2022 · 4 comments · Fixed by #1973

Comments

@damian-rzeszot
Copy link

Check List

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

Issue Description

What

The view I work with is quite complicated, but the whole problem comes down to this:

KFImage(url)
    .placeholder {
        EmptyView()
    }

This placeholder is used here and wrapped with a Group. For the Group only with EmptyView a callback for onAppear won't be called. This causes the image to not be loaded.

Other Comment

Sample code of onAppear behaviour

struct ContentView: View {
    var body: some View {
        Group {
            Color.clear
                .onAppear {
                    print("color clear in group ✅")
                }
            EmptyView()
                .onAppear {
                    print("empty view in group 🔴")
                }
        }
        .onAppear {
            print("group ✅")
        }

        Group {
            EmptyView()
                .onAppear {
                    print("empty view in group 🔴")
                }
        }
        .onAppear {
            print("group 🔴")
        }
    }
}
@onevcat
Copy link
Owner

onevcat commented Jul 29, 2022

@damian-rzeszot Nice catch and thank you for reporting this. Does #1973 (the fix/empty-placeholder-loading branch) work for you?

@damian-rzeszot
Copy link
Author

Yes, it works great. Thanks for this quick fix :shipit:

@onevcat
Copy link
Owner

onevcat commented Aug 10, 2022

After some checking, the same "issue" also happens in SwiftUI's AsyncImage. As Apple mentions in its documentation of EmptyView:

You will rarely, if ever, need to create an EmptyView directly. Instead, EmptyView represents the absence of a view.

I am now wondering if it is a bit anti-pattern to use an EmptyView as the placeholder.

A new PR #1975 was created to fix some broken layout behavior introduced in #1973, while keeping the ability of trigering a loading by EmptyView placeholder. However, I am still suggesting not to use that and maybe try to have a Color.clear placeholder instead.

@drspoton
Copy link

In my case I got EmptyView from optional Image

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 a pull request may close this issue.

3 participants