IPopupService: Show popup overload taking view model does not use it. No support for Popup instances?

[CraigRice]

I was trying to open a popup with this overload, passing an app-created (non resolved) view model instance.
Task<object?> ShowPopupAsync<TViewModel>(TViewModel viewModel, CancellationToken token)

This is useful because the viewmodel can be immutable, and therefore its ctor can be in charge of enforcing its state, its properties can have OneTime bindings, and no need for property changed.

All that seems to happen with this overload is an arg null check. What is the point of it then?
So then I was forced make a fully mutable view model and use the Action<TViewModel> onPresenting to set the model up.

When I saw the model instance overload did not use the instance, I thought about just newing up the Popup and passing my view model into its ctor, but IPopupService doesn't support showing a raw Popup instance and expects to resolve the popup and viewmodel pair.
It seems all that is needed to support that is make the following public:
private static Task<object?> ShowPopupAsync(Popup popup, CancellationToken token)

[bijington]

I originally added that method to enable the support you desire but decisions went against that approach and I understand the reasons why.

I do have a desire to implement the ability to just show a Popup without an associated view model. Would this solve your issue?

[CraigRice]

Yes thanks, if I can supply the Popup instance myself then I have full control of its creation and its viewmodel’s creation. Your service can still be responsible for showing it then.

[bijington]

Sorry the devil is in the details and I failed to spot the detail in your method. I didn't see the Popup instance being passed into the Popup service. I wasn't planning on that as we are very close to just using ShowPopup on a page. I guess you still want to show from a view model?

What type of initialisation are you wanting to do?

[CraigRice]

I don't understand why you want prevent an instance of Popup being created external to the service. The current implementation I went with works ok and won't be detrimental to the project, its just not as good as it could have been. I will add some more details for what I was thinking.

I have a IDialogService that is injected into page viewmodels. This is for showing alert and confirmation dialogs. So for the alert dialog all it needs is three strings: a title, mesage and button text.

Currently working implementation:

public Task ShowAlertDialogAsync(string title, string message)
{
    return popupService.ShowPopupAsync<DialogAlertModel>(onPresenting: model =>
    {
        model.Title = title;
        model.Message = message;
        model.ButtonText= _okButtonCaption;
    });
}

public sealed partial class DialogAlertModel : ObservableObject
{
    [ObservableProperty] private string _title;
    [ObservableProperty] private string _message;
    [ObservableProperty] private string _buttonText;
}

I wanted to do something like the following.

• The model is created once and supports OneTime binding, rather than being newed up with all strings null then set progressively over time during 'onPresenting'.
• The model guarantees all its properties are set, and ctor could also throw arg null exceptions for example.
• The model doesn't need to raise property change and have OneWay bindings to notify the properties are being set.
• I don't need to register the types. There's no issue doing that but no benefit when other injected services aren't required.
• I would definitely just use the other IPopupService overloads instead if injections were required into the view model.
• Just newing it up is trivial.

public sealed class DialogAlertModel(string title, string message, string buttonText)
{
    public string Title { get; } = title;
    public string Message { get; } = message;
    public string ButtonText { get; } = buttonText;
}

Your overload Task<object?> ShowPopupAsync<TViewModel>(TViewModel viewModel, CancellationToken token = default) makes it look like you can do this, but it throws System.InvalidOperationException: 'Unable to resolve service for type 'System.String' while attempting to activate {type} when trying to resolve the strings for the ctor :

public Task ShowAlertDialogAsync(string title, string message)
{
    return popupService.ShowPopupAsync(
        new DialogAlertModel(title, message, _okButtonCaption)
    );
}

Maybe the caller could do this instead

public Task ShowAlertDialogAsync(string title, string message)
{
    return popupService.ShowPopupAsync(new DialogAlertPopup(
        new DialogAlertModel(title, message, _okButtonCaption)
    ));
}

[bijington]

The plan isn't to control instance creation but more to encourage developers to make use of dependency injection.

I agree your scenario looks much better when it is immutable. I'll have a think on whether we can make this better and come back with a proposal

[CliffAgius]

Is this discussion stale? can it be closed?

[bijington]

I guess it's technically become stale but I would still like to address it

[Xynratron]

I have the exact same usecase and wondered, that my allready created ViewModel is not used for the Popup.

In my opinion, the method

public Task<object?> ShowPopupAsync<TViewModel>(CancellationToken token = default (CancellationToken))`

should create a new instance of TViewModel via dependency injection, but the following:

public Task<object?> ShowPopupAsync<TViewModel>(TViewModel viewModel, CancellationToken token = default (CancellationToken))` 

must use my instance of TViewModel. So I was a bit surprised, when I looked at the current implementation.

If we don't want a given instance (and want to create one via DI) , we can create an instance via ActivatorUtilities, which uses DependencyInjection via IServiceProvider and fill the missing Constructor Arguments with the given params:

public static object CreateInstance (IServiceProvider provider, Type instanceType, params object[] parameters);

This would result in something like this:

public Task<object?> ShowPopupAsync<TViewModel>(params object[] constructorParameters, CancellationToken token = default (CancellationToken)) where TViewModel : INotifyPropertyChanged
{
  Popup popup = this.GetPopup(typeof (TViewModel), constructorParameters);
  PopupService.ValidateBindingContext<TViewModel>(popup, out TViewModel _);
  return PopupService.ShowPopupAsync(popup, token);
}

private Popup GetPopup(Type viewModelType, params object[] constructorParameters)
{
  if ( ActivatorUtilities.GetService((this.serviceProvider, PopupService.viewModelToViewMappings[viewModelType], constructorParameters) is Popup service)
    return service;
  [..]
}