Feature/improve scrollview search

[gaetanzanella]

I based the scroll view search on the SwiftUI preferences to improve the performances

[gaetanzanella]

I think this is a SwiftUI bug. I discovered the same behavior in the Apple sample code. Anytime we inject an UIHostingController in the view hierarchy, the navigation bar modifier becomes inconsistent.

In your case, as a workaround, I fixed the issue by forcing a view update once all the view hierarchy is built:

@State var hidden = false

    var body: some View {
        NavigationView {
            ZStack {
                Color.red // map
                    .ignoresSafeArea()
                Color.green // something else
                    .ignoresSafeArea(.all, edges: .bottom)
                    .navigationTitle("Test")
                    .navigationBarHidden(hidden)
                    .dynamicOverlay(overlayView)
                    .dynamicOverlayBehavior(behavior)
                    .ignoresSafeArea(.all, edges: .bottom)
            }
        }
        .onAppear {
            CATransaction.setCompletionBlock {
                hidden = true
            }
        }
    }

Also, I moved the drivingScrollView modifier:

var overlayView: some View {
    VStack {
        header
        Button {
            contentSwapped.toggle()
        } label: {
            Text("Content swap")
        }
        list1
        if contentSwapped {
            list3
                .accessibilityIdentifier("List3")
        } else {
            list2
                .accessibilityIdentifier("List2")
        }
    }
    .drivingScrollView(scrollViewDriving)
    .background(Color.white)
    .foregroundColor(Color.black)
    .frame(maxWidth: .infinity)
}

So when the content changes, the current UIHostingController will not be removed from the hierarchy.

I think using UIViewRepresentable will create more bugs than this one as the UIHostingController in charge of the rendering is not in the hierarchy. I am practically sure it will create inconsistency in the UI event propagation. Apple would have provided an UIHostingView directly if it was not the case.

[Lumentus]

@gaetanzanella Well I just tried your suggested changes and they work, if I add all of them. Unfortunately that also re introduces the problem of the wrong ScrollView being picked as driving. If you think UIViewRepresentable will introduce bugs, then maybe we need to go back to the drawing board on finding a way to identify the correct ScrollView. Additionally this way of identifying the drivingScrollView makes the order of modifications very important, e.g. using .drivingScrollView().environment(.editMode, $editMode) on a list will not work correctly, while .environment(.editMode, $editMode).drivingScrollView() does.

Maybe we could do something like Introspect for SwiftUI does, where they put the "marker view" into an overlay of the modified view and then they check that markers siblings to find die view with the type you want.

[gaetanzanella]

Right @Lumentus, I missed your list1. You could use a Group or a computed property then:

@ViewBuilder
var myScrollableContent: some View {
    if contentSwapped {
       list3
    } else {
       list2
    }
}

VStack {
    header
    Button {
       contentSwapped.toggle()
    } label: {
        Text("Content swap")
    }
    list1
    myScrollableContent.drivingScrollView()
}

[Lumentus]

@gaetanzanella That seems like a possible workaround, though I would say, that this seems like a lot of burden to put on a user of this library, if there is a better way to do this. This places a large constraint of how the user has to implement their app. For example in the app my company is developing the way we currently do it, we would have to restructure a lot of views. We have different contents with different drivingScrollViews that we have in one View that is then included in the Overlay. Basically this situation:

var overlayView: some View {
    VStack {
        header
        Button {
            contentSwapped.toggle()
        } label: {
            Text("Content swap")
        }
        content
    }
    .background(Color.white)
        .foregroundColor(Color.black)
        .frame(maxWidth: .infinity)
}

@ViewBuilder
var content: some View {
    if contentSwapped {
        VStack {
            list4
            list3
                .accessibilityIdentifier("List3")
                .drivingScrollView(scrollViewDriving)
        }
    } else {
        VStack {
            list1
            list2
                .accessibilityIdentifier("List2")
                .drivingScrollView(scrollViewDriving)
        }
    }
}

We would need to split the lists into different views in order to put the scrolling views into a container that is annotated with drivingScrollView. And that already doesn't work anymore if I change content to:

if contentSwapped {
    VStack {
        list4
        list3
            .accessibilityIdentifier("List3")
            .drivingScrollView(scrollViewDriving)
    }
} else {
    VStack {
        list2
            .accessibilityIdentifier("List2")
            .drivingScrollView(scrollViewDriving)
        list1
    }
}

Because then the position of the drivingScrollView is no longer the same. So I think we need to find a better solution for this.

[gaetanzanella]

Yes, sorry... But I couldn't find a better solution... Swift Introspect searches in the hierarchy using a weird if statement based on the current os version. It will break in the future for sure. Maybe we could tweak the VC, by overriding some methods, to avoid this navigation bar issue but... until now I couldn't.

[Lumentus]

Hmm okay. I think I can take some more time at my workplace to give this a try. I guess you could merge this until then. Though I would adjust the README to reflect what the user needs to be careful about when using .drivingScrollView.

[gaetanzanella]

Hi @Lumentus, I tried another solution based on frames. It creates an obvious issue when two scroll views overlap but it should not happen very often and it is still better than the current implementation. Tell me what you think about it.

[Lumentus]

Hi @gaetanzanella. This seems like a great solution. I don't see it as a realistic scenario, that a user would have overlapping scroll views in the overlay. That seems rather unlikely. I have given this a try with my companies app that we use this for and there I have found this to be problematic, if the overlay is immediately "hidden" by a NavigationLink executing. In that case it seems that the preference key is reset to the default value. I'm trying to see if I can make a minimal example of this. But that should be able to be worked around (and is probably a relatively rare circumstance anyway).

[gaetanzanella]

SwiftUI is so unpredictable 😅
Let me know! I will take a look at this example.

[Lumentus]

Just FYI: I just found that the same problem can happen with the previous solution.

[gaetanzanella]

Okay let's merge it them. Could you create an issue?

[Lumentus]

@gaetanzanella I will. When do you expect to be able to merge these changes into master and create a release? Do you have a time table for that?

[gaetanzanella]

Done @Lumentus 🙂