Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation appears to be broken on iOS #12

Closed
MstrCamp opened this issue Jul 2, 2021 · 7 comments
Closed

Navigation appears to be broken on iOS #12

MstrCamp opened this issue Jul 2, 2021 · 7 comments

Comments

@MstrCamp
Copy link

MstrCamp commented Jul 2, 2021

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
@dbaroncelli
Copy link
Owner

Thanks for reporting this. I will have a look.

@Verdant90
Copy link

Also got this problem, any progress?

@azurlake
Copy link

azurlake commented Dec 20, 2021

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
Copy link

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.

@MvandeRuitenbeek
Copy link

MvandeRuitenbeek commented Feb 14, 2022

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.

@azurlake
Copy link

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.

@dbaroncelli
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants