-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Replace MessagingCenter in AlertManager #27888
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
Conversation
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
/rebase |
9b67592
to
518636c
Compare
@jfversluis Could you rebase and fix the conflict? Thanks in advance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the deprecated MessagingCenter usage from the AlertManager and related components in favor of direct method calls, in line with the plan to decouple MessagingCenter from alert interactions. Key changes include:
- Removal of MessagingCenter subscription/unsubscription in tests and platform implementations.
- Replacement of MessagingCenter.Send calls in Page.cs with corresponding AlertManager Request calls.
- Adjustments in Window.cs and NavigationPage.cs to align with the new AlertManager APIs.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Controls/tests/Core.UnitTests/PageTests.cs | Removed MessagingCenter invocations and related tests. |
src/Controls/tests/Core.UnitTests/AlertManagerTests.cs | Added unit tests validating the new AlertManager subscription behavior. |
src/Controls/src/Core/Window/Window.cs | Moved SendWindowAppearing call to occur after handler assignment for better synchronization. |
src/Controls/src/Core/Platform/AlertManager/*.cs | Updated platform-specific AlertManager implementations to use partial methods instead of MessagingCenter. |
src/Controls/src/Core/Page/Page.cs | Replaced MessagingCenter.Send calls with AlertManager.Request* calls for alerts, action sheets, prompts, and busy notifications. |
src/Controls/src/Core/NavigationPage/NavigationPage.cs | Reordered a child removal call to follow setting the current page. |
Comments suppressed due to low confidence (1)
src/Controls/src/Core/NavigationPage/NavigationPage.cs:787
- [nitpick] The reordering of removing the current page from inner children—now performed after setting the new current page—may impact page lifecycle or navigation behavior. Please verify that this change is intentional and consistent with expected navigation state transitions.
Owner.RemoveFromInnerChildren(currentPage);
Description of Change
This is #12910 but targeting .NET 10.
Since the
MessagingCenter
was obsolete and now marked internal, we would love to remove it! Now it is happening!!!This PR starts that process by removing usages of the
MessagingCenter
in theAlertManager
.Fixes #28857
Closes #12910