johnpatrickmorgan/FlowStacks

Nested coordinator can't open more than one screen

Closed this issue · 14 comments

Hi, I found another bug with nested coordinators. A nested coordinator can't open more than one screen, when trying to call a transition - nothing happens, and if you close the open screen after that - navigation will be broken.

  • iOS 17.5
  • 0.6.2

Example code that reproduces the behavior:

import SwiftUI
import FlowStacks

enum Path: Hashable {
    case sport(Int, SecondCoordinator)
}

enum NewPath: Hashable {
    case challenge(String, SecondCoordinator)
}

class FirstCoordinator: ObservableObject {
    @Published var path: Routes<Path> = []
    
    func push() {
        path.push(.sport(1, .init(self)))
    }
}

struct ContentView: View {
    @ObservedObject var coordinator: FirstCoordinator
    
    var body: some View {
        FlowStack($coordinator.path, withNavigation: true) {
            VStack {
                Button {
                    coordinator.push()
                } label: {
                    Text("Push")
                }
            }
            .navigationTitle("ROOT")
            .flowDestination(for: Path.self) { screen in
                switch screen {
                case let .sport(value, coordinator):
                    SportView(coordinator: coordinator, value: value)
                }
            }
            
        }
    }
}

class SecondCoordinator: ObservableObject {
    private let parent: FirstCoordinator
    private let id = UUID()
    
    @Published var path: Routes<NewPath> = []
    
    init(_ parent: FirstCoordinator) {
        self.parent = parent
    }
    
    func push() {
        path.push(.challenge("2", self))
    }
}

extension SecondCoordinator: Hashable, Equatable {
    static func == (lhs: SecondCoordinator, rhs: SecondCoordinator) -> Bool {
        lhs.id == rhs.id
    }
    
    func hash(into hasher: inout Hasher) {
        hasher.combine(id)
    }
}

struct SportView: View {
    @ObservedObject var coordinator: SecondCoordinator
    
    let value: Int
    
    var body: some View {
        FlowStack($coordinator.path) {
            VStack {
                Button {
                    coordinator.push()
                } label: {
                    Text("push")
                }
                Text(value.description)
            }
            .navigationTitle("Sport")
            .flowDestination(for: NewPath.self) { screen in
                switch screen {
                case let .challenge(value, coordinator):
                    ChallengeView(coordinator: coordinator, value: value)
                }
            }
        }
    }
}

struct ChallengeView: View {
    @ObservedObject var coordinator: SecondCoordinator
    
    let value: String
    var body: some View {
        VStack {
            Text(value)
            Button {
                coordinator.push()
            } label: {
                Text("challenge")
            }
        }
        .navigationTitle("Challenge")
    }
}

I also attach a recording of the behavior
https://github.com/johnpatrickmorgan/FlowStacks/assets/43087859/557012be-0619-4838-af8f-a4bf5d63121b

@johnpatrickmorgan hi, do you have any ideas or suggestions?)

Hi @lisindima, thanks for raising this. Sorry for the delay - I've been looking into it, and I'm a bit puzzled so far. I'll give more info about my findings so far on Monday. Thanks!

Hi, here's a quick update:

FlowStack holds onto the binding passed in to its initialiser (in its externalTypedPath property) and ensures any changes made to it get propagated to its own internal source of truth - its path property. It does so via a call to onChange(of: ). The issue seems to arise because changes to the externalTypedPath are triggering the FlowStack to recompute its body, but the onChange closure is for some reason not getting called. Adding _printChanges shows that the mutation causes the FlowStack's body to be recomputed (@self, _externalTypedPath changed is printed), but the onChange(of: _externalTypedPath) closure is not called. I've been trying to figure out why but I haven't found anything concrete so far.

Along the way I did notice a simpler issue that was causing FlowLinks not to work within a nested FlowStack, so I put in a fix for that in v0.6.3.

Thanks for your investigation, I also found one rather dirty hack, but it works well so far.

Use withNavigation: true in the nested coordinator. This will result in duplication of the NavigationBar, but this can be fixed with navigationViewModifier and we get this code:

struct SportView: View {
    @ObservedObject var coordinator: SecondCoordinator
    
    let value: Int
    
    var body: some View {
        FlowStack($coordinator.path, withNavigation: true, navigationViewModifier: NavigationBarHidden()) {
            VStack {
                Button {
                    coordinator.push()
                } label: {
                    Text("push")
                }
                Text(value.description)
            }
            .navigationTitle("Sport")
            .flowDestination(for: NewPath.self) { screen in
                switch screen {
                case let .challenge(value, coordinator):
                    ChallengeView(coordinator: coordinator, value: value)
                }
            }
        }
    }
}

struct NavigationBarHidden: ViewModifier {
   func body(content: Content) -> some View {
        content
            .navigationBarHidden(true)
    }
}

Maybe this will help you)

with this hack there was a problem, on iOS 15 onAppear is called twice, without this hack the problem is similar - the nested coordinater is not able to open more than one screen.

also this hack doesn't work with FlowPath()

@johnpatrickmorgan hi, is there any progress on this issue?

@johnpatrickmorgan I don't want to seem intrusive, but my team is very much blocked by this bug, we encountered this after almost completely rewriting the application to this library, perhaps there is some progress with fixing this error?

@lisindima Sorry for the slow progress; I'm struggling to find a solution. In the meantime, some workarounds would be:

  • Use FlowLinks for navigation in the child FlowStack.
  • Use a FlowPath in both FlowStacks and use FlowPathNavigator for navigation in the child FlowStack.
  • Don't nest FlowStacks at all, use just one.

I imagine that none of these are quite what you're hoping for, but they might help you for the time being. Thanks!

Thanks for your patience. I now understand the problem at least: the onChange in the child FlowStack doesn't fire if it's no longer onscreen. The solution will require a bit of refactoring.

@johnpatrickmorgan I encountered this problem while creating a child FlowStack. I hope you resolve this issue quickly.

Thanks for your patience. I believe this issue should now be resolved in v0.7.0, but I'd appreciate your feedback @lisindima @phamdinhduc795397. I had to amend the way nesting FlowStacks works, but it shouldn't affect your use case - please see the amended docs.

@johnpatrickmorgan In v0.7.0, use withNavigation: true in presenting child FlowStack was not working. It only works if called routes.presentCover(.child, withNavigation: true)

@johnpatrickmorgan In v0.7.0, use withNavigation: true in presenting child FlowStack was not working. It only works if called routes.presentCover(.child, withNavigation: true)

Thanks @phamdinhduc795397 - if I understand correctly, that's the intended behaviour. When nesting FlowStacks, the parent FlowStack determines whether the child FlowStack is presented with navigation, and the withNavigation parameter passed to the child FlowStack is ignored. I'll clarify that in the docs, thanks!

`struct ParentCoordinator: View {
enum Screen: Hashable {
case child
}

@State private var routes: Routes<Screen> = []

var body: some View {
    FlowStack($routes, withNavigation: true) {
        rootView()
        .flowDestination(for: Screen.self) { screen in
            switch screen {
            case .child:
                ChildCoordinator()
            }
        }
    }
}

@ViewBuilder
func rootView() -> some View {
    Button("present child") {
        routes.presentCover(.child)
    }
}

}`

`struct ChildCoordinator: View {
enum Screen: Hashable {}
@State private var routes: Routes = []

var body: some View {
    FlowStack($routes, withNavigation: true) {
        Text("ChildCoordinator")
            .navigationTitle("ChildCoordinator")
    }
}

}Use FlowStack($routes, withNavigation: true) in child ChildCoordinator,withNavigation: truewill be ignored, except you passwithNavigation: truewithroutes.presentCover(.child, withNavigation: true)`. In v0.6.4, it works well.

Спасибо за ваше терпение. Я считаю, что эта проблема должна быть решена в v0.7.0, но я был бы признателен за ваш отзыв@lisindima @phamdinhduc795397Мне пришлось изменить способ работы вложенных FlowStacks, но это не должно повлиять на ваш вариант использования — см. измененную документацию .

Hi, the error with which I opened this problem is fixed in the new version, thank you