DYN-7099: notification focus fix #16321
Merged
Conversation
This file contains hidden or 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
- fix to the notification popup window to allow Dynamo window to be closed correctly (without having to hit 'X' multiple times)
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7099
- potential race condition to execute refocus during Dynamo window tear down (not sure why the notification popup would be involved, but possibly linked)
- testing if synchronous call might be better for the smoke test
- fixed test failing when no application is found during headless test execution
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the notification popup’s focus handling by reclaiming the main window’s focus when the popup opens and cleans up some XAML formatting.
- Adds an
Openedevent handler in XAML and implementsNotificationUI_Openedto force focus back to the main window. - Implements native calls (
SetForegroundWindow,ReleaseCapture) via P/Invoke inNotificationUI.xaml.cs. - Applies auto-formatting changes to imports and XAML attributes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Notifications/View/NotificationUI.xaml.cs | Added NotificationUI_Opened handler, ForceMainWindowFocus P/Invoke logic, plus formatting updates |
| src/Notifications/View/NotificationUI.xaml | Wired Opened event, reordered attributes, and cleaned up XAML formatting |
Comments suppressed due to low confidence (2)
src/Notifications/View/NotificationUI.xaml.cs:74
- This new focus-restoration logic is critical for UX; consider adding an integration or UI test to verify the main window regains focus when the popup opens.
private void NotificationUI_Opened(object sender, EventArgs e)
src/Notifications/View/NotificationUI.xaml.cs:80
- Add XML documentation comments to
ForceMainWindowFocus(andNotificationUI_Opened) to explain their purpose and any platform-specific implications.
private void ForceMainWindowFocus()
Comment on lines
85
to
93
| ReleaseCapture(); | ||
| SetForegroundWindow(new WindowInteropHelper(Application.Current.MainWindow).Handle); | ||
| } | ||
|
|
||
| [DllImport("user32.dll")] | ||
| [return: MarshalAs(UnmanagedType.Bool)] | ||
| private static extern bool SetForegroundWindow(IntPtr hWnd); | ||
|
|
||
| [DllImport("user32.dll")] |
There was a problem hiding this comment.
Consider adding SetLastError = true to the DllImport attribute and checking Marshal.GetLastWin32Error() after the call to aid debugging if the native call fails.
Suggested change
| ReleaseCapture(); | |
| SetForegroundWindow(new WindowInteropHelper(Application.Current.MainWindow).Handle); | |
| } | |
| [DllImport("user32.dll")] | |
| [return: MarshalAs(UnmanagedType.Bool)] | |
| private static extern bool SetForegroundWindow(IntPtr hWnd); | |
| [DllImport("user32.dll")] | |
| if (!ReleaseCapture()) | |
| { | |
| int errorCode = Marshal.GetLastWin32Error(); | |
| Console.WriteLine($"ReleaseCapture failed with error code: {errorCode}"); | |
| } | |
| if (!SetForegroundWindow(new WindowInteropHelper(Application.Current.MainWindow).Handle)) | |
| { | |
| int errorCode = Marshal.GetLastWin32Error(); | |
| Console.WriteLine($"SetForegroundWindow failed with error code: {errorCode}"); | |
| } | |
| } | |
| [DllImport("user32.dll", SetLastError = true)] | |
| [return: MarshalAs(UnmanagedType.Bool)] | |
| private static extern bool SetForegroundWindow(IntPtr hWnd); | |
| [DllImport("user32.dll", SetLastError = true)] |
aparajit-pratap
approved these changes
Jun 25, 2025
jasonstratton
approved these changes
Jun 25, 2025
Contributor
jasonstratton
left a comment
There was a problem hiding this comment.
As you mentioned in the meeting earlier, there was a lot of formatting changes in the resx, but they are not impactful. The core change of regaining the focus did the trick. Looks good.
Contributor
- added SetLastError to true - surfacing the errors
aparajit-pratap
approved these changes
Jun 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Purpose
Addresses an issue where the notification popup window was not getting focus correctly after activation. This would lead to the user not being able to close the Dynamo window unless they hit the close 'X' button multiple times. Furthermore, the webView2-hosted component would not correctly capture the mouseover 'hover' interactions. This PR fixes that.
Captured in this jira ticket: Dynamo does not close when click on X with notification open
Before
After
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Reviewers
@zeusongit
@reddyashish
FYIs
@jasonstratton
@achintyabhat