DYN-7099: notification focus fix #16321

dnenov · 2025-06-24T17:18:43Z

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

Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated
This PR contains no files larger than 50 MB
This PR introduces new feature code involve network connecting and is tested with no-network mode.

Release Notes

Fix the notification popup window to allow Dynamo window to be closed correctly (without having to hit 'X' multiple times)
small auto-formatting changes
Testing not added as no new logic is introduced, just window focus handling

Reviewers

@zeusongit
@reddyashish

FYIs

@jasonstratton
@achintyabhat

- fix to the notification popup window to allow Dynamo window to be closed correctly (without having to hit 'X' multiple times)

github-actions

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

- clean up alternative code

- fixed test failing when no application is found during headless test execution

Copilot

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 Opened event handler in XAML and implements NotificationUI_Opened to force focus back to the main window.
Implements native calls (SetForegroundWindow, ReleaseCapture) via P/Invoke in NotificationUI.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 (and NotificationUI_Opened) to explain their purpose and any platform-specific implications.

        private void ForceMainWindowFocus()

Copilot · 2025-06-25T20:46:04Z

src/Notifications/View/NotificationUI.xaml.cs

+            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")]


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)]

Good suggestion.

That's now done.

jasonstratton

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.

zeusongit · 2025-06-26T04:51:00Z

passed here: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/18055/

- added SetLastError to true - surfacing the errors

notification focus fix

5da184e

- fix to the notification popup window to allow Dynamo window to be closed correctly (without having to hit 'X' multiple times)

github-actions bot changed the title ~~notification focus fix~~ DYN-7099: notification focus fix Jun 24, 2025

github-actions bot reviewed Jun 24, 2025

View reviewed changes

dnenov added 4 commits June 25, 2025 12:38

fix failing smoke test

ab7c5bf

- potential race condition to execute refocus during Dynamo window tear down (not sure why the notification popup would be involved, but possibly linked)

test if changes are responsible for smoke test failure

3f109a6

sync call focus

92f7c94

- testing if synchronous call might be better for the smoke test

clean-up

1332b47

- clean up alternative code

dnenov requested a review from jasonstratton June 25, 2025 15:39

test fail fix

551b3a3

- fixed test failing when no application is found during headless test execution

aparajit-pratap requested a review from Copilot June 25, 2025 20:45

Copilot AI reviewed Jun 25, 2025

View reviewed changes

aparajit-pratap approved these changes Jun 25, 2025

View reviewed changes

jasonstratton approved these changes Jun 25, 2025

View reviewed changes

capture errors for native calls

e5e1e64

- added SetLastError to true - surfacing the errors

aparajit-pratap approved these changes Jun 30, 2025

View reviewed changes

zeusongit merged commit e806d54 into DynamoDS:master Jul 2, 2025
27 of 28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

DYN-7099: notification focus fix #16321

DYN-7099: notification focus fix #16321

Uh oh!

dnenov commented Jun 24, 2025 •

edited

Loading

Uh oh!

github-actions bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 25, 2025

Uh oh!

aparajit-pratap Jun 26, 2025

Uh oh!

dnenov Jun 30, 2025

Uh oh!

jasonstratton left a comment

Uh oh!

zeusongit commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!

DYN-7099: notification focus fix #16321

DYN-7099: notification focus fix #16321

Uh oh!

Conversation

dnenov commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Before

After

Declarations

Release Notes

Reviewers

FYIs

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

aparajit-pratap Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

dnenov Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

jasonstratton left a comment

Choose a reason for hiding this comment

Uh oh!

zeusongit commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!

dnenov commented Jun 24, 2025 •

edited

Loading