pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.55k stars 1.45k forks source link

Multiple levels of navigation #66

Closed boudavid closed 4 years ago

boudavid commented 4 years ago

Hey guys,

I'm trying to do something rather simple using SwiftUI and TCA which is to have multiple levels of navigation. Basically, I have 3 screens :

The code for each screen is very similar so I'll post only the second one but here's the full project.

import ComposableArchitecture
import SwiftUI

struct SecondState {
    var third: ThirdState?
}

enum SecondAction {
//    case tappedBackButton
    case tappedNextButton
    case dismissedThird
    case third(ThirdAction)
}

struct SecondEnvironment {}

let secondReducer = Reducer.combine(
    Reducer<SecondState, SecondAction, SecondEnvironment> { state, action, environment in
        switch action {
//        case .tappedBackButton:
//            return .none
        case .tappedNextButton:
            state.third = ThirdState()
            return .none
//        case .dismissedThird,
//             .third(.tappedBackButton):
        case .dismissedThird:
            state.third = nil
            return .none
        }
    },
    thirdReducer.optional.pullback(
        state: \.third,
        action: /SecondAction.third,
        environment: { _ in ThirdEnvironment() }
    )
)

struct SecondView: View {
    struct ViewState: Equatable {
        let isThirdActive: Bool
    }

    enum ViewAction {
//        case tappedBackButton
        case tappedNextButton
        case dismissedThird
    }

    let store: Store<SecondState, SecondAction>

    var body: some View {
        WithViewStore(store.scope(
            state: \.view,
            action: SecondAction.view
        )) { viewStore in
            VStack {
                NavigationLink(
                    destination: IfLetStore(
                        self.store.scope(
                            state: \.third,
                            action: SecondAction.third
                        ),
                        then: ThirdView.init(store:)
                    ),
                    isActive: viewStore.binding(
                        get: \.isThirdActive,
                        send: { $0 ? .tappedNextButton : .dismissedThird }
                    )
                ) {
                    Text("Go to next screen")
                }
            }
            .navigationBarTitle(
                "Second",
                displayMode: .large
            )
//            .navigationBarBackButtonHidden(true)
//            .navigationBarItems(leading: Button("Back") {
//                viewStore.send(.tappedBackButton)
//            })
        }
    }
}

extension SecondState {
    var view: SecondView.ViewState {
        SecondView.ViewState(isThirdActive: third != nil)
    }
}

extension SecondAction {
    static func view(_ viewAction: SecondView.ViewAction) -> Self {
        switch viewAction {
//        case .tappedBackButton:
//            return .tappedBackButton
        case .tappedNextButton:
            return .tappedNextButton
        case .dismissedThird:
            return .dismissedThird
        }
    }
}

(I commented some code about a custom back button with an action attached to it but that didn't solve the issue either.)

Here's what it looks like when I try to navigate between the screens :

Here's the output of the console when tapping on both navigation links :

received action:
  AppAction.first(
    FirstAction.tappedNextButton
  )
  AppState(
    first: FirstState(
−     second: nil
+     second: SecondState(
+       third: nil
+     )
    )
  )

received action:
  AppAction.first(
    FirstAction.second(
      SecondAction.tappedNextButton
    )
  )
  AppState(
    first: FirstState(
      second: SecondState(
−       third: nil
+       third: ThirdState()
      )
    )
  )

received action:
  AppAction.first(
    FirstAction.dismissedSecond
  )
  AppState(
    first: FirstState(
−     second: SecondState(
−       third: ThirdState()
−     )
+     second: nil
    )
  )

received action:
  AppAction.first(
    FirstAction.second(
      SecondAction.dismissedThird
    )
  )
  AppState(
    first: FirstState(
      second: nil
    )
  )

As you can see, the second screen is dismissed when I try to navigate to the third one which is also dismissed because the second property is nil. I tried to follow the examples of the repo but couldn't find a similar example with multiple levels of navigation. Why is the dismissedSecond action sent when pushing another screen on top of the second screen ? Is it a SwiftUI bug ?

I'd love to hear your thoughts on the matter and if you already implemented something similar.

stephencelis commented 4 years ago

@boudavid Thanks for the detailed report! You're hitting what appears to be a bug in SwiftUI. At this time it is very difficult to programmatically drive more than a single level in a navigation stack with state.

One workaround for iPhones is to add the stack navigation view style to the navigation view:

.navigationViewStyle(StackNavigationViewStyle())

This will break the column view style for iPads, so if you want universal support you'll need to account for this somehow.

Another issue you may come across is that deep linking more than a single level of navigation at once is currently not possible. You must wait a UI tick after programmatically navigating one level before navigating deeper.

I believe these issues have been filed as feedback with Apple but if you have the time it wouldn't hurt to dupe them with another example!

boudavid commented 4 years ago

Awesome, I’ll try the workaround. Thanks @stephencelis.

mbrandonw commented 4 years ago

Cool to close this @boudavid?

ghost commented 4 years ago

Hi,

Sorry for using a closed issue. Is it ok?

Did you find a way to overcome this issue @boudavid? I was able to create a small, working and non-working demo project that replicates this. It goes 3 levels deep in the navigation:

ContentView > 1st Level > 2nd Level > 3th Level

What seems to break the project, is when a reducer of the third level is combined from the second level.

Here's the non-working version (and then the change to the "working version" at the end):

Partly ignore the Tabbed navigation please, I was attempting to replicate a larger project.

ContentView.swift

import SwiftUI
import ComposableArchitecture

enum Tab {
    case dummy1
    case dummy2
    case content
}

struct ContentState: Equatable {
    var firstLevelState: FirstLevelState? = nil
    var isNavigationActive: Bool = false
    var selectedView: Tab = .dummy1
}

indirect enum ContentAction: Equatable {
    case setNavigation(isActive: Bool)
    case someView(FirstLevelAction)
    case tabSelected(Tab)
}

struct ContentEnvironment {}

let contentReducer: Reducer<ContentState, ContentAction, ContentEnvironment> = Reducer.combine(
    Reducer { state, action, _ in
        switch action {
        case .setNavigation(isActive: true):
            state.isNavigationActive = true
            state.firstLevelState = FirstLevelState()
            return .none
        case .setNavigation(isActive: false):
            state.isNavigationActive = false
            state.firstLevelState = nil
            return .none
        case let .tabSelected(tab):
            state.selectedView = tab
            return .none
        case .someView(_):
            return .none
        }
    },
    firstLevelReducer.optional.pullback(
        state: \ContentState.firstLevelState,
        action: /ContentAction.someView,
        environment: { _ in
            FirstLevelEnvironment()
        }
    )
)

struct ContentView: View {
    let store: Store<ContentState, ContentAction>

    var body: some View {
        WithViewStore(self.store) { viewStore in

            TabView(
                selection: viewStore.binding(
                    get: { $0.selectedView }, send: ContentAction.tabSelected)
            ) {

                NavigationView {
                    Text("Dummy1")
                }.tabItem {
                    Text("Dummy1")
                }.tag(Tab.dummy1)

                NavigationView {
                    Text("Dummy2")
                }.tabItem {
                    Text("Dummy2")
                }.tag(Tab.dummy2)

                NavigationView {
                    List {
                        NavigationLink(
                            destination: IfLetStore(
                                self.store.scope(
                                    state: { $0.firstLevelState },
                                    action: ContentAction.someView),
                                then: FirstLevelView.init(store:)
                            ),
                            isActive: viewStore.binding(
                                get: { $0.isNavigationActive },
                                send: ContentAction.setNavigation(isActive:)
                            ))
                        {
                            Text("Go to First ->")
                        }
                    }
                }
                .tabItem {
                    Text("Content")
                }.tag(Tab.content)

            }
        }
    }
}

FirstLevelView.swift

import SwiftUI
import ComposableArchitecture

struct FirstLevelState: Equatable {
    var secondLevelState: SecondLevelState? = nil
    var isNavigationActive = false
}

enum FirstLevelAction: Equatable {
    case setNavigation(isActive: Bool)
    case secondLevel(SecondLevelAction)
}

struct FirstLevelEnvironment {}

let firstLevelReducer: Reducer<FirstLevelState, FirstLevelAction, FirstLevelEnvironment> = Reducer.combine(
    Reducer { state, action, _ in
        switch action {
        case .setNavigation(isActive: true):
            state.isNavigationActive = true
            state.secondLevelState = SecondLevelState()
            return .none
        case .setNavigation(isActive: false):
            state.isNavigationActive = false
            state.secondLevelState = nil
            return .none
        case .secondLevel(_):
            return .none
        }
    },
    secondLevelReducer.optional.pullback(
        state: \FirstLevelState.secondLevelState,
        action: /FirstLevelAction.secondLevel,
        environment: { _ in
            SecondLevelEnvironment()
        }
    )
)

struct FirstLevelView: View {
    let store: Store<FirstLevelState, FirstLevelAction>

    var body: some View {
        WithViewStore(store) { viewStore in
            List {
                NavigationLink(
                    destination: IfLetStore(
                        self.store.scope(
                            state: { $0.secondLevelState },
                            action: FirstLevelAction.secondLevel),
                        then: SecondLevelView.init(store:)
                    ),
                    isActive: viewStore.binding(
                        get: { $0.isNavigationActive },
                        send: FirstLevelAction.setNavigation(isActive:)
                    ))
                {
                    Text("Go to Second ->")
                }
            }
        }
    }
}

SecondLevelView.swift

import SwiftUI
import ComposableArchitecture

struct SecondLevelState: Equatable {
    var thirdLevelState: ThirdLevelState? = nil
    var isNavigationActive = false
}

enum SecondLevelAction: Equatable {
    case setNavigation(isActive: Bool)
    case thirdLevel(ThirdLevelAction)
}

struct SecondLevelEnvironment {}

let secondLevelReducer: Reducer<SecondLevelState, SecondLevelAction, SecondLevelEnvironment> = Reducer.combine(
    Reducer { state, action, _ in
        switch action {
        case .setNavigation(isActive: true):
            state.isNavigationActive = true
            state.thirdLevelState = ThirdLevelState()
            return .none
        case .setNavigation(isActive: false):
            state.isNavigationActive = false
            state.thirdLevelState = nil
            return .none
        case .thirdLevel(_):
            return .none
        }
    },
    thirdLevelReducer.optional.pullback(
        state: \SecondLevelState.thirdLevelState,
        action: /SecondLevelAction.thirdLevel,
        environment: { _ in
            ThirdLevelEnvironment()
        }
    )
)

struct SecondLevelView: View {
    let store: Store<SecondLevelState, SecondLevelAction>

    var body: some View {
        WithViewStore(store) { viewStore in
            List {
                NavigationLink(
                    destination: IfLetStore(
                        self.store.scope(
                            state: { $0.thirdLevelState },
                            action: SecondLevelAction.thirdLevel),
                        then: ThirdLevelView.init(store:)
                    ),
                    isActive: viewStore.binding(
                        get: { $0.isNavigationActive },
                        send: SecondLevelAction.setNavigation(isActive:)
                    ))
                {
                    Text("Go to Third ->")
                }
            }
        }
    }
}

ThirdLevelView.swift

import SwiftUI
import ComposableArchitecture

struct ThirdLevelState: Equatable {}
enum ThirdLevelAction: Equatable {}
struct ThirdLevelEnvironment {}

let thirdLevelReducer = Reducer<ThirdLevelState, ThirdLevelAction, ThirdLevelEnvironment> { state, action, _ in
    .none
}

struct ThirdLevelView: View {
    let store: Store<ThirdLevelState, ThirdLevelAction>
    var body: some View {
        List {
            Text("Third Level!")
        }
    }
}

So, the code above, shows the issue happening.

Now get this, if in the SecondLevelView.swift, instead of:

WithViewStore(store) { viewStore in
    List {
        NavigationLink(
            destination: IfLetStore(
                self.store.scope(
                    state: { $0.thirdLevelState },
                    action: SecondLevelAction.thirdLevel),
                then: ThirdLevelView.init(store:)
            ),
            isActive: viewStore.binding(
                get: { $0.isNavigationActive },
                send: SecondLevelAction.setNavigation(isActive:)
            ))
        {
            Text("Go to Third ->")
        }
    }
}

We just navigate to a WorkingLevelView:

NavigationLink(destination: WorkingThirdLevelView()) {
    Text("Go to Third ->")
}

That is simply defined as:

struct WorkingThirdLevelView: View {
    var body: some View {
        List {
            Text("Working Third Level!")
        }
    }
}

Things start working!

I'm not sure if this validates the issue, or the fact that I have a demo working correctly indicates that it might not be a bug and we're missing something 🤔

Is it weird that the issue is gone when I navigate to a View without a Store and without the combined Reducer from the Previous View?

Sorry for the huge code paste, should be understandable I hope 😸

Edit: I'm using an iPad for this!

Also, like @stephencelis indicated, the

.navigationViewStyle(StackNavigationViewStyle())

Seems to fix the issue for the iPhone. For the iPad, the issue is "fixed" without using it, but I can't navigate to the last Screen with a Store and a reducer attached from the previous screen.

Anyways, I was just in the hopes of some development..

Thanks

boudavid commented 4 years ago

I ended up using @stephencelis workaround.

So you’re saying that might not be a SwiftUI bug ? I wanted to try multiple levels of navigation pushed programatically without using TCA at all to see if it was working but I don’t have much time right now.

ghost commented 4 years ago

Nyes.. 😕 First, I thought so, because I'm mostly on an iPad and the issue stopped happening when I removed the Store from the View. I created this whole demo replicating a working and non-working version, but then when I tested in an iPhone, it started happening again.

But get this,

import SwiftUI

struct ContentView: View {
    var body: some View {
        NavigationView {
            List {
                NavigationLink(destination: DetailView()) {
                    Text("Go")
                }
            }
        }
    }
}

struct DetailView: View {
    var body: some View {
        List {
            NavigationLink(destination: DetailView()) {
                Text("Go")
            }
        }
    }
}

The code above works, it will navigate to as many levels as needed on an iPhone Sim aparently.

It's probably an Apple bug.. But because it's so hard to pin point, sometimes it feels that it's not.

Anyways, feel free to just ignore everything above. This at least helped me put my thoughts in order. Hopefully WWDC will bring with it lot's of fixes and improvements to all things SwiftUI 🙏

Thanks again and sorry for the huge code spam

stephencelis commented 4 years ago

@tarjamorgado Yeah, programmatic navigation into multiple layers of a navigation stack is unfortunately impossible right now without some pretty gross workarounds. We're hoping Apple has some good fixes coming next month, though!

ferologics commented 3 years ago

I just want to add that this is still the case today. Using .navigationViewStyle(StackNavigationViewStyle()) on NavigationView solves a lot of funky NavigationLink behavior like multiple pushes/pops instead of just one. Maybe WWDC 2021 will finally address this pain point🤞