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

[XCode 14 beta] NavigationLink presenting a value must appear inside a NavigationContent-based NavigationView. Link will be disabled. #34

Open
josephktcheung opened this issue Jun 13, 2022 · 7 comments

Comments

@josephktcheung
Copy link

Hi,

I ran the example app in XCode 14 beta and get the below warning in console:

2022-06-13 11:52:11.725672+0800 FlowStacksApp[77257:2136526] [SwiftUI] NavigationLink presenting a value must appear inside a NavigationContent-based NavigationView. Link will be disabled.

Seems like FlowStacks needs to refactor quite a bit to support the new Navigation API.

Best,
Joseph

@josephktcheung
Copy link
Author

NavigationView is deprecated https://developer.apple.com/documentation/swiftui/navigationview

Screenshot 2022-06-13 at 11 58 18 AM

I wonder how FlowStacks would work using NavigationStack and NavigationSplitView

@johnpatrickmorgan
Copy link
Owner

Thanks for raising this issue @josephktcheung.

I think that the NavigationLink presenting a value... warning must be spurious, as there are no NavigationLinks that present values in use (FlowStacks uses the old NavigationLink format that takes a destination view). I haven't been able to create a minimal reproduction yet to prove that assumption though.

As for how FlowStacks might change in light of the new navigation APIs, I can see two options:

  • FlowStacks keeps the API it currently has. FlowStacks has some advantages over the new navigation APIs:
    • it has more type safety in guaranteeing that there will be a view builder for any screen data you push.
    • it can be used for presenting sheets and modals too.
    • it works on pre-SwiftUI 4 OSes.
  • FlowStacks evolves to match the new navigation APIs, e.g. there will be a FlowStack, FlowPath and flowDestination to match NavigationStack, NavigationPath and navigationDestination. This would embrace the greater flexibility these new APIs have. I'm sure this is possible, having implemented a backport of the new navigation APIs.

@josephktcheung
Copy link
Author

Thanks for raising this issue @josephktcheung.

I think that the NavigationLink presenting a value... warning must be spurious, as there are no NavigationLinks that present values in use (FlowStacks uses the old NavigationLink format that takes a destination view). I haven't been able to create a minimal reproduction yet to prove that assumption though.

As for how FlowStacks might change in light of the new navigation APIs, I can see two options:

  • FlowStacks keeps the API it currently has. FlowStacks has some advantages over the new navigation APIs:

    • it has more type safety in guaranteeing that there will be a view builder for any screen data you push.
    • it can be used for presenting sheets and modals too.
    • it works on pre-SwiftUI 4 OSes.
  • FlowStacks evolves to match the new navigation APIs, e.g. there will be a FlowStack, FlowPath and flowDestination to match NavigationStack, NavigationPath and navigationDestination. This would embrace the greater flexibility these new APIs have. I'm sure this is possible, having implemented a backport of the new navigation APIs.

I vote for option 2 since new Navigation API will replace NavigationView after iOS 16. We also want to know how we can apply coordinator pattern to new API.

@bbernberg
Copy link

I actually found a simpler way to comply with NavigationStack approach -- Apple (quietly) introduced this method in iOS 16 along with NavigationStack. While they state that binding to a path is preferred, this method works just fine, is not deprecated and fits in very nicely with the current FlowStacks implementation. The only steps required are modifications to Node.swift:

  1. Change the NavigationView to a NavigationStack
  2. Remove the background view/Navigation Link code and replace it with the navigationDestination view modifier, as seen here:

Screenshot 2023-04-12 at 11 13 25 AM

This, of course, is also only available on iOS 16 and above but I think it's a great solution otherwise.

@johnpatrickmorgan
Copy link
Owner

@bbernberg Thanks a lot for the suggestion! I think giving users the option to use NavigationStack on iOS 16 and above would be a useful feature in other ways too.

@karthiikmk
Copy link

karthiikmk commented Aug 3, 2023

Hey @bbernberg, it looks like the solution you provided above isn't working in the following scenario:

@state var routes = [.root(.recents, embedInNavigationView: true), .push(.chat)]

When there's a single route in the array, the navigation seems to be working, but with multiple routes, it's not functioning as expected.

Could you please share any forked repo or code snippet with a working solution for this issue? I'd really appreciate it. Thanks!

@bbernberg
Copy link

@karthikAdaptavant I've been using this fork/branch successfully with multiple routes in the array without issue.

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

4 participants