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

Include @ViewBuilder in state switch in the Readme #12

Open
dehlen opened this issue Sep 4, 2020 · 7 comments
Open

Include @ViewBuilder in state switch in the Readme #12

dehlen opened this issue Sep 4, 2020 · 7 comments
Labels
documentation Improvements or additions to documentation

Comments

@dehlen
Copy link

dehlen commented Sep 4, 2020

In your readme the following sample code is shown:

// ...
    var body: some View {
        // 5
        switch users.state {
            case .idle, .loading:
                return AnyView(Text("Loading..."))
            case .failed:
                return AnyView(Text("Oops..."))
            case let .succeed(users):
                return AnyView(
                    List(users) { user in
                        Text(user.name)
                    }
                )
        }
    }

As far as I can see you can use the ViewBuilder feature in this case. The QueryRenderer protocol of course is useful too but this would make the code a little but more readable imo and provides another alternative for your users:

// ...
    @ViewBuilder var body: some View {
        // 5
        switch users.state {
            case .idle, .loading:
                Text("Loading...")
            case .failed:
                Text("Oops...")
            case let .succeed(users):
                List(users) { user in
                    Text(user.name)
                }
        }
    }

Please also note that @ViewBuilder is automatically added to the body on iOS 14.

@fmo91 fmo91 added the documentation Improvements or additions to documentation label Sep 4, 2020
@fmo91
Copy link
Owner

fmo91 commented Sep 4, 2020

Hi @dehlen , thank you for making this suggestion!
Wow, that makes the code way more readable for sure. To be honest, I haven't used it before. I'll test it later today and fix it in the README.md

@fmo91 fmo91 changed the title Improve Readme Include @ViewBuilder in state switch in the Readme Sep 4, 2020
@fmo91
Copy link
Owner

fmo91 commented Sep 4, 2020

I've also changed the title to make it clearer in my opinion. Thank you!

@dehlen
Copy link
Author

dehlen commented Sep 4, 2020

No problem glad if this helps :) I just removed the return keyword as well since I think it is not needed/wrong when using ViewBuilders. I typed all of this on my phone so let me know if something does not work for you and I‘ll take a look.

@fmo91
Copy link
Owner

fmo91 commented Sep 6, 2020

Hi @dehlen . I've been testing it. Unfortunately, it doesn't work for me. Am I doing something wrong? This is my code:

import SwiftUI
import Pigeon

struct ContentView: View {
    @ObservedObject private var users = Query<Void, [User]>.init(
        key: .users,
        behavior: .startImmediately(()),
        fetcher: {
            URLSession.shared
                .dataTaskPublisher(for: URL(string: "https://jsonplaceholder.typicode.com/users")!)
                .map(\.data)
                .decode(type: [User].self, decoder: JSONDecoder())
                .receive(on: DispatchQueue.main)
                .eraseToAnyPublisher()
        }
    )
    
    @ViewBuilder var body: some View {
        // 5
        switch users.state {
            case .idle, .loading:
                Text("Loading...")
            case .failed:
                Text("Oops...")
            case let .succeed(users):
                List(users) { user in
                    Text(user.name)
                }
        }
    }
}

extension QueryKey {
    static var users: QueryKey { .init(value: "users") }
}

struct User: Codable, Identifiable {
    let id: Int
    let name: String
}

If you could send something like this, an autocontained piece of code that actually works (not like mine), I'll test it and fix it in the README.md.

Thank you!

@dehlen
Copy link
Author

dehlen commented Sep 7, 2020

Your example just compiles and runs fine for me. I am using Xcode 12 beta 5 currently.

@fmo91
Copy link
Owner

fmo91 commented Sep 7, 2020

Maybe that's it, I'm using Xcode 11.7

@fmo91
Copy link
Owner

fmo91 commented Sep 7, 2020

I'll test it in Xcode 12 and fix the readme. I should also do that to replace @ObservedObject with @StateObject

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants