[Codebase] Use IDialogService for dialogs

[d2dyno1]

Is your feature request related to a problem?
We currently don't have any solution for showing ContentDialogs in ViewModels. Creating an instance of a ContentDialog doesn't comply with MVVM since the ViewModel should now nothing about the UI and it's components.

Describe the solution
To solve that we could use a IDialogService to show dialogs instead.

Describe alternatives you've considered
no.

Scope

Capability	Priority
Use IDialogService to replace ContentDialog creation in ViewModels	Must

Additional comment
I already have the code for that in Clipboard Canvas so it could be reused here, if assigned. For #2341

[d2dyno1]

@yaichenbaum Can you assign me?

[yaira2]

This issue hasn't been approved yet but I can assign you if/when it is approved.

[lukeblevins]

@d2dyno1 Can you provide a quick run-down of what IDialogService would consist of and the pattern it will use to enable showing dialogs from ViewModels?

[d2dyno1]

@duke7553 Take a look at the implementation. It pretty much explains itself what it does: IDialogService, DefaultDialogService

Example usage:

IDialogService dialogService = Ioc.Default.GetService<IDialogService>(); // Get IDialogService from IoC (dependency injection)

DialogVM viewModel = new(); // Initialize dialog View Model - this allows access to VM's properties without holding a reference to the UI element (ContentDialog)
IDialog<DialogVM> dialog = dialogService.GetDialog<DialogVM>(viewModel); // Get IDialog<DialogVM>

DialogResult option = await dialog.ShowAsync(); // Show dialog
// You can also call IDialogService.ShowDialog() directly, see above reference links to CC's code

[d2dyno1]

@duke7553 @yaichenbaum Any thoughts about the above logic and how we can use it in Files?

[gave92]

It looks pretty similar to what we're already doing, eg from FilesystemHelpers.cs:

var viewModel = new FilesystemItemsOperationDataModel(...);
FilesystemOperationDialog dialog = await FilesystemOperationDialogViewModel.GetDialog(viewModel);
ContentDialogResult result = await dialog.ShowAsync();

I'm not sure I'm understanding what benefits this change would bring.

[d2dyno1]

I'll clarify it a bit better this time 😁

It looks pretty similar to what we're already doing, eg from FilesystemHelpers.cs:

FilesystemHelpers.cs is the only case, and it's not even mvvm compliant since we don't have any way to show the dialog while following MVVM pattern.

The most important rule defined by MVVM says that View Models and Models should know nothing about the UI.
View Models should only expose properties that View binds to.
What we're currently doing, is we're using UI elements (like dialogs) in View Models:

public class SomeViewModel : ObservableObject
{
    public async Task Foo()
    {
        // Not compliant!
        ContentDialog dialog = new AddItemDialog();
        await dialog.ShowAsync();
    }
}

As you can see, we're using a UI control inside a View Model. If the UI was swapped or updated we'd have to also change the View Models which indeed defies MVVM pattern and makes View Models redundant (it's like having two xaml.cs codebehind classes at that point). To solve the above problem, we can use IDialogService service. Such services are completely abstract and leave UI to implement them. This way our Model can provide abstraction class/interface (i.e. IDialogService) and let the UI part implement it. By that, our View Model knows nothing about the UI and it's [dialog] implementation. (Services are not limited to being able to implement UI interfaces)

public class SomeViewModel : ObservableObject
{
    private IDialogService DialogService { get; } = Ioc.Default.GetService<IDialogService>();    

    public async Task Foo()
    {
        // Compliant!
        var dialogViewModel = new AddItemDialogViewModel();
        IDialog<AddItemDialogViewModel> dialog = DialogService.GetDialog(dialogViewModel);
        CustomDialogResult result = await dialog.ShowAsync();
    }
}

With the above example our View Model retrieves the dialog via dependency injection through IoC container. This way, it doesn't know anything about the UI. Additional code on constructing IoC container
Additional material on benefits of DI.

@gave92

[gave92]

I guess I'm not seeing the benefit here since we only call ContentDialog methods and props. We could change the UI with no problem as long as it inherits from ContentDialog.

Is there an example in Files code where the benefits would be more visible? (I admit I don't give a fig about "being compliant with mvvm" where it brings no actual code simplification/maintainability improvement xD)

Anyway this change looks like a good one in the general case, so.. approved :)

[d2dyno1]

@gave92

I guess I'm not seeing the benefit here since we only call ContentDialog methods and props. We could change the UI with no problem as long as it inherits from ContentDialog.

That's right but we couldn't call them from a View Model (e.g. ShowAsync()), nor create instance of ContentDialog in a VM.

Is there an example in Files code where the benefits would be more visible?

The main reason it's useful would be the consistency in creating ContentDialogs - from one class. Also, there's the benefit of injecting a DialogViewModel directly to ContentDialog.DataContext and obviously MVVM guidelines.

(I admit I don't give a fig about "being compliant with mvvm" where it brings no actual code simplification/maintainability improvement xD)

Huh? That's what MVVM is meant for -- maintainability and modularity 😁
https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93viewmodel
Also you can't just ignore the rules of mvvm if we're going full on with it. It's either yes or no ;)