Inspector's content disappears before the inspector is fully closed and doesn't play nicely with inspectorColumnWidth (macOS)

kuglee commented 2 weeks ago

Description

It's nice to see Inspector added to the library. I had my own implementation before. When implementing my own version I discovered that there are some issues with the binding based approach for inspector on macOS.

I'm curious if anyone has a nice solution for these.

Checklist

[ ] I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
[ ] If possible, I've reproduced the issue using the main branch of this package.
[X] This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

the inspector's content DOESNt disappears before the inspector is fully closed
when using the inspectorColumnWidth modifier the inspector is already has the set width during the opening animation

Actual behavior

the inspector's content disappears before the inspector is fully closed, which doesn't look great
when using the inspectorColumnWidth modifier the inspector is resized to the set with during the opening animation, causing a visual glitch

Steps to reproduce

Here's a demo project showing the difference between the isPresented and the binding based approach (The implementation can be toggled with a button and the inspector can be opened/closed with the toolbar button): SwiftUIInspectorToolbar

SwiftUI Navigation version information

2.2.0

Destination operating system

macOS

Xcode version information

Xcode 16.1 beta (16B5001e)

Swift Compiler version information

swift-driver version: 1.113 Apple Swift version 6.0 (swiftlang-6.0.0.7.6 clang-1600.0.24.1)
Target: arm64-apple-macosx14.0

stephencelis commented 2 weeks ago

@kuglee While I don't think the optional-based presentation can fully replace the boolean-based presentation due to the eagerness of the API, I do think we can address the common case: https://github.com/pointfreeco/swift-navigation/pull/221

Wanna take it for a spin and report back?

kuglee commented 2 weeks ago

Thanks for the quick response. I've tested it with the demo project. Both issues seem to be fixed.

kuglee commented 2 weeks ago

I've forgotten that there's an another issue with the binding approach. The toolbar buttons of the inspector's view aren't visible, but they're visible with the isPresented inspector. I've pushed demo code to the toolbar-button branch of the same demo projectSwiftUIInspectorToolbar

The toolbar button is visible with the isPresented inspector:

struct ContentView: View {
  var body: some View {
    EmptyView()

      // THE TOOLBAR BUTTON IS VISIBLE
      .inspector(isPresented: self.$isInspectorPresented) {
        Text("Inspector")
          .toolbar {
            Spacer()
            Button(action: { self.isInspectorPresented.toggle() }) {
              Image(systemName: "sidebar.trailing")
            }
          }
      }
  }

  @State var isInspectorPresented = false
}

The toolbar button is not visible with the Binding inspector:

struct ContentView: View {
  var body: some View {
    EmptyView()

      // THE TOOLBAR BUTTON IS NOT VISIBLE
      .inspector(item: $data) { $data in
        Text("Inspector")
          .toolbar {
            Spacer()
            Button(action: { self.data = if self.data != nil { nil } else { 1 } }) {
              Image(systemName: "sidebar.trailing")
            }
          }
      }
  }

  @State var data: Int?
}

stephencelis commented 2 weeks ago

@kuglee Sadly I think we've come to the conclusion that we can't add optional-driven support to this API. The PR has been updated to remove the helper entirely. It seems that Apple requires the underlying data of an inspector be non-optional for the view to function in a non-glitchy way.

kuglee commented 2 weeks ago

Yeah. I had similar concerns. I also think it's better not to be included in the library. Anyway this implementation is better than mine, so I can still use it and add an additional toolbar argument. Thanks 🙏

pointfreeco / swift-navigation