Feature: Redesign password dialogs#3231
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request redesigns and migrates password-related dialogs from Metro dialogs and UserControls to child windows, improving usability and user experience across the application.
Key Changes
- Introduced a new
DialogHelperutility class that centralizes the creation of OK and OK/Cancel dialogs, reducing code duplication - Migrated three credential dialogs (
CredentialsPasswordDialog,CredentialsSetPasswordDialog,CredentialsChangePasswordDialog) from UserControls to ChildWindows - Refactored multiple ViewModels to use the new
DialogHelperinstead of manual dialog creation - Updated parameter ordering in
OKMessageViewModelandOKCancelMessageViewModelfor consistency
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Website/docs/changelog/next-release.md | Added changelog entries documenting the credential dialog migration and DialogHelper introduction |
| Source/NETworkManager/DialogHelper.cs | New utility class providing ShowOKMessageAsync and ShowOKCancelMessageAsync helper methods with comprehensive documentation |
| Source/NETworkManager/Views/ProfileFileChildWindow.xaml | Updated command binding from AcceptCommand to OKCommand for consistency |
| Source/NETworkManager/Views/OKMessageChildWindow.xaml | Added CloseButtonCommand binding to OK command |
| Source/NETworkManager/Views/OKCancelMessageChildWindow.xaml | Renamed from OKCancelInfoMessageChildWindow, added CloseButtonCommand binding to Cancel command |
| Source/NETworkManager/Views/OKCancelMessageChildWindow.xaml.cs | Renamed class from OKCancelInfoMessageChildWindow to OKCancelMessageChildWindow |
| Source/NETworkManager/Views/Credentials*ChildWindow.xaml | Migrated three credential dialogs from UserControl to ChildWindow with updated margins and styling |
| Source/NETworkManager/Views/Credentials*ChildWindow.xaml.cs | Replaced credential dialog code-behind files with new versions using DispatcherPriority.ContextIdle for focus management |
| Source/NETworkManager/ViewModels/OKMessageViewModel.cs | Reordered constructor parameters: icon moved before okButtonText for consistency |
| Source/NETworkManager/ViewModels/OKCancelMessageViewModel.cs | Renamed from OKCancelInfoMessageViewModel and reordered constructor parameters |
| Source/NETworkManager/ViewModels/ProfileFileViewModel.cs | Renamed AcceptCommand to OKCommand for consistency |
| Source/NETworkManager/ViewModels/SettingsProfilesViewModel.cs | Refactored to use DialogHelper and migrated credential dialogs to child windows |
| Source/NETworkManager/ViewModels/WebConsoleSettingsViewModel.cs | Refactored DeleteBrowsingData to use DialogHelper |
| Source/NETworkManager/ViewModels/SettingsSettingsViewModel.cs | Refactored ResetSettings to use DialogHelper |
| Source/NETworkManager/ViewModels/*SettingsViewModel.cs | Multiple settings ViewModels refactored to use DialogHelper for delete confirmations |
| Source/NETworkManager/ViewModels/HostsFileEditorViewModel.cs | Refactored to use DialogHelper with corrected parameter ordering |
| Source/NETworkManager/ProfileDialogManager.cs | Refactored profile and group deletion dialogs to use DialogHelper |
| Source/NETworkManager/NETworkManager.csproj | Updated XAML page reference from OKCancelInfoMessageChildWindow to OKCancelMessageChildWindow |
| Source/NETworkManager.Utilities/SecureStringHelper.cs | Added blank line for code readability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (4)
Source/NETworkManager/Views/CredentialsSetPasswordChildWindow.xaml:14
- Trailing whitespace detected at the end of these lines. This should be removed to maintain code cleanliness.
Source/NETworkManager/Views/CredentialsChangePasswordChildWindow.xaml:14 - Trailing whitespace detected at the end of these lines. This should be removed to maintain code cleanliness.
Source/NETworkManager/Views/OKCancelMessageChildWindow.xaml:13 - Trailing whitespace detected at the end of this line. This should be removed to maintain code cleanliness.
Source/NETworkManager/Views/CredentialsPasswordChildWindow.xaml:14 - Trailing whitespace detected at the end of these lines. This should be removed to maintain code cleanliness.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| ConfigurationManager.Current.IsChildWindowOpen = true; | ||
|
|
There was a problem hiding this comment.
Unnecessary blank line. There are two consecutive blank lines here where only one is needed.
Source/NETworkManager/ViewModels/PortScannerSettingsViewModel.cs
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| childWindow.Title = Strings.DeleteEntry; | ||
|
|
There was a problem hiding this comment.
Unnecessary blank line. There are two consecutive blank lines here where only one is needed.
|
|
||
| - Documentation updated | ||
| - Code cleanup & refactoring | ||
| - Code cleanup & refactoring |
There was a problem hiding this comment.
Trailing whitespace detected at the end of this line. This should be removed to maintain code cleanliness.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Changes proposed in this pull request
To-Do
Contributing
By submitting this pull request, I confirm the following: