StanfordSpezi / SpeziOnboarding

Spezi Onboarding module to inform a user or retrieve consent for a study participation
https://swiftpackageindex.com/StanfordSpezi/SpeziOnboarding/documentation/
MIT License
11 stars 5 forks source link

Onboarding cannot handle nested NavigationStack #21

Closed Supereg closed 4 months ago

Supereg commented 10 months ago

Description

To my current observation, it is not possible to use NavigationStacks inside a onboarding step. The App will crash without further information.

Reproduction

  1. Place a view like below in any of your onboarding steps which is not the first one (the first one renders without issues).
  2. Press the button on the previous onboarding view to navigate to the next view that contains a NavigationStack.
  3. Crash
struct ExampleView: View {
    var body: some View {
        NavigationStack {
            Text("Hello World")
        }
    }
}

Expected behavior

The onboarding flow should complete without issues.

Additional context

If one doesn't use the simple example above, but e.g. the new AccountSetup view from the upcoming SpeziAccount release, one receives, additionally, the following log output from SwiftUI.

A NavigationLink is presenting a value of type “OnboardingStepIdentifier” but there is no matching navigationDestination declaration visible from the location of the link. The link cannot be activated.

Note: Links search for destinations in any surrounding NavigationStack, then within the same column of a NavigationSplitView.

Code of Conduct

PSchmiedmayer commented 10 months ago

Thank you @Supereg! @philippzagar Do you have an idea where this might originate from and if we figure out a workaround here? Or is this a bigger SwiftUI issue?

philippzagar commented 10 months ago

Hey @Supereg @PSchmiedmayer, thanks for raising the issue! So, I haven't investigated the problem fully yet, but I'm not sure if nested NavigationStack's are actually a thing in SwiftUI as they can interfere with each other. As far as I'm reading in some forums, this isn't really encouraged and leads to bizarre behavior. Still, the crash within the SpeziOnboarding module shouldn't happen, I will take a look into that more closely soon.

Here are some of the (rare) comments I found online:

Regarding your additional NavigationStack: May there be an option to use OnboardingNavigationPath/append(customView:), OnboardingNavigationPath/append(customViewInit:) and OnboardingNavigationPath/removeLast() in order to achieve your desired functionality? Those methods exist for exactly these more "customized" onboarding navigation flows. Or does that lead to issues as you are dealing with separate modules?

PSchmiedmayer commented 10 months ago

Thank you for the context @philippzagar! I have seen NavigationStack nesting properly working and being quite flexible. I had a conversation with @Supereg, and he had some good ideas about where this might originate from and how he handles NavigationStacks in SpeziAccount. We can probably use that as a good starting point to investigate this, @Supereg can weigh in here a bit more 👍

philippzagar commented 10 months ago

@Supereg @PSchmiedmayer Just tested: Crash occurring only under iOS 16, no crash for iOS 17 beta. However, iOS 17 navigation behavior is still very weird as the onboarding step wrapped inside a NavigationStack is only rendered for around half a second, then disappears (meaning no "Next" button to get to the next onboarding step). Back navigation is still possible. However, after navigating back, the NavigationPath from the OnboardingStack still contains the step which is wrapped in an additional NavigationStack, meaning the overall navigation behavior is off.

I'm in closer contact with @Supereg, let's see if we are able to fix this issue without talking to the SwiftUI team.

PSchmiedmayer commented 10 months ago

@philippzagar Thank you!

Supereg commented 9 months ago

Okay, it seems like this is more complicated than expected. To reproduce the issue I modified the TestApp of SpeziOnboarding on the explore-nested-navigation-stacks branch.

Observations

The issue boils down to placing a NavigationStack inside a .navigationDestination modifier. Just regularly placing a NavigationStack inside a NavigationStack (e.g. wrapping the first view inside a NavigationStack) works fine. I'm not sure how we would work around this issue.

PSchmiedmayer commented 9 months ago

Updated the Navigation Stack Reproduction with a few elements that help us to reproduce the issue: NavigationStackReproducion.zip

Will reach out once I have anything or some good workarounds. For now https://github.com/StanfordSpezi/SpeziAccount/pull/23 is a good workaround and seems to work. Would be great if we could nevertheless take a look at the issue for future considerations.

Supereg commented 9 months ago

I don't think the Reproduction Code is really painting the issue we are having?

What the current version from Paul shows, is that you cannot reach the outer navigationDestination from the inner NavigationStack. This might be unexpected, and it also messes with any further navigation, it is not the issues we are facing. Or at least we fail at an earlier point.

Our issue is (currently), that you cannot place a NavigationStack inside a navigationDestination modifier (see https://github.com/StanfordSpezi/SpeziOnboarding/issues/21#issuecomment-1721165595). Doing so will navigate to the view in the modifier, but will instantly navigate back to the root view. Similar to the current version, it will permanently mess with the NavigationPath. I assume once this is fixed, that we would experience the above issue once we try to actually perform any navigation steps?

I attached the modified version here: NavigationStackReproduction.zip.

Seems like there are a lot more quirks.

philippzagar commented 4 months ago

After some deliberation, we came to the conclusion that we can circumvent this limitation of SwiftUI with workarounds in the SpeziAccount module so we're not required to use a NavigationStack within the navigationDestination modifier.