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

Routing API revamp RFC #64

Open
mteper opened this issue May 5, 2016 · 3 comments
Open

Routing API revamp RFC #64

mteper opened this issue May 5, 2016 · 3 comments
Assignees

Comments

@mteper
Copy link

mteper commented May 5, 2016

I just started experimenting with routing (UWP app), so while I have fresh eyes, some thoughts:

Rename RoutingState to Router

"Routing state" implies a container of state data rather than a class responsible for navigation actions. "Router" seems a better name for a class that actually handles routing. This is further reinforced by the fact that even on the IScreen interface, the RoutingState property name is "Router". IMO, Router is both more apt and concise.

Formalize navigation stack into a class

Today, we have a handful of commands hanging off RoutingState along with a ObservableCollection called "NavigationStack" (note that here it's not even a stack in the data structures sense of the term). It would be better to introduce a new class, NavigationStack with the following signature:

public class NavigationStack
{
  ReactiveCommand<Unit> Advance { get; }
  ReactiveCommand<Unit> Backtrack { get; }
  IRoutableViewModel Current { get; }
}

This would leave no ambiguity about what the commands do. If there is value in it, the class can implement ObservableCollection to enable subscribing to notifications. Router would then have a read-only property named NavigationStack.

Replace the Navigate command with method + property

Is there value in Navigate being a command? Writing router.Navigate.Execute(vm) feels clunky. Would be better to have Navigate be a method, and have a companion Navigating observable property on Router. Another benefit is that the method can have an (optional) bool reset attribute, thus letting us drop the NavigateAndReset command on Router.

@kentcb
Copy link

kentcb commented May 20, 2016

👍 A lot of this is in line with my thoughts on re-vamping routing, perhaps for V8. However, I'd prefer stack semantics for NavigationStack members, such as Push and Pop.

But that's pretty much irrelevant minutia right now. I think the important thing is to take routing in a direction that gives us:

  • far greater consistency across platforms (by sharing far more code and pushing more responsibility into that shared code)
  • far greater flexibility (not dictating so much about how routing occurs at the UI level)
  • a story for platforms that aren't VM-first friendly (e.g. Android)

These are difficult problems to solve, and I think we should look for inspiration elsewhere (Prism, MVVMCross etc).

@stale stale bot closed this as completed Sep 1, 2017
@ghuntley ghuntley reopened this Sep 1, 2017
@qrzychu
Copy link

qrzychu commented Apr 10, 2018

When revamping navigation, I would also like something like Router.PushDialog.

This is a common scenario (I think), when I want edit an item in another ViewModel and then fall back to previous VM with new value.

Rigth now I use some hacks like this:

 EditItem = ReactiveCommand.CreateFromObservable<Item, Item>(item =>
            {


                var editVM = new EditItemViewModel(HostScreen, resolver, item);
                HostScreen.Router.Navigate.Execute(editVM).Subscribe();

                
               
                return HostScreen.WhenAnyValue(x => x.CurrentViewModel)
                        .Where(x => x == this)
                        .Take(1)
                        .TakeUntil(HostScreen.Router.NavigationStack.ShouldReset)
                        .Select(x => item);

                // or use EditDone command of EditVM

                return editVM.EditDone.Select(x => item)
                       .Take(1)
                       .TakeUntil(HostScreen.Router.NavigationStack.ShouldReset); 
                 // and navigate back in EditItem.Subscribe
});

I think that with NavigationStack it would be great to write something like this:

return HostScreen.Router.PushDialog.Execute(editVM).Select(x => item);

PushDialog would return editVM when Router.Pop would bring us back to CurrentViewModel and handle all edge cases of navigating to terminate returned observable.

It's quite easy to make an extension method for that, but if you want to revamp the whole thing either way, it could be just built-in functionality.

If there is already a easy way to make it error free, please tell me :)

@glennawatson
Copy link
Contributor

Assigning to @RLittlesII I know he's been wanting to get his feet dirty on this type of thing.

@glennawatson glennawatson transferred this issue from reactiveui/ReactiveUI May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants