Closed brminnick closed 1 year ago
Approve. Can we change only return type from void to Task/ValueTask and keep the name “Close”? The app will behave the same until user fix the warning and await method
Thanks Vlad! That was my initial plan, but I decided against replacing the existing void Close()
method for two reasons:
void Close()
-> Task Close()
would be a breaking changeawait
the closure of a Popup (eg using a non-async
button handler) That being said, you know how much I love async/await and I always love to push developers to use the "better" API, so if the other maintainers agree that we should replace void Close()
with Task Close()
, I'm happy to commit!
Approve. I like the solution you propose @brminnick for the following reasons:
ShowPopup
/Close
and the ShowPopupAsync
/CloseAsync
methods.I'm happy to approve this also
Great! Approved ✅
Hey, quick question. Does it solve #1111 ?
Does it solve #1111 ?
Yup! Thanks, I've added linked it to the PR.
Reopening Proposal.
Only Proposals moved to the Closed
Project Column and Completed
Project Column can be closed.
Feature name
Add
Popup.CloseAsync()
Link to discussion
Discussed in June 2023 Standup:
https://github.com/CommunityToolkit/Maui/wiki/2023-June-Standup
Progress tracker
Summary
This Proposal adds the following API to
Popup
:CloseAsync()
returns once the operating system has dismissedPopup
from the page.Motivation
Currently,
Popup
only offers one method to programmatically dismiss it from the screen:public void Close();
.However, on MacCatalyst and iOS, the code required to dismiss the Popup uses async/await to dismiss the
UIViewController
:https://github.com/CommunityToolkit/Maui/blob/e89e7dabd2c348601f75dc68867f6cf6486d8595/src/CommunityToolkit.Maui.Core/Handlers/Popup/PopupHandler.macios.cs#L14-L23
The existing
Close()
API is thus acting in a fire-and-forget manner; the method is returning to the caller before the Popup has been dismissed on iOS + MacCatalyst.Detailed Design
IPopup.shared.cs
This API update requires a
TaskCompletionSource
inIPopup
that can be referenced by both the Handler and the Control.This is required because the PropertyMappers / CommandMappers that .NET MAUI use for Handlers are not asynchronous (they cannot be
await
d).We will instead tell the Control to
await PopupDismissedTaskCompletionSource.Task
afterIPopup.Closed()
has been called. Then, inPopupHandler.MapOnClosed
, we will callPopupDismissedTaskCompletionSource TrySetResult()
after the operating system has dismissed thePopup
from the page.Popup.shared.cs
This Proposal also requires a new API
public Task Popup.CloseAsync(object? result = null)
;Usage Syntax
Drawbacks
Only iOS + MacCatalyst defer to a different thread when dismissing the Popup; Android and Windows can dismiss the Popup synchronously. Adding a method that returns
Task
adds a bit of overhead to our users, however, the overhead should be negligible as users typically only display, and subsequently close, one Popup at a time.Alternatives
This Proposal can be updated to remove the fire-and-forget method,
void Close()
.However, removing an existing API is a breaking change.
Unresolved Questions
Should we instead return
ValueTask
?