[macOS] Display alert by Window #14361
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsuarezruiz
added
t/bug
ghost
added
the
legacy-area-controls
mandel-macaque
previously requested changes
Apr 3, 2023
src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs
Outdated
Show resolved
Hide resolved
src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs
Outdated
Show resolved
Hide resolved
src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs
Outdated
Show resolved
Hide resolved
mattleibow
reviewed
Apr 3, 2023
Comment on lines
-247
to
+249
| bool PageIsInThisWindow(Page page) | ||
| { | ||
| var window = page?.Window; | ||
| var platformWindow = window?.MauiContext.GetPlatformWindow(); | ||
|
|
||
| if (window is null || platformWindow is null) | ||
| return false; | ||
|
|
||
| return platformWindow == Window; | ||
| } | ||
| bool PageIsInThisWindow(Page page) => | ||
| page?.Window == VirtualView; |
There was a problem hiding this comment.
The alert manager already knows about the window, so this is now possible. The iOS code had everything set up, but Windows just needed an extra constructor argument for the helper to simplify all this.
mattleibow
approved these changes
Apr 3, 2023
mattleibow
dismissed
mandel-macaque’s stale review
All the logic has been rewritten.
PureWeen
previously requested changes
Apr 3, 2023
There was a problem hiding this comment.
On windows if you close a window while a dialog box is open then dialog boxes stop working all together
- open the Gallery
- navigate to core => multi window
- open a new window
- click "open dialog" on that new window
- close the new window without closing the dialog
- now try to click "open dialog" on the original window
|
Oh wow. Let me look at that then... |
mattleibow
reviewed
Apr 3, 2023
Comment on lines
-40
to
+41
| static Task<bool>? CurrentAlert; | ||
| static Task<string?>? CurrentPrompt; | ||
| Task<bool>? CurrentAlert; | ||
| Task<string?>? CurrentPrompt; |
There was a problem hiding this comment.
@PureWeen this was causing the issue. No need for statics anymore! We have multiple windows!
mattleibow
added
the
backport/suggested
| } | ||
|
|
||
| internal void Unsubscribe(Window window) | ||
| { | ||
| var platformWindow = window.MauiContext.GetPlatformWindow(); | ||
|
|
||
| var toRemove = Subscriptions.Where(s => s.Window == platformWindow).ToList(); | ||
| var toRemove = Subscriptions.Where(s => s.PlatformView == platformWindow).ToList(); |
There was a problem hiding this comment.
@PureWeen I though that the use of LINQ was limited, should we remove it from here?
mandel-macaque
approved these changes
Apr 4, 2023
hartez
added
the
backport/approved
|
/backport to net7.0 |
|
Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/5270383073 |
|
@hartez backporting to net7.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Display alert by Window on macOS
Applying: Unify the logic a bit
error: sha1 information is lacking or useless (src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Unify the logic a bit
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@hartez an error occurred while backporting to net7.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
hartez
added a commit
that referenced
this pull request
Jun 14, 2023
* Display alert by Window on macOS * Unify the logic a bit --------- Co-authored-by: Matthew Leibowitz <[email protected]>
rmarinho
pushed a commit
that referenced
this pull request
Jun 15, 2023
* Display alert by Window on macOS * Unify the logic a bit --------- Co-authored-by: Matthew Leibowitz <[email protected]>
Eilon
removed
the
legacy-area-controls
samhouts
added
the
fixed-in-8.0.0-preview.4.8333
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Display alert by Window on macOS.
This fix is similar to #14229
To test the changes, launch the .NET MAUI Gallery and navigate to Core > MultiWindow. Open a secondary Window and then, tap the button to display an alert. The alert only must appear in the parent Window.
Issues Fixed
Fixes #14343