reactiveui / rfcs

RFCs for changes to ReactiveUI
https://reactiveui.net/rfcs
5 stars 5 forks source link

RFC: Stack Routing API #20

Open RLittlesII opened 6 years ago

RLittlesII commented 6 years ago

Add Stack Routing API

Summary

This RFC is based on https://github.com/reactiveui/ReactiveUI/issues/1107

The current implementation of RoutingState exposes navigation as a set of commands on an ObservableCollection<IRoutableViewModel>

Xamarin.Forms has built in navigation based on a stack data structure.

public interface INavigation
{
  IReadOnlyList<Page> ModalStack { get; }

  IReadOnlyList<Page> NavigationStack { get; }

  void InsertPageBefore(Page page, Page before);

  Task<Page> PopAsync();

  Task<Page> PopAsync(bool animated);

  Task<Page> PopModalAsync();

  Task<Page> PopModalAsync(bool animated);

  Task PopToRootAsync();

  Task PopToRootAsync(bool animated);

  Task PushAsync(Page page);

  Task PushAsync(Page page, bool animated);

  Task PushModalAsync(Page page);

  Task PushModalAsync(Page page, bool animated);

  void RemovePage(Page page);
}

Motivation

The current RoutingState currently has no support for the following concerns:

  1. Animation
  2. Navigation and Modal stack
  3. Passing parameters on navigation

I am proposing we introduce a reactive abstraction base on Custom Routing in ReactiveUI. This would provide view model based navigation and better reflect stack navigation patterns.

public interface IRoutingStack
{
    IObservable<IImmutableList<IRoutableViewModel>> ModalStack { get; }

    IObservable<IImmutableList<IRoutableViewModel>> PageStack { get; }

    IView View { get; }

    IObservable<Unit> PopModal(bool animate = true);

    IObservable<Unit> PopPage(bool animate = true);

    IObservable<Unit> PopTo<TViewModel>() where TViewModel : IRoutableViewModel;

    IObservable<Unit> PopToRootPage(bool animate = true);

    IObservable<Unit> PushModal(IRoutableViewModel modal, string contract = null);

    IObservable<Unit> PushPage(IRoutableViewModel page, string contract = null, bool resetStack = false, bool animate = true);

    IObservable<IRoutableViewModel> TopModal();

    IObservable<IRoutableViewModel> TopPage();
}

Discussion

I know there are benefits to the current RoutingState implementation, I want to make this an option for stack based navigation platforms.

Questions

How do we handle platforms that don't have a concept of a modal stack?

If we implement passing parameters does it make sense for all platforms?

Do we allow consumers to implement their own IRoutingStack and register it for use with ReactiveUI?

Are there any performance concerns or improvements that could come from this change?

anaisbetts commented 6 years ago

A few thoughts on this:

  1. It seems like ModalStack and PageStack act more like Behaviors (i.e. properties that always have a current value), so it would probably be way more ergonomic if IRoutingStack was a ReactiveObject that you can WhenAny on. This simplifies TopModal and TopPage too because it can just be an OAPH

  2. A lot of these fancy list types just get in the way, a regular IEnumerable is probably fine here (or IReadOnlyList maybe). tbh I'd probably just use a regular List.

  3. While Xamarin lets you have a stack of modal dialogs, is that really a Good Idea from a UX perspective? I would argue that you should have exactly One modal dialog slot active at any time, and that calling PushModal when a Modal is already active should just replace the previous one completely.

Do we allow consumers to implement their own IRoutingStack and register it for use with ReactiveUI?

I'd argue RoutingStack is more like List or String - it's really just a Bag of Data, so people shouldn't need to write a custom implementation (just like we don't have IString). People will ask for it, and you should Tell Them No :) Instead, make RoutingStack so easy to construct (i.e. with a constructor that optionally lets you fill in the stack explicitly for test time) and hackable that people won't need to.

RLittlesII commented 6 years ago

@paulcbetts The moments where words matter. IRoutingStack should have been IRoutingStackService or IRouter.

I agree that the collection of data can be as basic as possible.

  1. ... I would argue that you should have exactly one modal dialog slot active at any time, and that calling PushModal when a Modal is already active should just replace the previous one completely.

I agree with there should only ever be a single modal present at a time. I am not sure that ReactiveUI should impose a limit in it's implementation that doesn't exist on the target platform.

glennawatson commented 6 years ago

I would seperate the concepts of modal and navigation into two instances of the same interface. That way it's easier for the user to add new stacks to support y platform.

glennawatson commented 6 years ago

Also reduces the number of methods in your interfaces eg navigation.Modal.Pop(true)

glennawatson commented 5 years ago
BoutemineOualid commented 5 years ago

Hello everyone,

I think these features are worth adding to the framework:

glennawatson commented 5 years ago

Cool thanks @bouteminequalid I'm sure Rodney will take it into consideration

RLittlesII commented 5 years ago

@BoutemineOualid All of these I believe are already captured on the Sextant Repository . If not feel free to make sure they are captured.

mediabuff commented 3 years ago

1) The API seems to lack 'forward view' stack. Platforms like WPF - frame based navigation support these 2) Additionally, these platforms persist the 'view' state/visual tree - on their corresponding stacks. Thus the there are two stack model - one based on identity and other based on content. Which would the 'view model' navigation support ? is the view model re-created or cached ? 3) Don't think there should be separate stack for modal and non-modal. The modal entries should stack on non-modal. 4) what about memory cleanup - either of the views or view models in all scenarios

I have a working WPF version, based on WPF-Frame.

RLittlesII commented 3 years ago

@mediabuff

  1. I am not sure I understand. I've not worked much with WPF
  2. My thought would be the content. The ViewModel should be persisted as the binding context? I wouldn't want to recreate that state everytime you Pop from the stack.
  3. This initial API was based on Xamarin's INavigation interface. I think WPF doesn't care, and @glennawatson has pointed out that WPF would prefer these concerns separated out because it doesn't make sense.
  4. Currently there is support for Destroying ViewModels in Sextant. I have been asked to open support for Views as well. I think providing an extension point for clean up is extremely important.