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

[Proposal] PopupService #981

Closed
8 tasks done
bijington opened this issue Feb 11, 2023 · 14 comments · Fixed by #1165
Closed
8 tasks done

[Proposal] PopupService #981

bijington opened this issue Feb 11, 2023 · 14 comments · Fixed by #1165
Assignees
Labels
approved This Proposal has been approved and is ready to be added to the Toolkit champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature documentation approved proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail

Comments

@bijington
Copy link
Contributor

bijington commented Feb 11, 2023

Feature name

PopupService

Link to discussion

#933

Progress tracker

  • Android Implementation
  • iOS Implementation
  • MacCatalyst Implementation
  • Windows Implementation
  • Tizen Implementation
  • Unit Tests
  • Samples
  • Documentation

Summary

The following proposal provides the ability to register and show popups with associated view models. It will also allow this to be done from a view model.

Motivation

To make it easier to show popups from a view model.

Detailed Design

PopupService

using System.ComponentModel;
using CommunityToolkit.Maui.Views;

namespace Popups;

public class PopupService
{
    private readonly IServiceProvider serviceProvider;

    private readonly static IDictionary<Type, Type> viewModelToViewMappings = new Dictionary<Type, Type>();

    public PopupService(IServiceProvider serviceProvider)
    {
        this.serviceProvider = serviceProvider;
    }

    public static void AddTransientPopup<TPopupView, TPopupViewModel>(IServiceCollection services)
        where TPopupView : Popup
        where TPopupViewModel : INotifyPropertyChanged
    {
        viewModelToViewMappings.Add(typeof(TPopupViewModel), typeof(TPopupView));

        services.AddTransient(typeof(TPopupView));
        services.AddTransient(typeof(TPopupViewModel));
    }

    public Task<object> ShowPopupAsync<TViewModel>()
    {
        var viewModelType = typeof(TViewModel);

        var viewModel = (TViewModel)this.serviceProvider.GetService<TViewModel>();

        if (viewModel is null)
        {
            throw new InvalidOperationException($"Unable to resolve type {viewModelType} please make sure that you have called RegisterPopup");
        }

        return ShowPopupAsync<TViewModel>(viewModel);
    }

    public Task<object> ShowPopupAsync<TViewModel>(TViewModel viewModel)
    {
        var viewModelType = typeof(TViewModel);

        var view = (Popup)this.serviceProvider.GetRequiredService(viewModelToViewMappings[viewModelType]);

        view.BindingContext = viewModel;

        // TODO: Do we need to provide an override for determine the main page? Or we at least need to verify there is a main page.
        return Application.Current.MainPage.ShowPopupAsync(view);
    }
}

ServiceCollectionExtension

public static class ServiceCollectionExtensions
{
    public static IServiceCollection AddTransientPopup<TPopupView, TPopupViewModel>(this IServiceCollection services)
        where TPopupView : Popup
        where TPopupViewModel : INotifyPropertyChanged
    {
        PopupService. AddTransientPopup<TPopupView, TPopupViewModel>(services);

        return services;
    }
}

Usage Syntax

// Registration in MauiProgram.cs
builder.Services.AddTransientPopup<ConfirmationPopupView, ConfirmationPopupViewModel>();

// Showing of popup from a VM
popupService.ShowPopupAsync<ConfirmationPopupViewModel>();

Drawbacks

It is heavily MVVM focused

Alternatives

No response

Unresolved Questions

No response

@bijington bijington added new proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail labels Feb 11, 2023
@VladislavAntonyuk
Copy link
Collaborator

approve

@acaliaro
Copy link

is not possible to use the current "AddTransient" instead of a new method?

@bijington
Copy link
Contributor Author

is not possible to use the current "AddTransient" instead of a new method?

We can utilise the AddTransient extension method in this method but it needs to do a little more by storing the mapping between view and view model. This then allows us from another view model to ask to show the popup for the view model and therefore removing the need for any view information in the view model. I might consider going a step further and allowing a string to be registered a bit like with Shell routes so we don't have to know the view model either.

@bijington
Copy link
Contributor Author

Investigate the concept of AddTransientPopup to register just a PopupView and not also the view model

@bijington
Copy link
Contributor Author

Also possibly the concept of registering the view model lifetime differently to the view

@gabsamples6
Copy link

@bijington very interested in your proposal, currently we cannot use this popup because we are using MVVM.. do you have a working sample that we could look at and use till this becomes official?

many thanks

@bijington
Copy link
Contributor Author

@bijington very interested in your proposal, currently we cannot use this popup because we are using MVVM.. do you have a working sample that we could look at and use till this becomes official?

many thanks

Apologies for the delayed response. I do have a draft PR open here that you could check out: #1165

@ghost ghost added champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature and removed new labels Aug 3, 2023
@ghost ghost added approved This Proposal has been approved and is ready to be added to the Toolkit help wanted This proposal has been approved and is ready to be implemented labels Aug 3, 2023
@bijington bijington mentioned this issue Aug 11, 2023
6 tasks
@bijington bijington removed the help wanted This proposal has been approved and is ready to be implemented label Aug 11, 2023
@hansmbakker
Copy link

Will this proposal also cover Page.DisplayAlert(...)?

@bijington
Copy link
Contributor Author

Will this proposal also cover Page.DisplayAlert(...)?

No that is not my intention. This is purely focussed around the toolkits Popup implementation

@acaliaro
Copy link

Hi @bijington , do you have an ETA for this?

@bijington
Copy link
Contributor Author

I forgot about this. As far as I am aware the PR is good to go so I should poke the team to review it.

@ghost ghost reopened this Nov 1, 2023
@ghost
Copy link

ghost commented Nov 1, 2023

Reopening Proposal.

Only Proposals moved to the Closed Project Column and Completed Project Column can be closed.

@ghost ghost closed this as completed Nov 3, 2023
@RuddyOne
Copy link

RuddyOne commented Feb 20, 2024

Investigate the concept of AddTransientPopup to register just a PopupView and not also the view model

@bijington Was this overlooked? I'm a little confused, at the moment is it not possible to register a Popup without a View model.

In my case I have generic popups in my application, they do not need a VM due to being super simple, I cant see a way to register them without a VM?

@bijington
Copy link
Contributor Author

It was necessarily overlooked, we decided to get just the view model support in for v1. I'm currently working on adding in Close support and the display of a popup without having to create a Popup. Then I'm planning on addressing this, I've had some thoughts but if you wanted to get involved I'd happily back the idea.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved This Proposal has been approved and is ready to be added to the Toolkit champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature documentation approved proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail
Development

Successfully merging a pull request may close this issue.

6 participants