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

Generic SettingView #23

Open
aheze opened this issue Apr 29, 2023 · 1 comment
Open

Generic SettingView #23

aheze opened this issue Apr 29, 2023 · 1 comment
Labels
enhancement New feature or request Generic SettingView Help needed! help wanted Extra attention is needed

Comments

@aheze
Copy link
Owner

aheze commented Apr 29, 2023

The problem

Currently SettingView renders each element via a huge switch.

Huge `switch` statement that checks if the element is a text, button, toggle, page, etc.

This has several limitations:

No support for custom Settings

The switch statement checks against SettingText, SettingButton, SettingToggle, etc. Say we had our own element called SettingLabel:

struct SettingLabel: View, Setting {
    var id: AnyHashable?

    var body: some View {
        Text("Hello!")
    }
}

This would fall through in the switch statement and error out.

switch setting {
case let text as SettingText:
    text
case let button as SettingButton:
    button
case let toggle as SettingToggle:
    /* ... */
default:
    Text("Unsupported setting, please file a bug report.")
}

Text displaying "Unsupported setting, please file a bug report"

Limited customization (no subclass / composing support)

This is related to the above issue. Let's say you want to make a bunch of SettingTexts blue. Right now you need to do this:

SettingText(title: "Hello", foregroundColor: .blue)
SettingText(title: "Hello again", foregroundColor: .blue)
SettingText(title: "Bruh", foregroundColor: .blue)

Ideally you'd be able to make a new element that conforms from Setting.

struct BlueSettingText: View, Setting {
    var id: AnyHashable?
    var title: String

    var body: some View {
        SettingText(title: title, foregroundColor: .blue)
    }
}

/* ... */

BlueSettingText(title: "Hello")
BlueSettingText(title: "Hello again")
BlueSettingText(title: ":)")

This code compiles, but it will fall through in the above mentioned issue with the switch statement.

Reliance on stable ID identifiers

You might have noticed all the var id: AnyHashable?s floating around. This is because of the ForEach loop for SettingPage, SettingGroup, and SettingTupleView.

ForEach(page.tuple.settings, id: \.identifier) { setting in
    SettingView(setting: setting, isPagePreview: true)
}

ForEach(group.tuple.settings, id: \.identifier) { setting in
    SettingView(setting: setting)
}

ForEach(tuple.settings, id: \.identifier) { setting in
    SettingView(setting: setting)
}

Looping over each element's children, for example `page.tuple.settings`

Currently, Setting synthesizes this ID depending on the element — for SettingText, it gets the title property. For SettingTupleView, it gets tuple.flattened.compactMap { $0.textIdentifier }.joined().

https://github.com/aheze/Setting/blob/main/Sources/Setting.swift#L18-L38

Ideally, we could get rid of this ID and just use a View's default identity. (For more details, see How the SwiftUI View Lifecycle and Identity work, "Identity of a view" section.)

No support for custom view modifiers

SettingToggle(title: "This value is persisted!", isOn: $isOn)
    .tint(.red) /// Argument type 'some View' does not conform to expected type 'Setting'

This requires changes to SettingBuilder.

The solution

SettingView needs to be generic, resembling SwiftUI's View as closely as possible.

If possible, we should make Setting conform to View, so that we can render it directly without the need for a huge switch.

I've attempted this and got lost quickly — I'm not good with generics.

Any help? :)

@aheze aheze pinned this issue Apr 29, 2023
@aheze aheze added enhancement New feature or request help wanted Extra attention is needed labels Apr 29, 2023
This was referenced Apr 29, 2023
@aheze aheze added the Generic SettingView Help needed! label Apr 29, 2023
@adisve
Copy link

adisve commented May 21, 2023

If I understood correctly and you need any item conforming to Setting to basically act as a View then maybe you could do

public protocol Setting: View, Identifiable { /// Hashable can be used instead, just add hash function to below extension
    /// Random required parameters
    
    var id: String { get } 
    
    var title: String { get }
    
    var icon: Image { get }
}

/// To identify the View
public extension Setting {
    var id: String {
        /// Some string id
    }
}

Then declare and create the views with

struct SomeSettingItem: Setting {

    var title: String
    
    var icon: Image
    
    var body: some View {
        Button(action: {
            /// Do something
        }) {
            HStack {
                icon
                    .resizable()
                    .frame(width: 20, height: 20)
                    .foregroundColor(.red)
                
                Text(title)
                    .font(.body)
                    .foregroundColor(.red)
            }
        }
    }
}

SomeSettingItem(title: "Item 1", icon: Image(systemName: "house"))

And then declaring some view to hold these generic elements would possibly be

public struct SettingView<T>: View where T: Setting

And for example then render them easily in a ForEach as views

ForEach(items, id: \.id) { item in
      item
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Generic SettingView Help needed! help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants