stevengharris / SplitView

A flexible way to split SwiftUI views with a draggable splitter
MIT License
146 stars 17 forks source link

divider's visual bug #19

Closed serge-ivamov closed 1 year ago

serge-ivamov commented 1 year ago
// …
let lh = LayoutHolder(.vertical)
// …
Split(primary: { Color.red }, secondary: { TabView { Color.green } })
    .layout(lh)
Button("bug") { withAnimation { lh.toggle() } }
Button("ok") { lh.toggle() }

// …
Screenshot 2023-09-21 at 22 17 27
stevengharris commented 1 year ago

It looks like you just need to include tabItems for the items in your tabs, like:

// …
let lh = LayoutHolder(.vertical)
// …
Split(primary: { Color.red }, secondary: {
    TabView {
        Color.green
            .tabItem {
                Label("Green", systemImage: "hand.thumbsup")
            }
        Color.yellow
            .tabItem {
                Label("Yellow", systemImage: "hand.thumbsdown")
            }
    }
})
.layout(lh)
Button("bug") { withAnimation { lh.toggle() } }
Button("ok") { lh.toggle() }
// …

I am running this on Sonoma with the latest version 3.2.2, so maybe something was fixed between what you are running and what I just released. In any case, when I paste your code into a fresh view, it works for me even without tabItems. At least I cannot see the image you included above where the splitter is at the wrong orientation. I see:

Screenshot 2023-09-21 at 2 35 42 PM

or

Screenshot 2023-09-21 at 2 35 33 PM
serge-ivamov commented 1 year ago

Tnx, Steven!

I am on Ventura 13.6 and the bug is still here, even with .tabItem(). Let's hope for Sonoma :)

stevengharris commented 1 year ago

It's very odd! I don't have a Ventura machine easily accessible, but I tried it on my old MacBook running Big Sur, and it also shows no issue.

The only time I have seen something looking like this has been when I messed something up in a custom splitter. The default Splitter gets its layout from the Environment, established in Split, so it should not be possible for it to get out-of-sync with the Split view like this shows. It looks visually like that is what happened here - that Splitter's value for layout is different than the value Split has.

Just to be sure we are testing the exact same thing in the same way, you could paste your code into the DemoApp so it replaces the .simpleDefaults case as follows:

case .simpleDefaults:
    let lh = LayoutHolder(.vertical)
    Split(primary: { Color.red }, secondary: { TabView { Color.green } })
        .layout(lh)
    Button("bug") { withAnimation { lh.toggle() } }
    Button("ok") { lh.toggle() }

If running the demo with this pasted-in shows the same problem for you, then I'm willing to blame it on a weird Ventura SwiftUI bug (that I'm not working around 😜).

serge-ivamov commented 1 year ago
start

at start

1click

first click

2click

second click


1click+resize

first click + a lil resize of window - it's autofixed.

P.S. w/o TabView - no problems at all.

stevengharris commented 1 year ago

Can you try it using DemoSplitter? It will look ugly, but if the layouts match between DemoSplitter and Split, then it will tell us if I might be able to fix it by using ObservedObject instead of Environment. Like this:

case .simpleDefaults:
    let lh = LayoutHolder(.vertical)
    Split(primary: { Color.red }, secondary: { TabView { Color.green } })
        .splitter { DemoSplitter(layout: lh, hide: SideHolder()) }
        .layout(lh)
    Button("bug") { withAnimation { lh.toggle() } }
    Button("ok") { lh.toggle() }
serge-ivamov commented 1 year ago
Screenshot 2023-09-22 at 22 38 37

at start

Screenshot 2023-09-22 at 22 39 31

1st click

Screenshot 2023-09-22 at 22 42 54

2nd click

so it works like w/o custom .splitter BUT!!! with a 50% chance everything works correctly! in 10 apps run – 5/5 bugged and ok.

stevengharris commented 1 year ago

I didn't realize it would be so confusing. The DemoSplitter is clear, so the triangle direction in the button is indicating the layout for the DemoSplitter. It's showing the same behavior as the Splitter, which is:

  1. At start: DemoSplitter layout = .vertical, Split layout = .vertical
  2. 1st click: DemoSplitter layout = .horizontal, Split layout = .vertical
  3. 2nd click: DemoSplitter layout = .vertical, Split layout = .horizontal

I was thinking the Splitter was in the wrong orientation, but it's the Split view that's layed out wrong.

I would debug this myself if I could reproduce it. I don't know how much time you want to spend on it in this way. If you're up for more, here are some more things to try:

  1. Hold the LayoutHolder in a StateObject. So in DemoApp, it would look like:
struct DemoApp: View {
    @State private var demoID: DemoID = .simpleDefaults
    @StateObject private var lh = LayoutHolder(.vertical)

    var body: some View {
        let demo = demos[demoID]!
        VStack {
            DemoToolbar(demoID: $demoID)
            switch demoID {
            case .simpleDefaults:
                Split(primary: { Color.red }, secondary: { TabView { Color.green } })
                    .layout(lh)
                Button("bug") { withAnimation { lh.toggle() } }
...
  1. Use a VStack instead of the TabView to see if it has the same problems:
struct DemoApp: View {
    @State private var demoID: DemoID = .simpleDefaults
    @StateObject private var lh = LayoutHolder(.vertical)

    var body: some View {
        let demo = demos[demoID]!
        VStack {
            DemoToolbar(demoID: $demoID)
            switch demoID {
            case .simpleDefaults:
                Split(primary: { Color.red }, secondary: {
                    VStack {
                        Button(lh.isHorizontal ? "Horizontal": "Vertical") {}
                        Color.green
                    }
                })
                .layout(lh)
                Button("bug") { withAnimation { lh.toggle() } }
...
serge-ivamov commented 1 year ago
  1. still bugged

  2. Screenshot 2023-09-23 at 10 22 03

no problems with VStack + Button.

  1. VStack + TabView = bugged

P.S. btw, IMO, using an @EnvironmentObject is incorrect anyway, since it only stores them of one type, which will create problems if the splitter is used in further views.

P.P.S. let's wait for Sonoma, mb problems are related to xcode 15 on Ventura…

stevengharris commented 1 year ago

Agreed on Environment. Looking at this problem, I thought: why did I do that!? I think it was something I did when I was sorting out some other problem and never reverted 🤷‍♂️. Thanks for spending so much time debugging. Let's see if it changes with Sonoma.

serge-ivamov commented 1 year ago

No bugs on Sonoma :)

stevengharris commented 1 year ago

Great!!! Thanks again for spending so much time looking into it.