johnpatrickmorgan / FlowStacks

FlowStacks allows you to hoist SwiftUI navigation and presentation state into a Coordinator
MIT License
783 stars 56 forks source link

Fix iOS 16 NavigationLink console warning #47

Closed DavidKmn closed 1 year ago

DavidKmn commented 1 year ago

This fixes the iOS 16 NavigationLink warning printed in the console as noted by #34

Tested using Xcode Beta 6.

johnpatrickmorgan commented 1 year ago

Thanks a lot for looking into this @DavidKmn ! Unfortunately, this change would break pushing more than 2 screens in a navigation stack. embedInNavigationView should only be true for the initial screen of a navigation stack, so with this change, the NavigationLink would no longer appear on the second screen.

DavidKmn commented 1 year ago

Nice catch I see. Yes I missed that, so it seems like what we actually want is to check whether the view is in a navigation stack (ie its either the first one via embedInNavigationView or it has been pushed). Is that right?

DavidKmn commented 1 year ago

Just pushed this check

johnpatrickmorgan commented 1 year ago

Thanks very much for the changes @DavidKmn. Unfortunately, even with this change, the warning is still logged when popping back more than one view (you can test in the example app). Also, people might be manually adding their own NavigationView, so it's not always possible to detect if there is a NavigationView in the hierarchy this way.

I think this is a SwiftUI bug to be honest, and I'm not sure this library can do anything to mitigate it. Your investigations helped me understand what seems to trigger the warning. E.g. I can trigger it with the following vanilla SwiftUI code:


import SwiftUI

@main
struct NavigationWarningLogDemoApp: App {
  var body: some Scene {
    WindowGroup {
      NavigationView {
        Content1View()
      }.navigationViewStyle(.stack)
    }
  }
}

struct Content1View: View {
  @State var pushing = false

  var body: some View {
    VStack {
      NavigationLink(destination: Content2View(popToRoot: { pushing = false }), isActive: $pushing, label: { Text("Go")})
    }
    .navigationTitle("1")
  }
}

struct Content2View: View {
  @State var pushing = false
  let popToRoot: () -> Void
  var body: some View {
    VStack {
      NavigationLink(destination: Content3View(popToRoot: popToRoot), isActive: $pushing, label: { Text("Go")})
      Button(action: popToRoot, label: { Text("Pop to root")})
    }
    .navigationTitle("2")
  }
}

struct Content3View: View {
  @State var pushing = false
  let popToRoot: () -> Void

  var body: some View {
    VStack {
      // The warning does not appear if you remove this NavigationLink.
      NavigationLink(destination: Text("4"), isActive: $pushing, label: { Text("Go")})
      // Tapping this button triggers a warning to be logged:
      //
      // NavigationLink presenting a value must appear inside a NavigationContent-based NavigationView. Link will be disabled.
      Button(action: popToRoot, label: { Text("Pop to root")})
    }
    .navigationTitle("3")
  }
}

I'll file a radar with this example code.

johnpatrickmorgan commented 1 year ago

Filed feedback FB11490806.

DavidKmn commented 1 year ago

Nice!

DavidKmn commented 1 year ago

Filed feedback FB11490806.

Any update on this?

ivarvanwooning commented 1 year ago

Also experiencing this issue. Any updates?

johnpatrickmorgan commented 1 year ago

No update so far on the feedback I'm afraid (FB11490806). I'll close this PR as it doesn't solve the issue sadly.