Skip to content

Change to display Popup on Active Window page on Windows#1617

Merged
TheCodeTraveler merged 9 commits intoCommunityToolkit:mainfrom
cat0363:Issue-1616
Mar 25, 2024
Merged

Change to display Popup on Active Window page on Windows#1617
TheCodeTraveler merged 9 commits intoCommunityToolkit:mainfrom
cat0363:Issue-1616

Conversation

@cat0363
Copy link
Copy Markdown
Contributor

@cat0363 cat0363 commented Dec 21, 2023

This PR resolves the issue where popups are not displayed on the active window page on Windows.

Description of Change

To get the active windows in Windows, look for each window whose IsActivated property is true in Application.Current.Windows.
Call the ShowPopup and ShowPopupAsync methods on the active window page.
Below are the changes.

[src\CommunityToolkit.Maui\PopupService.cs]

public void ShowPopup<TViewModel>() where TViewModel : INotifyPropertyChanged
{
    var popup = GetPopup(typeof(TViewModel));

    ValidateBindingContext<TViewModel>(popup, out _);

#if WINDOWS
    Application.Current!.Windows.First(x => x.IsActivated).Page!.ShowPopup(popup);
#else
    CurrentPage.ShowPopup(popup);
#endif
}

public void ShowPopup<TViewModel>(TViewModel viewModel) where TViewModel : INotifyPropertyChanged
{
    ArgumentNullException.ThrowIfNull(viewModel);

    var popup = GetPopup(typeof(TViewModel));

    ValidateBindingContext<TViewModel>(popup, out _);

#if WINDOWS
    Application.Current!.Windows.First(x => x.IsActivated).Page!.ShowPopup(popup);
#else
    CurrentPage.ShowPopup(popup);
#endif
}

public void ShowPopup<TViewModel>(Action<TViewModel> onPresenting) where TViewModel : INotifyPropertyChanged
{
    ArgumentNullException.ThrowIfNull(onPresenting);

    var popup = GetPopup(typeof(TViewModel));

    ValidateBindingContext(popup, out TViewModel viewModel);

    onPresenting.Invoke(viewModel);

#if WINDOWS
    Application.Current!.Windows.First(x => x.IsActivated).Page!.ShowPopup(popup);
#else
    CurrentPage.ShowPopup(popup);
#endif
}

public Task<object?> ShowPopupAsync<TViewModel>(CancellationToken token = default) where TViewModel : INotifyPropertyChanged
{
    var popup = GetPopup(typeof(TViewModel));

    ValidateBindingContext<TViewModel>(popup, out _);

#if WINDOWS
    return Application.Current!.Windows.First(x => x.IsActivated).Page!.ShowPopupAsync(popup, token);
#else
    return CurrentPage.ShowPopupAsync(popup, token);
#endif
}

public Task<object?> ShowPopupAsync<TViewModel>(TViewModel viewModel, CancellationToken token = default) where TViewModel : INotifyPropertyChanged
{
    ArgumentNullException.ThrowIfNull(viewModel);

    var popup = GetPopup(typeof(TViewModel));

    ValidateBindingContext<TViewModel>(popup, out _);

#if WINDOWS
    return Application.Current!.Windows.First(x => x.IsActivated).Page!.ShowPopupAsync(popup, token);
#else
    return CurrentPage.ShowPopupAsync(popup, token);
#endif
}

Linked Issues

PR Checklist

Additional information

Below are the verification results.

2023-12-21.180127.mp4

You can see that the Popup is displayed in the active window.

@cat0363
Copy link
Copy Markdown
Contributor Author

cat0363 commented Dec 21, 2023

In cases where a popup is displayed by setting a timer, it will not be displayed intentionally. . .
In such cases, the active window may change from the Popup's caller until the Popup is displayed.
If you need to consider such cases, please discard this PR.

@cat0363
Copy link
Copy Markdown
Contributor Author

cat0363 commented Dec 21, 2023

It would be best if there was a way to determine which page is calling the Popup. . .

@cat0363
Copy link
Copy Markdown
Contributor Author

cat0363 commented Jan 19, 2024

Perhaps to solve this problem we need some variation of Popup's ShowPopup, ShowPopupAsync methods that pass an instance of the page we want to display.
If you use the current page, the problem I previously described will occur.

VladislavAntonyuk
VladislavAntonyuk reviewed Feb 8, 2024
View reviewed changes
Comment thread src/CommunityToolkit.Maui/PopupService.cs Outdated

ValidateBindingContext<TViewModel>(popup, out _);

#if WINDOWS
Copy link
Copy Markdown
Contributor

@VladislavAntonyuk VladislavAntonyuk Feb 8, 2024

Choose a reason for hiding this comment

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

can you extract it to the single method and call only once? also could you please remove all null-forgiving operators. they are forbidden in this repo

Copy link
Copy Markdown
Contributor Author

@cat0363 cat0363 Feb 13, 2024

Choose a reason for hiding this comment

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

@VladislavAntonyuk , Thank you for your review.
I cut it out to a single method and removed null-forgiving.

@bijington
Copy link
Copy Markdown
Contributor

bijington commented Feb 26, 2024

I am just wondering if we could opt for registering the PopupService as scoped given .NET MAUI scopes per window, then we could somehow obtain the active window at the time of resolution and rely on that through the life of the PopupService instance. What do people think?

@TheCodeTraveler TheCodeTraveler enabled auto-merge (squash) March 25, 2024 22:49
@TheCodeTraveler TheCodeTraveler merged commit 5684eb7 into CommunityToolkit:main Mar 25, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@VladislavAntonyuk VladislavAntonyuk VladislavAntonyuk left review comments

@TheCodeTraveler TheCodeTraveler TheCodeTraveler approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Popup show on unexpected window when app using multiple window

4 participants

@cat0363 @bijington @TheCodeTraveler @VladislavAntonyuk