CommunityToolkit
[BUG] Popup V2 Sometimes Waits Indefinitely
Is there an existing issue for this?
- [x] I have searched the existing issues
Did you read the "Reporting a bug" section on Contributing file?
- [x] I have read the "Reporting a bug" section on Contributing file: https://github.com/CommunityToolkit/Maui/blob/main/CONTRIBUTING.md#reporting-a-bug
Current Behavior
In some circumstances an await on ShowPopupAsync will wait indefinitely without showing a popup.
Expected Behavior
The popup should show.
Steps To Reproduce
- Download the repository from GitHub.
- Open, build and run the project on Windows.
You should see this (you can using the workaround branch):
But what you'll actually see is this (from the main branch:
Link to public reproduction project repository
https://github.com/david-maw/PopupPage.git
Environment
- .NET MAUI CommunityToolkit: 1.2.0
- OS: Windows 11
- .NET MAUI: 9.0.80
Anything else?
The failing program is in the main branch, there's a workaround in the workaround branch. The workaround is to add
await Task.Delay(100);
before the await on ShowPopupAsync.
Because the workaround is trivial, this is not a significant problem for me.
Why are you calling base.OnAppearing inside your event handle for Loaded? This doesn't seem like something you should be doing
Because I copied the code from OnAppearing where it originally was and forgot to remove that call! I don't think it's doing any harm, but it shouldn't be there, sorry about that.
With a quick test running on an emulator this is behaving although my emulator is only targeting SDK version 30 so it isn't the latest.
One thing to consider rather than using await Task.Delay(100); would be to use Dispatcher.Dispatch(). This doesn't get to the bottom of the issue but it will provide a more reliable workaround
Sorry my bad, it works fine on Android, it's Windows where it fails, I should have made that clear.
I could not persuade Dispatcher.Dispatch() to compile, but it was a promising idea, I used await Task.Yield() which also worked around the problem and has less of a code smell about it.
FYI, the changes to the sample app are trivial (remove the offending OnAppearing call and replace Delay with Yield), but I pushed them to GitHub anyway.
@david-maw I am writing this in the browser so I apologise if it doesn't work, for the Dispatcher option I was thinking something like:
Dispatcher.Display(async () =>
{
await this.ShowPopupAsync(new Label
{
Text = "Tap to dismiss this popup",
BackgroundColor = Colors.Red
});
});
As for when it fails - have you tried placing a try catch around the call to ShowPopupAsync and observed whether any exceptions are being thrown?
If all of that fails then I wonder if the platform isn't ready yet. I mentioned that I am unable to reproduce the issue but if you can reliably and the try catch fails I could knock up a change which might help - would you be willing to test the output of a PR build to verify it?
Sorry, I'd taken you literally! , You can't just use Dispatcher.Dispatch because then the program execution continues immediately and exits SimulatedInitialization before the popup has shown, but what you can do is:
await Dispatcher.DispatchAsync(async () =>
{
await this.ShowPopupAsync(new Label
{
Text = "Tap to dismiss this popup",
BackgroundColor = Colors.Red
});
});
And that's also a fine workaround.
On the theory that we might not be on the UI thread I checked Environment.CurrentManagedThreadId in a couple of places. It was always 1, which is what I'd expect, so there is probably just some unfinished navigation screwing things up. Both the Yield() and DispatchAsync() would give that a chance to finish before showing the popup.
Sorry, I hadn't realized you couldn't reproduce it on Windows (the only place it fails for me). There's no catchable exception, the await ShowPopupAsync just never completes, perhaps there's an exception inside the popup code that's being caught but not recovered from correctly?
This does remind me that I use popup as a prettier DisplayAlert and do not worry much about whether I'm on the UI thread at the time. Is that how it's supposed to work?
I'm certainly willing to test a proposed change if you wish.
No that's on me I was testing on Android and not Windows. I'll try testing on windows later in the week.
As for using the UI thread I would strongly recommend that you do for using the popup. I'm not convinced we do any marshalling to help there
ok, if it works for you on Windows that opens a whole new can of worms, so I'll keep my fingers crossed!
Probably worth thinking about whether that UI thread requirement is a new one with popup V2 and either documenting the change or removing the requirement (which would be my preference) if it is.
@bijington were you able to reproduce this on Windows?
Not yet sadly. Work has been rather chaotic in recent weeks to keep me too tired from doing much in the evenings. It is on my list though
Yes, work can rather get in the way of life sometimes, no problem, I have a workaround.
FYI, I was able to apply a simple change to guarantee the UI thread is always used for my popups (it probably was before, but those seem like Famous Last Words to me).