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

[Community Toolkit Feature Promotion] Expand WeakEventManager to Support Delegate #2703

Closed
brminnick opened this issue Sep 26, 2021 · 2 comments
Labels
fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3! fixed-in-6.0.200-preview.12 Look for this fix in 6.0.200-preview.12! proposal/open t/enhancement ☀️ New feature or request

Comments

@brminnick
Copy link
Contributor

brminnick commented Sep 26, 2021

WeakEventManager

Microsoft.Maui.Controls.WeakEventManger only supports event EventHandler.

It does not currently work with the other event types in C#, like Delegate.

This means that Microsoft.Maui.Controls.WeakEventManger does not currently work for the most common Delegate event, INotifyPropertyChanged.PropertyChanged.

Because EventHandler is a Delegate, we can expand WeakEventManager, adding support for Delegate without any breaking changes.

Working Example

For EventHandler events, Microsoft.Maui.Controls.WeakEventManger works great!

readonly WeakEventManager weakEventManager = new WeakEventManager();

public event EventHandler Completed
{
  add => weakEventManager.AddEventHandler(value);
  remove => weakEventManager.RemoveEventHandler(value);
}

void OnCompleted() => weakEventManager.HandleEvent(this, EventArgs.Empty, nameof(Completed));

Non-Working Example

For delegate events, Microsoft.Maui.Controls.WeakEventManger is not compatible.

readonly WeakEventManager weakEventManager = new WeakEventManager();

event PropertyChangedEventHandler INotifyPropertyChanged.PropertyChanged
{
    add => weakEventManager.AddEventHandler(value); // Compiler Error
    remove => weakEventManager.RemoveEventHandler(value);  // Compiler Error
}

void OnPropertyChanged([CallerMemberName] in string propertyName = "") => weakEventManager.HandleEvent(this, new PropertyChangedEventArgs(propertyName), nameof(INotifyPropertyChanged.PropertyChanged));

Xamarin Community Toolkit, DelegateWeakEventManager

The Xamarin.CommunityToolkit has expanded on WeakEventManager, adding support for delegate events:
https://docs.microsoft.com/xamarin/community-toolkit/helpers/delegateweakeventmanager?WT.mc_id=mobile-43581-bramin

This feature was named DelegateWeakEventManager to avoid a name-collision with WeakEventManager despite providing (almost) the exact same feature set; the only difference is the additional support for delegate events.

The Xamarin Community Toolkit has included this expanded WeakEventManager since v1.0.0-pre4 released on October 21, 2020.

We currently have an open proposal to add this functionality to Maui Community Toolkit, but we'd prefer to promote this feature into the official .NET MAUI library.

API Changes

WeakEventManager

Methods

Current

The current implementation only supports EventHandler

public void AddEventHandler(EventHandler handler, [CallerMemberName] string eventName = null)
{
	if (IsNullOrEmpty(eventName))
		throw new ArgumentNullException(nameof(eventName));

	if (handler == null)
		throw new ArgumentNullException(nameof(handler));

	AddEventHandler(eventName, handler.Target, handler.GetMethodInfo());
}

public void RemoveEventHandler(EventHandler handler, [CallerMemberName] string eventName = null)
{
	if (IsNullOrEmpty(eventName))
		throw new ArgumentNullException(nameof(eventName));

	if (handler == null)
		throw new ArgumentNullException(nameof(handler));

	RemoveEventHandler(eventName, handler.Target, handler.GetMethodInfo());
}

Updated

The updated implementation adds support for Delegate

public void AddEventHandler(Delegate? handler, [CallerMemberName] string eventName = null)
{
	ArgumentNullException.ThrowIfNull(eventName);
	ArgumentNullException.ThrowIfNull(handler)

	var methodInfo = handler.GetMethodInfo() ?? throw new NullReferenceException("Could not locate MethodInfo");

	AddEventHandler(eventName, handler.Target, methodInfo, eventHandlers);
}

public void RemoveEventHandler(Delegate? handler, [CallerMemberName] string eventName = "")
{
	ArgumentNullException.ThrowIfNull(eventName);
	ArgumentNullException.ThrowIfNull(handler)

	var methodInfo = handler.GetMethodInfo() ?? throw new NullReferenceException("Could not locate MethodInfo");

	RemoveEventHandler(eventName, handler.Target, methodInfo, eventHandlers);
}

Scenarios

C# Example

The most common usage of a delegate event is implementing INotifyPropertyChanged:

readonly WeakEventManager weakEventManager = new WeakEventManager();

event PropertyChangedEventHandler? INotifyPropertyChanged.PropertyChanged
{
    add => weakEventManager.AddEventHandler(value);
    remove => weakEventManager.RemoveEventHandler(value);
}

void OnPropertyChanged([CallerMemberName] in string propertyName = "") => weakEventManager.HandleEvent(this, new PropertyChangedEventArgs(propertyName), nameof(INotifyPropertyChanged.PropertyChanged));

XAML Example

N/A

CSS Example

N/A

Backward Compatibility

Because EventHandler is a Delegate, this update is completely backwards compatible and does not introduce any breaking changes.

Minimum API levels? None
Breaking changes? None
Unsupported platforms? None

Difficulty : Low

I am happy to open a PR to implement this feature

@brminnick brminnick changed the title [Community Toolkit Feature Promotion] Expand WeakEventManager to Support delegate [Community Toolkit Feature Promotion] Expand WeakEventManager to Support Delegate Sep 26, 2021
@vhugogarcia
Copy link
Contributor

Awesome proposal to be integrated natively in .NET MAUI. If you allow me, I would like to suggest to integrate also from community toolkit the object model.

https://docs.microsoft.com/en-us/xamarin/community-toolkit/objectmodel/asynccommand

I believe that this feature and detailed level of integration should be natively since all controls support command and command parameter, having this natively makes sense. @jamesmontemagno did an amazing work initially with this feature and merged it on XCT. I think it is time to be set on .NET MAUI.

This is just my grain of salt.

Thanks 😊

@jsuarezruiz jsuarezruiz added the t/enhancement ☀️ New feature or request label Oct 22, 2021
@jsuarezruiz jsuarezruiz added this to Under consideration in Enhancements Oct 22, 2021
@jsuarezruiz jsuarezruiz added this to To Review in API Review Nov 8, 2021
@rmarinho
Copy link
Member

fixed #3253

@jsuarezruiz jsuarezruiz moved this from To Review to Reviewed in API Review Nov 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 18, 2022
@samhouts samhouts added fixed-in-6.0.200-preview.12 Look for this fix in 6.0.200-preview.12! fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3! labels Jul 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3! fixed-in-6.0.200-preview.12 Look for this fix in 6.0.200-preview.12! proposal/open t/enhancement ☀️ New feature or request
Projects
API Review
Reviewed
Enhancements
Under consideration
Development

No branches or pull requests

6 participants