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

DisplayActionSheet later #5047

Merged
merged 8 commits into from
Mar 9, 2022

Conversation

cshung
Copy link
Member

@cshung cshung commented Mar 4, 2022

Description of Change

There are a couple of issues with this bug.

  1. We need to wait further before handling the pending actions, and
  2. Async operation needs to be completed no matter what.

The description reflects the execution sequence:

By the time DisplayActionSheet is called in OnAppearing, the Page knows it IsPlatformEnabled is false, so it adds this task into a _pendingActions list.

This list will be drained when IsPlatformEnabled is set to true during CreatePlatformWindow. While the Handler is available, the underlying WinUI objects are not on the VisualTree yet, so it isn't really ready to run the _pendingActions.

In particular, at that point of time, the page is not null, but its parent is null, so the call will silently terminate without signaling the async result, and the async call would appear to be forever executing. So I changed the code to SetResult(null) even in those cases, those case should not happen, but it could if the user calls the API inappropriately.

If we give the page instead of its parent to actionSheet.ShowAt(), the call would fail with an E_INVALIDARG returned from the underlying native code, presumably because it is not in the VisualTree as the comment indicates. The exception handler would try to use UI.Xaml.Window.Current.Content instead, but UI.Xaml.Window.Current is null at that point of time, further confirming it is too early to make this call.

I experimented with moving the flushing of the _pendingActions to some time later, such as OnSizeAllocated, this will work. This is not ideal though, as the method is called quite frequently on resizing. I am wondering if there is a better place to flush them. It has to be at a point where the objects are already in the VisualTree.

The code is tested on both Windows and Android emulator, at the moment I am not able to test on other platforms. I assumed that OnLoaded should function properly on all platforms.

Issues Fixed

Fixes #3726

@cshung cshung force-pushed the public/display-action-sheet-later branch from 45a39bd to ba3a330 Compare March 4, 2022 21:44
@cshung cshung changed the title [WIP] DisplayActionSheet later DisplayActionSheet later Mar 4, 2022
@Redth Redth requested a review from PureWeen March 8, 2022 02:34
@PureWeen PureWeen enabled auto-merge (squash) March 9, 2022 17:26
@PureWeen PureWeen merged commit fffaae1 into dotnet:main Mar 9, 2022
@cshung cshung deleted the public/display-action-sheet-later branch March 9, 2022 18:27
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-6.0.300-rc.1 Look for this fix in 6.0.300-rc.1! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-6.0.300-rc.1 Look for this fix in 6.0.300-rc.1! platform/windows 🪟
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DisplayActionSheet not opening the action dialog on Windows
5 participants