FlowStacks icon indicating copy to clipboard operation
FlowStacks copied to clipboard

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

Open josephktcheung opened this issue 2 years ago • 7 comments

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 avatar Jun 13 '22 03:06 josephktcheung

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

josephktcheung avatar Jun 13 '22 03:06 josephktcheung

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.

johnpatrickmorgan avatar Jun 15 '22 22:06 johnpatrickmorgan

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.

josephktcheung avatar Jun 17 '22 09:06 josephktcheung

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.

bbernberg avatar Apr 12 '23 15:04 bbernberg

@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.

johnpatrickmorgan avatar Apr 13 '23 23:04 johnpatrickmorgan

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!

karthiikmk avatar Aug 03 '23 09:08 karthiikmk

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

bbernberg avatar Aug 04 '23 14:08 bbernberg