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

Reconsider if ViewModelBase should implement NavigationService #1721

Open
JerryNixon opened this issue Dec 17, 2019 · 1 comment
Open

Reconsider if ViewModelBase should implement NavigationService #1721

JerryNixon opened this issue Dec 17, 2019 · 1 comment
Assignees

Comments

@JerryNixon
Copy link
Member

JerryNixon commented Dec 17, 2019

It's already possible to retrieve the NavigationService in a Template 10 view-model using the GetNavigationService() extension method off the NavigationParameters passed into the OnNavigatedTo and OnNavigatedToAsync methods. You can also use GetSynchronizationContext() to help with threading. These are not discoverable, I know, but they are easy.

Here's the syntax:

public class MyViewModel : ViewModelBase
{
    private INavigationService _nav;

    public override void OnNavigatedTo(INavigationParameters parameters)
    {
        _nav = parameters.GetNavigationService();
    }
}

The issue here, however, is the change in heart I have had with NavigationService. I believe that an app should create its own NavigationService, lately I have been calling mine NavigationManager in order to disambiguate the name. The reason to remove it from ViewModelBase as a public property is that it encourages a generic use case instead of an app-specific one. It also shows up when using binding and that's really confusing.

Here's what I am recommending:

public class NavigationManager
{
    private readonly NavigationService _navigationService;

    public NavigationManager(NavigationService navigationService)
    {
        _navigationService = navigationService;
    }
}

public class MyViewModel : ViewModelBase
{
    private readonly NavigationManager _nav;

    public MyViewModel(NavigationManager navigationManager)
    {
        _nav = navigationManager;
    }
}

This is not a recommendation to remove the extension methods or the ability to access the NavigationService. Just removing the public property on the view-model in order to encourage better patterns. If you don't want better patterns, then the extension methods are still there. You can also sub-class ViewModelBase in order to restore the property, too.

@JerryNixon JerryNixon self-assigned this Dec 17, 2019
@panoukos41
Copy link

I think this is the right way to proceed, that way people can always implement their own services if they wish to without worrying for services they don't need.

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

No branches or pull requests

2 participants