hishnash / KeyWindow

Share values provided by views in the Key window to all other parts of your SwiftUI application including the commands block.
MIT License
45 stars 5 forks source link

@KeyWindowValueBinding doesn't work when declared in "Commands" (maybe fix or document) #3

Closed jessegrosjean closed 3 years ago

jessegrosjean commented 3 years ago

Thanks for KeyWindow!

I've adapted this example from your blog post on KeyWindow. I initially tried binding to the key document directly from my TextDocumentCommands struct. I tried this for a long time and decided KeyWindow was buggy! ... but actually it works great, I just needed to bind in my ClearDocument: View struct.

So in the below code "Bold" is always disabled (bad) while "Clear" is disabled when there's no windows and enabled when there are windows (good!). Binding from Commands implementation doesn't work, binding from View implementation does work.

If you have a moment:

  1. Can this be fixed in KeyWindow implementation
  2. If not can you give a quick explanation of what's happening ... I guess maybe Commands aren't fully part of SwiftUI view tree and so don't get environment or preferences that KeyWindow relies on for internal implementation?
  3. Also if not maybe add a note somewhere that @KeyWindowValueBinding must be used in view. And sorry if you already have and I just missed it.
import SwiftUI
import KeyWindow

struct TextDocumentCommands: Commands {

    @KeyWindowValueBinding(TextDocument.self) var document: TextDocument?

    var body: some Commands {
        CommandMenu("Format") {
            Button("Bold") {}
                .disabled(document == nil)

            ClearDocument()
        }
    }
}

struct ClearDocument: View {

    @KeyWindowValueBinding(TextDocument.self) var document: TextDocument?

    var body: some View {
        Button("Clear") {}
            .disabled(document == nil)
    }
}
hishnash commented 3 years ago

Yer your write the @KeyWindowValueBinding and for that matter any @ based attribute that needs to trigger the view to re-render does not work in Commands.

I will update the documentation to make this clear.

If you have lots of menu commands that follow this pattern (in my apps this is a common pattern after all). To avoid repeated boilerplate you could use the View Provider pattern.

struct WithKeyWindowTextDocumentBinding<Wrapped: View>: View {
     @ViewBuilder
     var wrapped: (Binding<Optional<TextDocument>>) -> Wrapped

     @KeyWindowValueBinding(TextDocument.self) 
     var document: TextDocument?

     var body: some View {
          wrapped($document).disabled(document == nil)
     }
}

This could then be used in your commands directly:

struct TextDocumentCommands: Commands {

    var body: some Commands {
        CommandMenu("Format") {
            WithKeyWindowTextDocumentBinding {  _ in
                 Button("Bold") {}
            }

            ....
        }
    }
}

if you think its helpful i could add a more generic version of this Provider view to the package that supports passing in the TextDocument.self as a type.

jessegrosjean commented 3 years ago

Thanks for the info and tip. I think that binding pattern is good and I'll use in my own code (also many menu items). I'm not sure if it should be in framework or not. One one hand it's useful, and encourages people to avoid my mistake. On the other hand it adds one more layer of "I wonder how this works". I'm still getting used to SwiftUI so at the moment each of those layers adds some confusion. Maybe that won't be the case as I use framework more. Anyway thanks for the lib and help!