Skip to content

Proposal - Replace internal use of MessagingCenter #3880

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.

Sign up for GitHub

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

Closed
hartez wants to merge 9 commits into from
Closed

Proposal - Replace internal use of MessagingCenter #3880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
de967b9
testing moving the Android page stuff
hartez Dec 20, 2021
39867d8
Update the Page stuff for all platforms
hartez Dec 29, 2021
6cb67bb
Replace the rest of the MC stuff
hartez Dec 29, 2021
4a01020
Convert message classes to records
hartez Dec 30, 2021
c4e8f49
Don't make empty classes into records
hartez Dec 30, 2021
63a4020
Fix some closures and method group usage
hartez Dec 30, 2021
b425af7
Mark lambdas static
hartez Dec 30, 2021
ca9f0d7
Clean up more closures and method groups
hartez Dec 30, 2021
b5125da
Update NuGet.config
Redth Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion NuGet.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<packageSources>
<clear />
Expand Down
5 changes: 2 additions & 3 deletions src/Compatibility/Core/src/Android/AppCompat/Platform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Android.OS;
using Android.Views;
using Android.Views.Animations;
using CommunityToolkit.Mvvm.Messaging;
using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Controls.Platform;
using Microsoft.Maui.Graphics;
Expand All @@ -27,8 +28,6 @@ public class Platform : BindableObject, IPlatformLayout, INavigation
internal static string PackageName { get; private set; }
internal static string GetPackageName() => PackageName;

internal const string CloseContextActionsSignalName = "Xamarin.CloseContextActions";

internal static readonly BindableProperty RendererProperty = BindableProperty.CreateAttached("Renderer", typeof(IVisualElementRenderer), typeof(Platform), default(IVisualElementRenderer),
propertyChanged: (bindable, oldvalue, newvalue) =>
{
Expand Down Expand Up @@ -81,7 +80,7 @@ internal bool NavAnimationInProgress
return;
_navAnimationInProgress = value;
if (value)
MessagingCenter.Send(this, CloseContextActionsSignalName);
WeakReferenceMessenger.Default.Send(new CloseContextActionsMessage());
Copy link

@Sergio0694 Sergio0694 Dec 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this could be simplied through the extension:

Suggested change
WeakReferenceMessenger.Default.Send(new CloseContextActionsMessage());
WeakReferenceMessenger.Default.Send<CloseContextActionsMessage>();

}
}

Expand Down
15 changes: 7 additions & 8 deletions src/Compatibility/Core/src/Android/PopupManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Android.Text;
using Android.Views;
using Android.Widget;
using CommunityToolkit.Mvvm.Messaging;
using Microsoft.Maui.Controls.Internals;
using AppCompatActivity = AndroidX.AppCompat.App.AppCompatActivity;
using AppCompatAlertDialog = AndroidX.AppCompat.App.AlertDialog;
Expand Down Expand Up @@ -50,20 +51,18 @@ internal sealed class PopupRequestHelper : IDisposable
internal PopupRequestHelper(Activity context)
{
Activity = context;
MessagingCenter.Subscribe<Page, bool>(Activity, Page.BusySetSignalName, OnPageBusy);
MessagingCenter.Subscribe<Page, AlertArguments>(Activity, Page.AlertSignalName, OnAlertRequested);
MessagingCenter.Subscribe<Page, PromptArguments>(Activity, Page.PromptSignalName, OnPromptRequested);
MessagingCenter.Subscribe<Page, ActionSheetArguments>(Activity, Page.ActionSheetSignalName, OnActionSheetRequested);

WeakReferenceMessenger.Default.Register<PopupRequestHelper, PageBusyMessage>(this, static (r, m) => r.OnPageBusy(m.Page, m.IsBusy));
WeakReferenceMessenger.Default.Register<PopupRequestHelper, PageAlertMessage>(this, static (r, m) => r.OnAlertRequested(m.Page, m.Arguments));
WeakReferenceMessenger.Default.Register<PopupRequestHelper, PromptMessage>(this, static (r, m) => r.OnPromptRequested(m.Page, m.Arguments));
WeakReferenceMessenger.Default.Register<PopupRequestHelper, ActionSheetMessage>(this, static (r, m) => r.OnActionSheetRequested(m.Page, m.Arguments));
}

public Activity Activity { get; }

public void Dispose()
{
MessagingCenter.Unsubscribe<Page, bool>(Activity, Page.BusySetSignalName);
MessagingCenter.Unsubscribe<Page, AlertArguments>(Activity, Page.AlertSignalName);
MessagingCenter.Unsubscribe<Page, PromptArguments>(Activity, Page.PromptSignalName);
MessagingCenter.Unsubscribe<Page, ActionSheetArguments>(Activity, Page.ActionSheetSignalName);
WeakReferenceMessenger.Default.UnregisterAll(this);
}

public void ResetBusyCount()
Expand Down
5 changes: 3 additions & 2 deletions src/Compatibility/Core/src/Android/Renderers/ListViewAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Android.Util;
using Android.Views;
using Android.Widget;
using CommunityToolkit.Mvvm.Messaging;
using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Graphics;
using AListView = Android.Widget.ListView;
Expand Down Expand Up @@ -64,7 +65,7 @@ public ListViewAdapter(Context context, AListView realListView, ListView listVie
realListView.OnItemClickListener = this;
realListView.OnItemLongClickListener = this;

MessagingCenter.Subscribe<ListViewAdapter>(this, Platform.CloseContextActionsSignalName, lva => CloseContextActions());
WeakReferenceMessenger.Default.Register<ListViewAdapter, CloseContextActionsMessage>(this, static (r, m) => r.CloseContextActions());

InvalidateCount();
}
Expand Down Expand Up @@ -436,7 +437,7 @@ protected override void Dispose(bool disposing)
{
CloseContextActions();

MessagingCenter.Unsubscribe<ListViewAdapter>(this, Platform.CloseContextActionsSignalName);
WeakReferenceMessenger.Default.Unregister<CloseContextActionsMessage>(this);

_realListView.OnItemClickListener = null;
_realListView.OnItemLongClickListener = null;
Expand Down
26 changes: 18 additions & 8 deletions src/Compatibility/Core/src/Windows/Platform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.Maui.Controls.Platform;
using WVisibility = Microsoft.UI.Xaml.Visibility;
using Microsoft.Extensions.Logging;
using CommunityToolkit.Mvvm.Messaging;

namespace Microsoft.Maui.Controls.Compatibility.Platform.UWP
{
Expand Down Expand Up @@ -136,10 +137,10 @@ internal Platform(Microsoft.UI.Xaml.Window page)

_container.SizeChanged += OnRendererSizeChanged;

MessagingCenter.Subscribe(this, Page.BusySetSignalName, (Page sender, bool enabled) =>
WeakReferenceMessenger.Default.Register<UI.Xaml.Window, PageBusyMessage>(page, (_, message) =>
{
Microsoft.UI.Xaml.Controls.ProgressBar indicator = GetBusyIndicator();
indicator.Visibility = enabled ? WVisibility.Visible : WVisibility.Collapsed;
indicator.Visibility = message.IsBusy ? WVisibility.Visible : WVisibility.Collapsed;
});

_toolbarTracker.CollectionChanged += OnToolbarItemsChanged;
Expand Down Expand Up @@ -601,13 +602,16 @@ internal IToolbarProvider GetToolbarProvider()

internal static void SubscribeAlertsAndActionSheets()
{
MessagingCenter.Subscribe<Page, AlertArguments>(Forms.MainWindow, Page.AlertSignalName, OnPageAlert);
MessagingCenter.Subscribe<Page, ActionSheetArguments>(Forms.MainWindow, Page.ActionSheetSignalName, OnPageActionSheet);
MessagingCenter.Subscribe<Page, PromptArguments>(Forms.MainWindow, Page.PromptSignalName, OnPagePrompt);
WeakReferenceMessenger.Default.Register<UI.Xaml.Window, PageAlertMessage>(Forms.MainWindow, (r,m) => OnPageAlert(m));
WeakReferenceMessenger.Default.Register<UI.Xaml.Window, PromptMessage>(Forms.MainWindow, (r, m) => OnPagePrompt(m));
WeakReferenceMessenger.Default.Register<UI.Xaml.Window, ActionSheetMessage>(Forms.MainWindow, (r, m) => OnPageActionSheet(m));
}

static void OnPageActionSheet(Page sender, ActionSheetArguments options)
static void OnPageActionSheet(ActionSheetMessage message)
{
var sender = message.Page;
var options = message.Arguments;

bool userDidSelect = false;

if (options.FlowDirection == FlowDirection.MatchParent)
Expand Down Expand Up @@ -654,8 +658,11 @@ static void OnPageActionSheet(Page sender, ActionSheetArguments options)
}
}

static async void OnPagePrompt(Page sender, PromptArguments options)
static async void OnPagePrompt(PromptMessage message)
{
var sender = message.Page;
var options = message.Arguments;

var promptDialog = new PromptDialog
{
Title = options.Title ?? string.Empty,
Expand Down Expand Up @@ -693,8 +700,11 @@ static async Task<string> ShowPrompt(PromptDialog prompt)
return null;
}

static async void OnPageAlert(Page sender, AlertArguments options)
static async void OnPageAlert(PageAlertMessage message)
{
var sender = message.Page;
var options = message.Arguments;

string content = options.Message ?? string.Empty;
string title = options.Title ?? string.Empty;

Expand Down
26 changes: 18 additions & 8 deletions src/Compatibility/Core/src/iOS/Platform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.Messaging;
using CoreGraphics;
using Foundation;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -655,30 +656,43 @@ internal static string ResolveMsAppDataUri(Uri uri)
internal void SubscribeToAlertsAndActionSheets()
{
var busyCount = 0;
MessagingCenter.Subscribe(this, Page.BusySetSignalName, (Page sender, bool enabled) =>

WeakReferenceMessenger.Default.Register<Platform, PageBusyMessage>(this, (platform, message) =>
Copy link

@Sergio0694 Sergio0694 Dec 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is introducing a closure to capture this in the handler, as it's calling that PageIsChildOfPlatform. Why not invoke it from the recipient directly instead to avoid allocations? Same thing for the other 3 handlers defined below as well.

{
var sender = message.Page;
var enabled = message.IsBusy;

if (!PageIsChildOfPlatform(sender))
return;
busyCount = Math.Max(0, enabled ? busyCount + 1 : busyCount - 1);
UIApplication.SharedApplication.NetworkActivityIndicatorVisible = busyCount > 0;
});

MessagingCenter.Subscribe(this, Page.AlertSignalName, (Page sender, AlertArguments arguments) =>
WeakReferenceMessenger.Default.Register<Platform, PageAlertMessage>(this, (platform, message) =>
{
var sender = message.Page;
var arguments = message.Arguments;

if (!PageIsChildOfPlatform(sender))
return;
PresentAlert(arguments);
});

MessagingCenter.Subscribe(this, Page.PromptSignalName, (Page sender, PromptArguments arguments) =>
WeakReferenceMessenger.Default.Register<Platform, PromptMessage>(this, (platform, message) =>
{
var sender = message.Page;
var arguments = message.Arguments;

if (!PageIsChildOfPlatform(sender))
return;
PresentPrompt(arguments);
});

MessagingCenter.Subscribe(this, Page.ActionSheetSignalName, (Page sender, ActionSheetArguments arguments) =>
WeakReferenceMessenger.Default.Register<Platform, ActionSheetMessage>(this, (platform, message) =>
{
var sender = message.Page;
var arguments = message.Arguments;

if (!PageIsChildOfPlatform(sender))
return;

Expand All @@ -695,10 +709,6 @@ static bool IsModalPresentedFullScreen(Page modal)

internal void UnsubscribeFromAlertsAndActionsSheets()
{
MessagingCenter.Unsubscribe<Page, ActionSheetArguments>(this, Page.ActionSheetSignalName);
MessagingCenter.Unsubscribe<Page, AlertArguments>(this, Page.AlertSignalName);
MessagingCenter.Unsubscribe<Page, PromptArguments>(this, Page.PromptSignalName);
MessagingCenter.Unsubscribe<Page, bool>(this, Page.BusySetSignalName);
}

internal void MarkForRemoval()
Expand Down
12 changes: 7 additions & 5 deletions src/Compatibility/Core/src/iOS/Renderers/NavigationRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
using PointF = CoreGraphics.CGPoint;
using RectangleF = CoreGraphics.CGRect;
using SizeF = CoreGraphics.CGSize;
using CommunityToolkit.Mvvm.Messaging;

namespace Microsoft.Maui.Controls.Compatibility.Platform.iOS
{
public class NavigationRenderer : UINavigationController, IVisualElementRenderer, IEffectControlProvider
{
internal const string UpdateToolbarButtons = "Xamarin.UpdateToolbarButtons";
bool _appeared;
bool _ignorePopCall;
bool _loaded;
Expand All @@ -40,11 +40,11 @@ public class NavigationRenderer : UINavigationController, IVisualElementRenderer
[Preserve(Conditional = true)]
public NavigationRenderer() : base(typeof(FormsNavigationBar), null)
{
MessagingCenter.Subscribe<IVisualElementRenderer>(this, UpdateToolbarButtons, sender =>
WeakReferenceMessenger.Default.Register<NavigationRenderer, UpdateToolBarButtonsMessage>(this, static (receiver, message) =>
{
if (!ViewControllers.Any())
if (!receiver.ViewControllers.Any())
return;
var parentingViewController = GetParentingViewController();
var parentingViewController = receiver.GetParentingViewController();
parentingViewController?.UpdateLeftBarButtonItem();
});
}
Expand Down Expand Up @@ -262,7 +262,7 @@ protected override void Dispose(bool disposing)

if (disposing)
{
MessagingCenter.Unsubscribe<IVisualElementRenderer>(this, UpdateToolbarButtons);
WeakReferenceMessenger.Default.Unregister<UpdateToolBarButtonsMessage>(this);

foreach (var childViewController in ViewControllers)
childViewController.Dispose();
Expand Down Expand Up @@ -1711,4 +1711,6 @@ protected override void Dispose(bool disposing)
}
}
}

internal sealed class UpdateToolBarButtonsMessage { }
}
8 changes: 5 additions & 3 deletions src/Compatibility/Core/src/iOS/Renderers/TabletFlyoutPageRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.Maui.Graphics;
using ObjCRuntime;
using UIKit;
using CommunityToolkit.Mvvm.Messaging;

namespace Microsoft.Maui.Controls.Compatibility.Platform.iOS
{
Expand Down Expand Up @@ -391,7 +392,8 @@ public override void WillRotate(UIInterfaceOrientation toInterfaceOrientation, d
}

FlyoutPage.UpdateFlyoutLayoutBehavior();
MessagingCenter.Send<IVisualElementRenderer>(this, NavigationRenderer.UpdateToolbarButtons);

WeakReferenceMessenger.Default.Send(new UpdateToolBarButtonsMessage());
Copy link

@Sergio0694 Sergio0694 Dec 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this can be simplified:

Suggested change
WeakReferenceMessenger.Default.Send(new UpdateToolBarButtonsMessage());
WeakReferenceMessenger.Default.Send<UpdateToolBarButtonsMessage>();

Same for the other two occurrences below in this file.

}

base.WillRotate(toInterfaceOrientation, duration);
Expand Down Expand Up @@ -449,7 +451,7 @@ void ClearControllers()
void HandleFlyoutPropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == Page.IconImageSourceProperty.PropertyName || e.PropertyName == Page.TitleProperty.PropertyName)
MessagingCenter.Send<IVisualElementRenderer>(this, NavigationRenderer.UpdateToolbarButtons);
WeakReferenceMessenger.Default.Send(new UpdateToolBarButtonsMessage());
}

void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
Expand All @@ -470,7 +472,7 @@ void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
else if (e.Is(Microsoft.Maui.Controls.FlyoutPage.FlyoutLayoutBehaviorProperty))
UpdateFlyoutLayoutBehavior(base.View.Bounds.Size);

MessagingCenter.Send<IVisualElementRenderer>(this, NavigationRenderer.UpdateToolbarButtons);
WeakReferenceMessenger.Default.Send(new UpdateToolBarButtonsMessage());
}

public override void ViewWillTransitionToSize(CGSize toSize, IUIViewControllerTransitionCoordinator coordinator)
Expand Down
11 changes: 5 additions & 6 deletions src/Compatibility/Maps/src/Android/MapRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@
using Math = System.Math;
using Polygon = Microsoft.Maui.Controls.Maps.Polygon;
using Polyline = Microsoft.Maui.Controls.Maps.Polyline;
using CommunityToolkit.Mvvm.Messaging;

namespace Microsoft.Maui.Controls.Compatibility.Maps.Android
{
public class MapRenderer : ViewRenderer<Map, MapView>, GoogleMap.IOnCameraMoveListener, IOnMapReadyCallback
{
const string MoveMessageName = "MapMoveToRegion";

static Bundle s_bundle;

bool _disposed;
Expand Down Expand Up @@ -82,8 +81,8 @@ protected override void Dispose(bool disposing)
{
if (Element != null)
{
MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName);

WeakReferenceMessenger.Default.Unregister<MapSpan>(this);
((ObservableCollection<Pin>)Element.Pins).CollectionChanged -= OnPinCollectionChanged;
foreach (Pin pin in Element.Pins)
{
Expand Down Expand Up @@ -142,7 +141,7 @@ protected override void OnElementChanged(ElementChangedEventArgs<Map> e)
child.PropertyChanged -= MapElementPropertyChanged;
}

MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName);
WeakReferenceMessenger.Default.Unregister<MapSpan>(this);

if (NativeMap != null)
{
Expand All @@ -158,7 +157,7 @@ protected override void OnElementChanged(ElementChangedEventArgs<Map> e)

Control.GetMapAsync(this);

MessagingCenter.Subscribe<Map, MapSpan>(this, MoveMessageName, OnMoveToRegionMessage, Map);
WeakReferenceMessenger.Default.Register<MapRenderer, MapSpan>(this, static (r, m) => r.OnMoveToRegionMessage(r.Element, m));

((INotifyCollectionChanged)Map.Pins).CollectionChanged += OnPinCollectionChanged;
((INotifyCollectionChanged)Map.MapElements).CollectionChanged += OnMapElementCollectionChanged;
Expand Down
10 changes: 5 additions & 5 deletions src/Compatibility/Maps/src/iOS/MapRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.Maui.Controls.Maps;
using Microsoft.Maui.Graphics;
using Microsoft.Maui.Controls.Platform;
using CommunityToolkit.Mvvm.Messaging;

#if __MOBILE__
using UIKit;
Expand All @@ -34,8 +35,6 @@ public class MapRenderer : ViewRenderer
UITapGestureRecognizer _mapClickedGestureRecognizer;
#endif

const string MoveMessageName = "MapMoveToRegion";

public override SizeRequest GetDesiredSize(double widthConstraint, double heightConstraint)
{
return Control.GetSizeRequest(widthConstraint, heightConstraint);
Expand Down Expand Up @@ -64,7 +63,7 @@ protected override void Dispose(bool disposing)
if (Element != null)
{
var mapModel = (Map)Element;
MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName);
WeakReferenceMessenger.Default.Unregister<MapSpan>(this);
((ObservableCollection<Pin>)mapModel.Pins).CollectionChanged -= OnPinCollectionChanged;
((ObservableCollection<MapElement>)mapModel.MapElements).CollectionChanged -= OnMapElementCollectionChanged;
foreach (Pin pin in mapModel.Pins)
Expand Down Expand Up @@ -118,7 +117,7 @@ protected override void OnElementChanged(ElementChangedEventArgs<View> e)
{
var mapModel = (Map)e.OldElement;

MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName);
WeakReferenceMessenger.Default.Unregister<MapSpan>(this);

((ObservableCollection<Pin>)mapModel.Pins).CollectionChanged -= OnPinCollectionChanged;
foreach (Pin pin in mapModel.Pins)
Expand Down Expand Up @@ -165,7 +164,8 @@ protected override void OnElementChanged(ElementChangedEventArgs<View> e)
#endif
}

MessagingCenter.Subscribe<Map, MapSpan>(this, MoveMessageName, (s, a) => MoveToRegion(a), mapModel);
WeakReferenceMessenger.Default.Register<MapRenderer, MapSpan>(this, static (r, m) => r.MoveToRegion(m));

if (mapModel.LastMoveToRegion != null)
MoveToRegion(mapModel.LastMoveToRegion, false);

Expand Down
Loading