johnpatrickmorgan / TCACoordinators

Powerful navigation in the Composable Architecture via the coordinator pattern
MIT License
439 stars 42 forks source link

[0.4.3] Multiple pushed views being erroneously removed from stack #53

Closed ChloeMayhewELT closed 1 year ago

ChloeMayhewELT commented 1 year ago

In our app we have a coordinator that can be minimized, so the user can return to it later. It works fine if only the root view and one pushed view exist. But anything more than that and when maximized an .updateRoutes action is received which removes all but the root view and the first pushed view.

Obviously this is an unusual use-case. Could well be something strange that we've done. But it seems like the .updateRoutesaction is coming from within the internals of TCACoordinators. Hoping someone can help with this.

NOTE: Also, ideally when maximizing the process wouldn't be animated. But we can live with that if that's just a by product of the the coordinator works. The main problem is the screens disappearing.

Package versions ComposableArchitecture version 0.55.1 TCACoordinators version 0.4.3

Expected behavior When the custom minimization is applied, and then reversed to maximize the content, we would not expect any of the pushed views to be removed from the stack.

Actual behavior All but the root view and the first pushed view get removed.

Steps to reproduce Example project which displays the problem: https://github.com/ChloeMayhewELT/MinimizableCoordinatorBug

  1. Launch the app
  2. Tap the 'Display content' button, which will present a modal in a navigation view.
  3. The content of the modal has a "Push another view" button. Each time this is pressed another view of the same type will be pushed to the stack.
  4. Once there are > 2 views in the stack, tap the down chevron in trailing position of the nav bar. This will minimize all of the content into a banner at the bottom landing screen.
  5. Tap the banner to re-expand the views.
johnpatrickmorgan commented 1 year ago

Thanks @ChloeMayhewELT for such a thorough explanation and reproduction! The issue as I understand it is this:

There are two workarounds I see:

        .ifLet(\.content, action: /Action.content) {
            Minimizable<MultiScreenCoordinator>(child: MultiScreenCoordinator.init)
            Reduce { state, action in
                switch action {
                case .binding(.set(\.$isMinimized, false)):
                    let routesToRestore = state.child.routes
                    return EffectTask<MultiScreenCoordinator.Action>.routeWithDelaysIfUnsupported([]) {
                        $0 = routesToRestore
                    }
                    .map(Minimizable.Action.child)
                default:
                    return .none
                }
            }
        }
ChloeMayhewELT commented 1 year ago

Thanks for taking a look at this so quickly, @johnpatrickmorgan. Much appreciated!

I had tried EffectTask.routeWithDelaysIfUnsupported when initially investigating the issue, but was clearly using it in entirely the wrong place. Using it here does indeed resolve the issue. Thanks for the fix!