sindresorhus / Settings

⚙ Add a settings window to your macOS app in minutes
MIT License
1.45k stars 100 forks source link

SwiftUI support #46

Closed fredyshox closed 4 years ago

fredyshox commented 4 years ago

Fixes #42

Resolved by defining PreferencePaneHostingController, wrapper around NSHostingController with PreferencePane support. User provides custom SwiftUI view and data required by the protocol. Also I've added example preference UI UserAccountsView.

I think this is optimal way of doing this, because we can just stick with existing API. Let me know if you have any thoughts on this.


IssueHunt Summary ### Referenced issues This pull request has been submitted to: - [#42: SwiftUI support](https://issuehunt.io/repos/139150459/issues/42) --- IssueHunt has been backed by the following sponsors. [Become a sponsor](https://issuehunt.io/membership/members)
sindresorhus commented 4 years ago

Awesome! Thanks for working on this 🙌

I was hoping to keep as much as possible in SwiftUI, including providing the required data.

One idea could be something like this:

protocol PreferencePaneView: View {
    var preferencePaneIdentifier: Identifier { get }
    var preferencePaneTitle: String { get }
    var toolbarItemIcon: Image { get }
}

struct UserAccountsView: PreferencePaneView {
    var preferencePaneIdentifier = .userAccounts
    var preferencePanelTitle = "User Acccounts"
    var toolbarItemIcon = Image(…)

    var body: some View {}
}
sindresorhus commented 4 years ago

Would also be nice to be able to use Identifiable somehow.

sindresorhus commented 4 years ago

Actually, maybe something like this would be nicer?

struct UserAccountsView: PreferencePaneView {
    var preferencePaneConfig = .init(
        identifier: .userAccounts,
        title: "User Acccounts",
        icon: Image(…)
    )

    var body: some View {}
}

However, I'm totally open to other ideas.

fredyshox commented 4 years ago

@sindresorhus

Hey I was thinking about bundling some components for simple panes, and came up with an idea to create custom Preferences and PreferenceSection components. They do all the alignment and layout magic, so user doesn’t need to know anything about custom AlignmentGuides etc..

I’ve also decided to define PreferencePaneView, just as you proposed.

sindresorhus commented 4 years ago

Hey, really sorry about the lack of response. This simply got lost in all my tabs.

It looks very good now. 👌

One thing I've realized is that we cannot currently ship SwiftUI stuff directly because of this dumb bug. Could you make a separate SwiftUI target for this for Swift Package Manager?

Only allowing PreferencesSection inside Preferences will probably be a bit too limiting. What if you need some custom view between two of the sections?

And just so we don't forget, this will also need some docs when we've finalized how it should work.

fredyshox commented 4 years ago

Ok so it seems to be complete. Here's description of major recent changes:

And as for the AnyView's, I don't see any reasonable alternative with ability to store Views in array. We could create custom ViewBuilder using TupleView, but it would require 10+ overloads, which is a bit ugly.

Also I've added section in README documenting usage. Let me know, if there's anything else 🙂

sindresorhus commented 4 years ago

Alright. I'm finally done with travelling and other things, so I can focus on this PR again.

sindresorhus commented 4 years ago

I think this was missed: https://github.com/sindresorhus/Preferences/pull/46/files#r377738897

sindresorhus commented 4 years ago

While PreferencePaneHostingController is nice when you want to mix normal panes and SwiftUI panes or if you need more control, I think we should also have a simpler way if you only want SwiftUI panes. I'm thinking an overload init for PreferencesWindowController that accepts an array of SwiftUI views instead of PreferencePane for the preferencePanes parameter.

sindresorhus commented 4 years ago

I have been thinking about this, and I think we should go with one config property like suggested earlier:

struct UserAccountsView: PreferencePaneView {
    var preferencePaneConfig = .init(
        identifier: .userAccounts,
        title: "User Accounts",
        icon: Image(…)
    )

    var body: some View {}
}

That way the required config doesn't get so mixed up with the view-specific properties.

sindresorhus commented 4 years ago

I think the doc comments for the main exported parts PreferencePaneView, PreferenceContainer, and PreferenceSection could be expanded and improved. Users will deal with these a lot, so having clear docs for them is important. Would also be nice to have an example in their doc comments too.

sindresorhus commented 4 years ago

NSImage has:

/* These images are intended for use in toolbars in preference windows.
     */
    @available(OSX 10.5, *)
    public class let userAccountsName: String

    @available(OSX 10.5, *)
    public class let preferencesGeneralName: String

    @available(OSX 10.5, *)
    public class let advancedName: String

I think we should wrap them in Image() (SwiftUI) and expose them as extensions on Image, so the user can easily use them.

sindresorhus commented 4 years ago

@fredyshox Friendly bump :)

fredyshox commented 4 years ago

I think this was missed: https://github.com/sindresorhus/Preferences/pull/46/files#r377738897

Wrap everything into namespace using enum?

sindresorhus commented 4 years ago

Yup

fredyshox commented 4 years ago

I’ve seen #51 , and this make me think, that in SwiftUI I often apply modifiers outside of View definition, for example: environmentObject in most cases will be applied with some state-managing object, upon creation of PreferencesWindowController in AppDelegate. So requiring Preferences.PaneView conformance can be limiting.

// …somewhere in AppDelegate
let view = 
    UserAccountsView()
        .environmentObject(self.accountManager)
// view type is now ModifierContent<UserAccountsView, Modifier>

let pane = Preferences.PaneHostingController(preferencePaneView: view) // error: view is not Preferences.PaneView

We can do this differently, by defining Preferences.PaneConfig to satisfy toolbar requirements and using it like this:

// …somewhere in AppDelegate
let config = Preferences.PaneConfig(
    identifier: "...",
    title: "User Accounts",
    toolbarIcon: NSImage(names: "img-name"))

let view = 
    UserAccountsView() // just plain view, not Preferences.PaneView
        .environmentObject(self.accountManager)
        .environmentObject(self.someOtherObject)
        .otherModifier()

let pane = Preferences.PaneHostingController(config: config, paneView: view)

@sindresorhus What do you think about this?

sindresorhus commented 4 years ago

Good point.

Could we make it a view modifier? The benefit would be that the user would be able to set/change the title and toolbar icon directly from SwiftUI.

let view = 
    UserAccountsView() // just plain view, not Preferences.PaneView
        .environmentObject(self.accountManager)
        .environmentObject(self.someOtherObject)
        .otherModifier()
        .preferencePane(
             identifier: "...",
             title: "User Accounts",
             toolbarIcon: NSImage(named: "img-name"))
        )

let pane = Preferences.PaneHostingController(paneView: view)

Another benefit is that it would work with https://github.com/sindresorhus/Preferences/pull/46#issuecomment-609729282.

sindresorhus commented 4 years ago

Another solution would be:

let view = 
    Preferences.PaneView(
        identifier: "...",
        title: "User Accounts",
        toolbarIcon: NSImage(named: "img-name"))
    ) {
        UserAccountsView()
            .environmentObject(self.accountManager)
            .environmentObject(self.someOtherObject)
            .otherModifier()
    }

let pane = Preferences.PaneHostingController(paneView: view)

The benefit of this is that it's more type-safe as we're forcing the user to pass the config and we only accept Preferences.PaneView-type views.

fredyshox commented 4 years ago

Modifier would also be type-safe, only catch is it'd need to be the last applied ViewModifier in the chain:

// Preferences.PaneHostingController
init<T: View>(paneView: ModifiedContent<T, PaneConfigModifier>) {
    // ...
    paneView.modifier //... identifier, title, toolbarIcon
}

I guess we can do both 😃

sindresorhus commented 4 years ago

I don't think we should do both. There should be one consistent way to do it.

sindresorhus commented 4 years ago

Modifier would also be type-safe, only catch is it'd need to be the last applied ViewModifier in the chain:

Yes, but then we need to document the fact that it needs to be last, and the user might accidentally add more modifiers and get a cryptic compile error.

sindresorhus commented 4 years ago

I personally prefer Preferences.PaneView(). Which do you prefer (and why)?

fredyshox commented 4 years ago

Modifier would also be type-safe, only catch is it'd need to be the last applied ViewModifier in the chain:

Yes, but then we need to document the fact that it needs to be last, and the user might accidentally add more modifiers and get a cryptic compile error.

I see, that can be a problem.

So Preferences.PaneView() is fine for me.

sindresorhus commented 4 years ago

So Preferences.PaneView() is fine for me.

👍 Let's go with that then.

fredyshox commented 4 years ago

@sindresorhus Done 👍

sindresorhus commented 4 years ago

I think the only thing missing now is:

While PreferencePaneHostingController is nice when you want to mix normal panes and SwiftUI panes or if you need more control, I think we should also have a simpler way if you only want SwiftUI panes. I'm thinking an overload init for PreferencesWindowController that accepts an array of SwiftUI views instead of PreferencePane for the preferencePanes parameter.

I think this way should be the documented first, and the current way can be documented later on. As the main use-case is SwiftUI-only preference panes.

sindresorhus commented 4 years ago

Let's rename PaneView to Pane as no other view has View-suffix.

fredyshox commented 4 years ago

@sindresorhus One issue with init<T: View>(paneViews: [PaneView<T>]) in PreferencesWindowController came out, that as we store generic views in the array, all of them would need to have same generic type T.

I solved it by defining PreferencePaneConvertible, which acts as type eraser.

sindresorhus commented 4 years ago

Can you fix the merge conflict?

sindresorhus commented 4 years ago

Nice work, @fredyshox 🙌

hisaac commented 3 years ago

I'm seeing some weird tab animation when using SwiftUI views. Any idea why that might be? #59