Skip to content

Commit 587385d

Browse files
Copilotdavidegiacomettiniels9001vanzue
authored
[Settings] Implement singleton pattern for ShortcutConflictWindow (#42440)
## Summary Fixes issue where multiple ShortcutConflictWindow instances could be opened simultaneously. The window now follows the same singleton pattern as OobeWindow - only one instance can exist at a time, and attempting to open another brings the existing window to the foreground. ## Changes Implemented singleton management for `ShortcutConflictWindow` following the established pattern used by `OobeWindow`: ### App.xaml.cs - Added static field to store the singleton window instance - Added `GetShortcutConflictWindow()`, `SetShortcutConflictWindow()`, and `ClearShortcutConflictWindow()` methods for lifecycle management ### ShortcutConflictWindow.xaml.cs - Updated `WindowEx_Closed` event handler to call `App.ClearShortcutConflictWindow()` to properly clean up the singleton reference when the window is closed ### Updated all three entry points that create ShortcutConflictWindow: - **ShortcutConflictControl.xaml.cs** (Dashboard conflict warning) - **ShortcutControl.xaml.cs** (Settings page shortcut controls) - **OobeOverview.xaml.cs** (OOBE overview page) Each location now checks if a window already exists using `App.GetShortcutConflictWindow()`: - If no window exists, creates a new one and registers it via `App.SetShortcutConflictWindow()` - If a window already exists, simply calls `Activate()` to bring it to the foreground ## Testing The fix ensures that: - ✅ Only one ShortcutConflictWindow can be open at a time - ✅ Clicking the shortcut conflict button when a window is already open activates the existing window instead of creating a duplicate - ✅ The window reference is properly cleared when closed, allowing a new instance to be created in future interactions Fixes #[issue_number] > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `i1qvsblobprodcus353.vsblob.vsassets.io` > - Triggering command: `dotnet build PowerToys.Settings.csproj --configuration Release` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/microsoft/PowerToys/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[Settings] Single Shortcuts Conflicts window</issue_title> > <issue_description>### Microsoft PowerToys version > > 0.95.0 > > ### Installation method > > GitHub > > ### Area(s) with issue? > > Settings > > ### Steps to reproduce > > Multiple shortcut conflicts window can be launched. > Should have the same behavior of OOBE window. > If shortcut conflicts window is already opened, pressing the button should bring the window in foreground. > > ### ✔️ Expected Behavior > > Single shortcuts conflicts window > > ### ❌ Actual Behavior > > Multiple shortcut conflicts window can be launched > > ### Additional Information > > _No response_ > > ### Other Software > > _No response_</issue_description> > > <agent_instructions>Settings ShortcutConflictWindow should have the same behavior of OobeWindow. > When ShortcutConflictWindow is already opened, activate that window instead of opening another one. </agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes #42437 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: davidegiacometti <[email protected]> Co-authored-by: Davide Giacometti <[email protected]> Co-authored-by: Niels Laute <[email protected]> Co-authored-by: vanzue <[email protected]>
1 parent 528fb52 commit 587385d

File tree

7 files changed

+88
-18
lines changed

7 files changed

+88
-18
lines changed

src/settings-ui/Settings.UI/SettingsXAML/App.xaml.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
using Microsoft.PowerToys.Settings.UI.OOBE.ViewModel;
1818
using Microsoft.PowerToys.Settings.UI.SerializationContext;
1919
using Microsoft.PowerToys.Settings.UI.Services;
20+
using Microsoft.PowerToys.Settings.UI.SettingsXAML.Controls.Dashboard;
2021
using Microsoft.PowerToys.Settings.UI.Views;
2122
using Microsoft.PowerToys.Telemetry;
2223
using Microsoft.UI.Xaml;
@@ -35,6 +36,8 @@ public partial class App : Application
3536

3637
private ScoobeWindow scoobeWindow;
3738

39+
private ShortcutConflictWindow shortcutConflictWindow;
40+
3841
private enum Arguments
3942
{
4043
PTPipeName = 1,
@@ -336,10 +339,10 @@ public static MainWindow GetSettingsWindow()
336339
return settingsWindow;
337340
}
338341

339-
public static bool IsOobeOrScoobeOpen()
342+
public static bool IsSecondaryWindowOpen()
340343
{
341344
var app = (App)Current;
342-
return app.oobeWindow != null || app.scoobeWindow != null;
345+
return app.oobeWindow != null || app.scoobeWindow != null || app.shortcutConflictWindow != null;
343346
}
344347

345348
public void OpenScoobe()
@@ -384,6 +387,25 @@ public void OpenOobe()
384387
}
385388
}
386389

390+
public void OpenShortcutConflictWindow()
391+
{
392+
if (shortcutConflictWindow == null)
393+
{
394+
shortcutConflictWindow = new ShortcutConflictWindow();
395+
396+
shortcutConflictWindow.Closed += (_, _) =>
397+
{
398+
shortcutConflictWindow = null;
399+
};
400+
401+
shortcutConflictWindow.Activate();
402+
}
403+
else
404+
{
405+
WindowHelpers.BringToForeground(shortcutConflictWindow.GetWindowHandle());
406+
}
407+
}
408+
387409
public static Type GetPage(string settingWindow)
388410
{
389411
switch (settingWindow)

src/settings-ui/Settings.UI/SettingsXAML/Controls/Dashboard/ShortcutConflictControl.xaml.cs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using System;
6-
using System.Collections.Generic;
75
using System.ComponentModel;
8-
using System.Linq;
96
using Microsoft.PowerToys.Settings.UI.Library.HotkeyConflicts;
107
using Microsoft.PowerToys.Settings.UI.Library.Telemetry.Events;
118
using Microsoft.PowerToys.Settings.UI.SettingsXAML.Controls.Dashboard;
@@ -154,11 +151,7 @@ private void ShortcutConflictBtn_Click(object sender, RoutedEventArgs e)
154151
ConflictCount = this.ConflictCount,
155152
});
156153

157-
// Create and show the new window instead of dialog
158-
var conflictWindow = new ShortcutConflictWindow();
159-
160-
// Show the window
161-
conflictWindow.Activate();
154+
((App)App.Current)!.OpenShortcutConflictWindow();
162155
}
163156
}
164157
}

src/settings-ui/Settings.UI/SettingsXAML/Controls/Dashboard/ShortcutConflictWindow.xaml.cs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using System;
65
using CommunityToolkit.WinUI.Controls;
76
using Microsoft.PowerToys.Settings.UI.Helpers;
87
using Microsoft.PowerToys.Settings.UI.Library;
9-
using Microsoft.PowerToys.Settings.UI.Library.Helpers;
108
using Microsoft.PowerToys.Settings.UI.Library.HotkeyConflicts;
119
using Microsoft.PowerToys.Settings.UI.Services;
1210
using Microsoft.PowerToys.Settings.UI.ViewModels;
1311
using Microsoft.PowerToys.Settings.UI.Views;
14-
using Microsoft.UI;
1512
using Microsoft.UI.Windowing;
1613
using Microsoft.UI.Xaml;
1714
using Microsoft.UI.Xaml.Controls;
@@ -26,7 +23,11 @@ public sealed partial class ShortcutConflictWindow : WindowEx
2623

2724
public ShortcutConflictWindow()
2825
{
26+
App.ThemeService.ThemeChanged += OnThemeChanged;
27+
App.ThemeService.ApplyTheme();
28+
2929
var settingsUtils = SettingsUtils.Default;
30+
3031
ViewModel = new ShortcutConflictViewModel(
3132
settingsUtils,
3233
SettingsRepository<GeneralSettings>.GetInstance(settingsUtils),
@@ -50,6 +51,11 @@ public ShortcutConflictWindow()
5051
ViewModel.OnPageLoaded();
5152
}
5253

54+
private void OnThemeChanged(object sender, ElementTheme theme)
55+
{
56+
WindowHelper.SetTheme(this, theme);
57+
}
58+
5359
private void CenterOnScreen()
5460
{
5561
var displayArea = DisplayArea.GetFromWindowId(this.AppWindow.Id, DisplayAreaFallback.Nearest);
@@ -127,6 +133,14 @@ private void UnignoreConflictGroup(HotkeyConflictGroupData conflictGroup)
127133
private void WindowEx_Closed(object sender, WindowEventArgs args)
128134
{
129135
ViewModel?.Dispose();
136+
137+
var mainWindow = App.GetSettingsWindow();
138+
if (mainWindow != null)
139+
{
140+
mainWindow.CloseHiddenWindow();
141+
}
142+
143+
App.ThemeService.ThemeChanged -= OnThemeChanged;
130144
}
131145

132146
private void Window_Activated_SetIcon(object sender, WindowActivatedEventArgs args)

src/settings-ui/Settings.UI/SettingsXAML/Controls/ShortcutControl/ShortcutControl.xaml.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using CommunityToolkit.WinUI;
1010
using Microsoft.PowerToys.Settings.UI.Helpers;
1111
using Microsoft.PowerToys.Settings.UI.Library;
12-
using Microsoft.PowerToys.Settings.UI.Library.HotkeyConflicts;
1312
using Microsoft.PowerToys.Settings.UI.Library.Telemetry.Events;
1413
using Microsoft.PowerToys.Settings.UI.Services;
1514
using Microsoft.PowerToys.Settings.UI.SettingsXAML.Controls.Dashboard;
@@ -300,9 +299,7 @@ private void C_LearnMoreClick(object sender, RoutedEventArgs e)
300299
// Close the current shortcut dialog
301300
shortcutDialog.Hide();
302301

303-
// Create and show the ShortcutConflictWindow
304-
var conflictWindow = new ShortcutConflictWindow();
305-
conflictWindow.Activate();
302+
((App)App.Current)!.OpenShortcutConflictWindow();
306303
}
307304

308305
private void UpdateKeyVisualStyles()

src/settings-ui/Settings.UI/SettingsXAML/MainWindow.xaml.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ private void Window_Closed(object sender, WindowEventArgs args)
162162
var hWnd = WindowNative.GetWindowHandle(this);
163163
WindowHelper.SerializePlacement(hWnd);
164164

165-
if (!App.IsOobeOrScoobeOpen())
165+
if (!App.IsSecondaryWindowOpen())
166166
{
167167
App.ClearSettingsWindow();
168168
}

src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public DashboardPage()
3939
DataContext = ViewModel;
4040

4141
Loaded += (s, e) => ViewModel.OnPageLoaded();
42+
Unloaded += (s, e) => ViewModel?.Dispose();
4243
}
4344

4445
public void RefreshEnabledState()

src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public partial class DashboardViewModel : PageViewModelBase
5555

5656
// Flag to prevent toggle operations during sorting to avoid race conditions.
5757
private bool _isSorting;
58+
private bool _isDisposed;
5859

5960
private AllHotkeyConflictsData _allHotkeyConflictsData = new AllHotkeyConflictsData();
6061

@@ -132,8 +133,18 @@ public DashboardViewModel(ISettingsRepository<GeneralSettings> settingsRepositor
132133

133134
private void OnSettingsChanged(GeneralSettings newSettings)
134135
{
136+
if (_isDisposed)
137+
{
138+
return;
139+
}
140+
135141
dispatcher.TryEnqueue(() =>
136142
{
143+
if (_isDisposed)
144+
{
145+
return;
146+
}
147+
137148
generalSettingsConfig = newSettings;
138149

139150
// Update local field and notify UI if sort order changed
@@ -149,8 +160,18 @@ private void OnSettingsChanged(GeneralSettings newSettings)
149160

150161
protected override void OnConflictsUpdated(object sender, AllHotkeyConflictsEventArgs e)
151162
{
163+
if (_isDisposed)
164+
{
165+
return;
166+
}
167+
152168
dispatcher.TryEnqueue(() =>
153169
{
170+
if (_isDisposed)
171+
{
172+
return;
173+
}
174+
154175
var allConflictData = e.Conflicts;
155176
foreach (var inAppConflict in allConflictData.InAppConflicts)
156177
{
@@ -363,6 +384,11 @@ private void EnabledChangedOnUI(ModuleListItem item)
363384
/// </summary>
364385
public void ModuleEnabledChangedOnSettingsPage()
365386
{
387+
if (_isDisposed)
388+
{
389+
return;
390+
}
391+
366392
// Ignore if this was triggered by a UI change that we're already handling.
367393
if (_isUpdatingFromUI)
368394
{
@@ -391,6 +417,17 @@ public void ModuleEnabledChangedOnSettingsPage()
391417
/// </summary>
392418
private void RefreshShortcutModules()
393419
{
420+
if (_isDisposed)
421+
{
422+
return;
423+
}
424+
425+
if (!dispatcher.HasThreadAccess)
426+
{
427+
_ = dispatcher.TryEnqueue(DispatcherQueuePriority.Normal, RefreshShortcutModules);
428+
return;
429+
}
430+
394431
ShortcutModules.Clear();
395432
ActionModules.Clear();
396433

@@ -804,6 +841,12 @@ internal void DashboardListItemClick(object sender)
804841

805842
public override void Dispose()
806843
{
844+
if (_isDisposed)
845+
{
846+
return;
847+
}
848+
849+
_isDisposed = true;
807850
base.Dispose();
808851
if (_settingsRepository != null)
809852
{

0 commit comments

Comments
 (0)