D-KMP-sample icon indicating copy to clipboard operation
D-KMP-sample copied to clipboard

Navigation appears to be broken on iOS

Open MstrCamp opened this issue 3 years ago • 6 comments

When setting up Navigation deeper then Level 2 on iOS, it stops working. As soon as a Level 3 screen is opened, the Navigation jumps back to Level 1. On Android, this does not happen, the Level 3 screen is displayed correctly.

Steps to reproduce:

  1. Create a basic new screen in the shared library

NEW file: TestScreenState.kt:

data class TestScreenState (
    val isLoading: Boolean = false
): ScreenState

NEW file: TestScreenInit.kt:

data class TestScreenParams(val name: String) : ScreenParams

fun Navigation.initTestScreen(params: TestScreenParams) = ScreenInitSettings (
    title = params.name,
    initState = { TestScreenState(isLoading = true)},
    callOnInit = {
        stateManager.updateScreen(TestScreenState::class) {
            it.copy(
                isLoading = false
            )
        }
    }
)

ScreenEnum.kt:

enum class Screen(
    val asString: String,
    val navigationLevel : Int = 1,
    val initSettings: Navigation.(ScreenIdentifier) -> ScreenInitSettings,
    val stackableInstances : Boolean = false,
) {
    CountriesList("countrieslist", 1, { initCountriesList(it.params()) }, true),
    CountryDetail("country", 2, { initCountryDetail(it.params()) }),
    TestScreen("testscreen", 3, {initTestScreen(it.params())}),
}
  1. Add the screen to the iOS App, so that it can be navigated to from within CountryDetailScreen

NEW file: TestScreen,swift:

struct TestScreen: View {
    var testScreenState: TestScreenState
    
    var body: some View {
        VStack {
            if testScreenState.isLoading {
                LoadingScreen()
            } else {
                Text("Hello, World!")
            }
        }
    }
}

ScreenPicker.swift:

case .countrydetail:
    CountryDetailScreen(
        countryDetailState: self.stateProvider.getToCast(screenIdentifier: sId) as! CountryDetailState,
        ontestScreenOpened: {name in self.navigate(.testscreen, TestScreenParams(name: name))}
    )
case .testscreen:
    TestScreen(
        testScreenState: self.stateProvider.getToCast(screenIdentifier: sId) as! TestScreenState
    )

CountryDetailScreen.swift:

NavLink(linkFunction: {ontestScreenOpened("population")}) {
    DataElement(label: "total population", value: data.population)
}
  1. Start App and navigate to the newly created screen

MstrCamp avatar Jul 02 '21 15:07 MstrCamp

Thanks for reporting this. I will have a look.

dbaroncelli avatar Jul 10 '21 12:07 dbaroncelli

Also got this problem, any progress?

Verdant90 avatar Nov 06 '21 13:11 Verdant90

Same problem. Has anyone sorted this out?

In my case, it looks like inside the NavLink code..:

NavigationLink(
                destination: LazyDestinationView(
                    appObj.dkmpNav.screenPicker(linkFunction())
                        .navigationBarTitle(appObj.dkmpNav.getTitle(screenIdentifier: linkFunction()), displayMode: .inline)
                        .onDisappear {
                            let screenIdentifier = linkFunction()
                            //print("onDisappear: "+screenIdentifier.URI)
                            if appObj.dkmpNav.isInCurrentVerticalBackstack(screenIdentifier: screenIdentifier) {
                                //print("confimed disappear")
                                self.selected = false
                                isActive.wrappedValue = false
                                appObj.dkmpNav.exitScreen(screenIdentifier: screenIdentifier, triggerRecomposition: false)
                            }
                        }
                ),
                isActive: isActive
            ) {
                content()
            }

... onDisappear() is being called prematurely for my navigation level 3 view (immediately after being shown)

azurlake avatar Dec 20 '21 06:12 azurlake

OK, investigating further, this problem seems to be a (yet another) bug in SwiftUI: https://forums.swift.org/t/14-5-beta3-navigationlink-unexpected-pop/45279/38 https://stackoverflow.com/questions/66559814/swiftui-navigationlink-pops-out-by-itself

I see the code for NavLink + the Router view implements some of the fixes suggested in the forums... but there must be something else since it's not working for me. I will post back when a find a good solution - the empty NavigationLink did not work for me.

azurlake avatar Dec 22 '21 06:12 azurlake

Had the same issue. Tried all the current workarounds as pointed out by @azurlake but unfortunately none of them worked for me. However, I found a workaround for the D-KMP sample. You only need to change a few lines of the iOS code. Not sure if it covers all navigation issues, but up to now it served me fairly well whenever I need multiple nested navigationlinks.

The problem is that any change to the appObj, such as changing anything in your model causes swift to redraw everything. NavigationLinks get redrawn as well, causing the .ondisapper event in NavLink to get triggered, and thus all related subsequent screens to disappear.

To prevent it, we must check within the .ondisappear event whether the change comes bottom up (i.e. from the main view redraw). If that is the case then just neglect it.

  1. add a bool property mainChanged (or whatever you want) in AppObservableObject
class AppObservableObject: ObservableObject {
    let model : DKMPViewModel = DKMPViewModel.Factory().getIosInstance()
    var dkmpNav : Navigation {
        return self.appState.getNavigation(model: self.model)
    }
    @Published var appState : AppState
    var mainChanged : Bool = false             // <- ADD THIS LINE. IT KEEPS TRACK OF ANY NEW REDRAWS BOTTOM-UP, INITIATED BY A SWIFT REDRAW AT THE MAIN LEVEL.

    
    init() {
        // "getDefaultAppState" and "onChange" are iOS-only DKMPViewModel's extension functions, defined in shared/iosMain
        self.appState = model.getDefaultAppState()
        model.onChange { newState in
            self.appState = newState
//            print("D-KMP-SAMPLE: recomposition Index: "+String(newState.recompositionIndex))
        }
    }
}
  1. In MainView, set this new property to true so it will be true on each swift redraw.
struct MainView: View {
    @EnvironmentObject var appObj: AppObservableObject
    
    var body: some View {
        let dkmpNav = appObj.dkmpNav
        appObj.mainChanged = true // <- ADD THIS LINE. WILL BE SET ON EACH REDRAW
  1. In Router -> NavLink, change this:
                let isActive = Binding<Bool> (
                    get: {
                        return selected && appObj.dkmpNav.isInCurrentVerticalBackstack(screenIdentifier: linkFunction())
                    },
                    set: { isActive in
                        appObj.mainChanged = false // <- ADD THIS LINE. IF isActive IS BEING SET, THE MAINCHANGE IS OVER 

                        if isActive {
                            let screenIdentifier = linkFunction()
                            appObj.dkmpNav.navigateByScreenIdentifier(screenIdentifier: screenIdentifier)
                            self.selected = true
                        }
                    }
                )

  1. Finally, in Router -> Navlink -> NavigationLink, change this:
                NavigationLink(
                    destination: LazyDestinationView(
                        appObj.dkmpNav.screenPicker(linkFunction())
                            .navigationBarTitle(appObj.dkmpNav.getTitle(screenIdentifier: linkFunction()), displayMode: .inline)
                            .onDisappear {
                                let screenIdentifier = linkFunction()
                                print("onDisappear: "+screenIdentifier.URI)
                                if appObj.dkmpNav.isInCurrentVerticalBackstack(screenIdentifier: screenIdentifier) {
                                    if (!appObj.mainChanged){   // <-- ADD THIS CHECK. ANY SIMPLE REDRAWS WILL NOW NOT CAUSE YOUR NAVIGATION STACK TO COLLAPSE
                                        print("confimed disappear")
                                        self.selected = false
                                        isActive.wrappedValue = false
                                        appObj.dkmpNav.exitScreen(screenIdentifier: screenIdentifier, triggerRecomposition: false)
                                    }
                                }
                            }
                    ),
                    isActive: isActive
                ) {
                    content()
                }

This code also prevents the Level1 navigationlinks to get frozen (selected but not clickable) once returning to level 1 from a deeply nested navigation. Occasionally navigation link does bump in and out if you are in a nested situation and tap on the navigation bar to initiate a different Level 1 route and then go back to the first route. Fortunately it ends up in the correct position.

Hope it works for others too. Loving the D-KMP sample, and hoping for future releases now the KMM effort gets more traction.

MvandeRuitenbeek avatar Feb 14 '22 15:02 MvandeRuitenbeek

Had the same issue. Tried all the current workarounds as pointed out by @azurlake but unfortunately none of them worked for me. However, I found a workaround for the D-KMP sample. You only need to change a few lines of the iOS code. Not sure if it covers all navigation issues, but up to now it served me fairly well whenever I need multiple nested navigationlinks.

The problem is that any change to the appObj, such as changing anything in your model causes swift to redraw everything. NavigationLinks get redrawn as well, causing the .ondisapper event in NavLink to get triggered, and thus all related subsequent screens to disappear.

To prevent it, we must check within the .ondisappear event whether the change comes bottom up (i.e. from the main view redraw). If that is the case then just neglect it.

  1. add a bool property mainChanged (or whatever you want) in AppObservableObject
class AppObservableObject: ObservableObject {
    let model : DKMPViewModel = DKMPViewModel.Factory().getIosInstance()
    var dkmpNav : Navigation {
        return self.appState.getNavigation(model: self.model)
    }
    @Published var appState : AppState
    var mainChanged : Bool = false             // <- ADD THIS LINE. IT KEEPS TRACK OF ANY NEW REDRAWS BOTTOM-UP, INITIATED BY A SWIFT REDRAW AT THE MAIN LEVEL.

    
    init() {
        // "getDefaultAppState" and "onChange" are iOS-only DKMPViewModel's extension functions, defined in shared/iosMain
        self.appState = model.getDefaultAppState()
        model.onChange { newState in
            self.appState = newState
//            print("D-KMP-SAMPLE: recomposition Index: "+String(newState.recompositionIndex))
        }
    }
}
  1. In MainView, set this new property to true so it will be true on each swift redraw.
struct MainView: View {
    @EnvironmentObject var appObj: AppObservableObject
    
    var body: some View {
        let dkmpNav = appObj.dkmpNav
        appObj.mainChanged = true // <- ADD THIS LINE. WILL BE SET ON EACH REDRAW
  1. In Router -> NavLink, change this:
                let isActive = Binding<Bool> (
                    get: {
                        return selected && appObj.dkmpNav.isInCurrentVerticalBackstack(screenIdentifier: linkFunction())
                    },
                    set: { isActive in
                        appObj.mainChanged = false // <- ADD THIS LINE. IF isActive IS BEING SET, THE MAINCHANGE IS OVER 

                        if isActive {
                            let screenIdentifier = linkFunction()
                            appObj.dkmpNav.navigateByScreenIdentifier(screenIdentifier: screenIdentifier)
                            self.selected = true
                        }
                    }
                )
  1. Finally, in Router -> Navlink -> NavigationLink, change this:
                NavigationLink(
                    destination: LazyDestinationView(
                        appObj.dkmpNav.screenPicker(linkFunction())
                            .navigationBarTitle(appObj.dkmpNav.getTitle(screenIdentifier: linkFunction()), displayMode: .inline)
                            .onDisappear {
                                let screenIdentifier = linkFunction()
                                print("onDisappear: "+screenIdentifier.URI)
                                if appObj.dkmpNav.isInCurrentVerticalBackstack(screenIdentifier: screenIdentifier) {
                                    if (!appObj.mainChanged){   // <-- ADD THIS CHECK. ANY SIMPLE REDRAWS WILL NOW NOT CAUSE YOUR NAVIGATION STACK TO COLLAPSE
                                        print("confimed disappear")
                                        self.selected = false
                                        isActive.wrappedValue = false
                                        appObj.dkmpNav.exitScreen(screenIdentifier: screenIdentifier, triggerRecomposition: false)
                                    }
                                }
                            }
                    ),
                    isActive: isActive
                ) {
                    content()
                }

This code also prevents the Level1 navigationlinks to get frozen (selected but not clickable) once returning to level 1 from a deeply nested navigation. Occasionally navigation link does bump in and out if you are in a nested situation and tap on the navigation bar to initiate a different Level 1 route and then go back to the first route. Fortunately it ends up in the correct position.

Hope it works for others too. Loving the D-KMP sample, and hoping for future releases now the KMM effort gets more traction.

This is great news. I can confirm it works perfectly. Now I'm a little bit stuck at a similar problem: how would you create a similar NavLink but with the capability to be fired programmatically? So far, this NavLink triggers isActive set function when the user taps on the visible NavLink, and that looks like it's a requirement so that the view is pushed into the stack. Any help would be appreciated.

azurlake avatar May 11 '22 22:05 azurlake

I just published an update. Now the navigation seems fully fixed, leveraging the new iOS 16 navigation under the hood.

dbaroncelli avatar Feb 16 '23 22:02 dbaroncelli