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

When replacing a root in a modal context, present a new modal flow #164

Closed

Conversation

olivaresf
Copy link
Member

Currently there's no way to jump from one modal flow to another. The app has to manually dismiss the first modal flow, then request a new modal flow by creating a new visit proposal with a modal context. Here's my proposal: make replace_root consider the given context.

  • If given context is default, then behavior is unchanged: Dismiss if modal controller, then pop to root, then replace root controller on main stack.
  • If given context is modal then behavior is now: Dismiss if modal controller, then present a new modal.

In my mind, this makes sense since replacing a modal root restarts the flow, but I'd like to get your thoughts before I move forward with tests.

@olivaresf olivaresf marked this pull request as draft November 20, 2023 19:03
@jayohms
Copy link
Collaborator

jayohms commented Nov 20, 2023

@olivaresf can you give an example of when you'd want this behavior? Android doesn't allow replacing the "root" with a modal context url. "Root" is always considered the root of the default context.

PR where it was implemented: hotwired/turbo-android#136

@olivaresf
Copy link
Member Author

@olivaresf can you give an example of when you'd want this behavior? Android doesn't allow replacing the "root" with a modal context url. "Root" is always considered the root of the default context.

Of course! For example, a menu presented modally that leads into another, unrelated modal flow. The presenting modal menu may not want to share its stack with the new modal flow due to size (e.g. if the presented modal was only half the screen and the unrelated modal flow requires full height).

In this case, the app is responsible for dismissing the first flow, then calling on TurboNavigator to present a new modal flow. I feel TurboNavigator should handle this interaction. You just call route(url:) and using the presentation and context, it does the right thing.

I think this is a scenario we should support, but it's not a huge deal if we don't feel this is the right approach or if we don't want to solve for this.

@joemasilotti
Copy link
Member

I want to make sure I understand this entirely. Currently, if you are in a modal and route to a new modal flow it will push on the modal context, ending with 2 screens on the modal stack. But you want to dismiss then present, ending with a single screen on the modal context. Is that correct?

@olivaresf
Copy link
Member Author

I want to make sure I understand this entirely. Currently, if you are in a modal and route to a new modal flow it will push on the modal context, ending with 2 screens on the modal stack. But you want to dismiss then present, ending with a single screen on the modal context. Is that correct?

Yes! That's correct.

@olivaresf
Copy link
Member Author

@joemasilotti FYI, I discussed this PR with @jayohms today and I'm going to try something different for this scenario. I think it's likely to be a common scenario in mobile, so it's worth looking into.

For now, I'm closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants