[fxcop] Settings UI (#7559)

* Remove redundant default initializations

* Implement IDisposable in HotkeySettingsControl

* Mark classes and methods as static

* Move Interop.ShowWindow to NativeMethods class

* Fix string-related warnings

* Remove unused argument for KeyEventHandler

* Log caught general exceptions

* Use safe navigation operator for null argument checks

* Suppress CA2007 warnings and enable FxCop

* Suppress warning for unused event handler params

* Use TryParse in ImageResizerPage

* Use ConfigureAwait(false) for CA2007
This commit is contained in:
Luthfi Mawarid
2020-10-29 14:24:16 -07:00
committed by GitHub
parent 7ec2ff5513
commit 215c353dee
18 changed files with 120 additions and 65 deletions

View File

@@ -44,21 +44,21 @@ namespace Microsoft.PowerToys.Settings.UI.Runner
if (shellPage != null)
{
// send IPC Message
shellPage.SetDefaultSndMessageCallback(msg =>
ShellPage.SetDefaultSndMessageCallback(msg =>
{
// IPC Manager is null when launching runner directly
Program.GetTwoWayIPCManager()?.Send(msg);
});
// send IPC Message
shellPage.SetRestartAdminSndMessageCallback(msg =>
ShellPage.SetRestartAdminSndMessageCallback(msg =>
{
Program.GetTwoWayIPCManager().Send(msg);
System.Windows.Application.Current.Shutdown(); // close application
});
// send IPC Message
shellPage.SetCheckForUpdatesMessageCallback(msg =>
ShellPage.SetCheckForUpdatesMessageCallback(msg =>
{
Program.GetTwoWayIPCManager().Send(msg);
});
@@ -83,8 +83,8 @@ namespace Microsoft.PowerToys.Settings.UI.Runner
}
};
shellPage.SetElevationStatus(Program.IsElevated);
shellPage.SetIsUserAnAdmin(Program.IsUserAnAdmin);
ShellPage.SetElevationStatus(Program.IsElevated);
ShellPage.SetIsUserAnAdmin(Program.IsUserAnAdmin);
shellPage.Refresh();
}

View File

@@ -2,6 +2,7 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
namespace Microsoft.PowerToys.Settings.UI.Activation
@@ -15,13 +16,13 @@ namespace Microsoft.PowerToys.Settings.UI.Activation
public abstract Task HandleAsync(object args);
}
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1402:File may only contain a single type", Justification = "abstract T and abstract")]
[SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1402:File may only contain a single type", Justification = "abstract T and abstract")]
internal abstract class ActivationHandler<T> : ActivationHandler
where T : class
{
public override async Task HandleAsync(object args)
{
await HandleInternalAsync(args as T);
await HandleInternalAsync(args as T).ConfigureAwait(false);
}
public override bool CanHandle(object args)

View File

@@ -29,7 +29,7 @@ namespace Microsoft.PowerToys.Settings.UI.Activation
}
NavigationService.Navigate(navElement, arguments);
await Task.CompletedTask;
await Task.CompletedTask.ConfigureAwait(false);
}
protected override bool CanHandleInternal(IActivatedEventArgs args)

View File

@@ -2,6 +2,7 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using Microsoft.PowerToys.Settings.UI.Helpers;
using Microsoft.Toolkit.Win32.UI.XamlHost;
namespace Microsoft.PowerToys.Settings.UI
@@ -15,7 +16,7 @@ namespace Microsoft.PowerToys.Settings.UI
// Hide the Xaml Island window
var coreWindow = Windows.UI.Core.CoreWindow.GetForCurrentThread();
var coreWindowInterop = Interop.GetInterop(coreWindow);
Interop.ShowWindow(coreWindowInterop.WindowHandle, Interop.SW_HIDE);
NativeMethods.ShowWindow(coreWindowInterop.WindowHandle, Interop.SW_HIDE);
}
}
}

View File

@@ -30,12 +30,12 @@ namespace Microsoft.PowerToys.Settings.UI.Behaviors
public static NavigationViewHeaderMode GetHeaderMode(Page item)
{
return (NavigationViewHeaderMode)item.GetValue(HeaderModeProperty);
return (NavigationViewHeaderMode)item?.GetValue(HeaderModeProperty);
}
public static void SetHeaderMode(Page item, NavigationViewHeaderMode value)
{
item.SetValue(HeaderModeProperty, value);
item?.SetValue(HeaderModeProperty, value);
}
public static readonly DependencyProperty HeaderModeProperty =
@@ -43,12 +43,12 @@ namespace Microsoft.PowerToys.Settings.UI.Behaviors
public static object GetHeaderContext(Page item)
{
return item.GetValue(HeaderContextProperty);
return item?.GetValue(HeaderContextProperty);
}
public static void SetHeaderContext(Page item, object value)
{
item.SetValue(HeaderContextProperty, value);
item?.SetValue(HeaderContextProperty, value);
}
public static readonly DependencyProperty HeaderContextProperty =
@@ -56,12 +56,12 @@ namespace Microsoft.PowerToys.Settings.UI.Behaviors
public static DataTemplate GetHeaderTemplate(Page item)
{
return (DataTemplate)item.GetValue(HeaderTemplateProperty);
return (DataTemplate)item?.GetValue(HeaderTemplateProperty);
}
public static void SetHeaderTemplate(Page item, DataTemplate value)
{
item.SetValue(HeaderTemplateProperty, value);
item?.SetValue(HeaderTemplateProperty, value);
}
public static readonly DependencyProperty HeaderTemplateProperty =

View File

@@ -11,13 +11,13 @@ using Windows.UI.Xaml.Controls;
namespace Microsoft.PowerToys.Settings.UI.Controls
{
public sealed partial class HotkeySettingsControl : UserControl
public sealed partial class HotkeySettingsControl : UserControl, IDisposable
{
private readonly UIntPtr ignoreKeyEventFlag = (UIntPtr)0x5555;
private bool _shiftKeyDownOnEntering = false;
private bool _shiftKeyDownOnEntering;
private bool _shiftToggled = false;
private bool _shiftToggled;
public string Header { get; set; }
@@ -30,7 +30,7 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
typeof(HotkeySettingsControl),
null);
private bool _enabled = false;
private bool _enabled;
public bool Enabled
{
@@ -73,6 +73,7 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
private HotkeySettings lastValidSettings;
private HotkeySettingsControlHook hook;
private bool _isActive;
private bool disposedValue;
public HotkeySettings HotkeySettings
{
@@ -109,7 +110,7 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
hook.Dispose();
}
private void KeyEventHandler(int key, bool matchValue, int matchValueCode, string matchValueText)
private void KeyEventHandler(int key, bool matchValue, int matchValueCode)
{
switch ((Windows.System.VirtualKey)key)
{
@@ -229,7 +230,7 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
{
await Dispatcher.RunAsync(CoreDispatcherPriority.Normal, () =>
{
KeyEventHandler(key, true, key, Library.Utilities.Helper.GetKeyName((uint)key));
KeyEventHandler(key, true, key);
// Tab and Shift+Tab are accessible keys and should not be displayed in the hotkey control.
if (internalSettings.Code > 0 && !internalSettings.IsAccessibleShortcut())
@@ -244,7 +245,7 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
{
await Dispatcher.RunAsync(CoreDispatcherPriority.Normal, () =>
{
KeyEventHandler(key, false, 0, string.Empty);
KeyEventHandler(key, false, 0);
});
}
@@ -278,5 +279,28 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
HotkeyTextBox.Text = hotkeySettings.ToString();
_isActive = false;
}
private void Dispose(bool disposing)
{
if (!disposedValue)
{
if (disposing)
{
// TODO: dispose managed state (managed objects)
hook.Dispose();
}
// TODO: free unmanaged resources (unmanaged objects) and override finalizer
// TODO: set large fields to null
disposedValue = true;
}
}
public void Dispose()
{
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
Dispose(disposing: true);
GC.SuppressFinalize(this);
}
}
}

View File

@@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.
using System;
using System.Globalization;
using Microsoft.PowerToys.Settings.UI.Library;
using Microsoft.PowerToys.Settings.UI.Library.Utilities;
using Windows.UI.Xaml;
@@ -22,10 +23,14 @@ namespace Microsoft.PowerToys.Settings.UI.Converters
bool isEnabled = (bool)value;
var defaultTheme = new Windows.UI.ViewManagement.UISettings();
var uiTheme = defaultTheme.GetColorValue(Windows.UI.ViewManagement.UIColorType.Background).ToString();
selectedTheme = SettingsRepository<GeneralSettings>.GetInstance(settingsUtils).SettingsConfig.Theme.ToLower();
if (selectedTheme == "dark" || (selectedTheme == "system" && uiTheme == "#FF000000"))
// Using InvariantCulture as this is an internal string and expected to be in hexadecimal
var uiTheme = defaultTheme.GetColorValue(Windows.UI.ViewManagement.UIColorType.Background).ToString(CultureInfo.InvariantCulture);
// Normalize strings to uppercase according to Fxcop
selectedTheme = SettingsRepository<GeneralSettings>.GetInstance(settingsUtils).SettingsConfig.Theme.ToUpperInvariant();
if (selectedTheme == "DARK" || (selectedTheme == "SYSTEM" && uiTheme == "#FF000000"))
{
// DARK
if (isEnabled)

View File

@@ -13,5 +13,8 @@ namespace Microsoft.PowerToys.Settings.UI.Helpers
[DllImport("user32.dll", CharSet = CharSet.Auto, CallingConvention = CallingConvention.StdCall, SetLastError = true)]
internal static extern short GetAsyncKeyState(int vKey);
[DllImport("user32.dll")]
public static extern bool ShowWindow(System.IntPtr hWnd, int nCmdShow);
}
}

View File

@@ -8,7 +8,7 @@ using Windows.UI.Xaml;
namespace Microsoft.PowerToys.Settings.UI.Helpers
{
public class NavHelper
public static class NavHelper
{
// This helper class allows to specify the page that will be shown when you click on a NavigationViewItem
//
@@ -19,12 +19,12 @@ namespace Microsoft.PowerToys.Settings.UI.Helpers
// NavHelper.SetNavigateTo(navigationViewItem, typeof(MainPage));
public static Type GetNavigateTo(NavigationViewItem item)
{
return (Type)item.GetValue(NavigateToProperty);
return (Type)item?.GetValue(NavigateToProperty);
}
public static void SetNavigateTo(NavigationViewItem item, Type value)
{
item.SetValue(NavigateToProperty, value);
item?.SetValue(NavigateToProperty, value);
}
public static readonly DependencyProperty NavigateToProperty =

View File

@@ -23,9 +23,6 @@ namespace Microsoft.PowerToys.Settings.UI
}
}
[DllImport("user32.dll")]
public static extern bool ShowWindow(System.IntPtr hWnd, int nCmdShow);
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop naming consistancy")]
public const int SW_HIDE = 0;
}

View File

@@ -201,6 +201,11 @@
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers">
<Version>3.3.0</Version>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
</ItemGroup>
<ItemGroup>
<PRIResource Include="Strings\*\Resources.resw" />

View File

@@ -38,7 +38,7 @@ namespace Microsoft.PowerToys.Settings.UI.Services
{
// Initialize services that you need before app activation
// take into account that the splash screen is shown while this code runs.
await InitializeAsync();
await InitializeAsync().ConfigureAwait(false);
// Do not repeat app initialization when the Window already has content,
// just ensure that the window is active
@@ -51,7 +51,7 @@ namespace Microsoft.PowerToys.Settings.UI.Services
// Depending on activationArgs one of ActivationHandlers or DefaultActivationHandler
// will navigate to the first page
await HandleActivationAsync(activationArgs);
await HandleActivationAsync(activationArgs).ConfigureAwait(false);
lastActivationArgs = activationArgs;
if (IsInteractive(activationArgs))
@@ -60,13 +60,13 @@ namespace Microsoft.PowerToys.Settings.UI.Services
Window.Current.Activate();
// Tasks after activation
await StartupAsync();
await StartupAsync().ConfigureAwait(false);
}
}
private async Task InitializeAsync()
private static async Task InitializeAsync()
{
await Task.CompletedTask;
await Task.CompletedTask.ConfigureAwait(false);
}
private async Task HandleActivationAsync(object activationArgs)
@@ -76,7 +76,7 @@ namespace Microsoft.PowerToys.Settings.UI.Services
if (activationHandler != null)
{
await activationHandler.HandleAsync(activationArgs);
await activationHandler.HandleAsync(activationArgs).ConfigureAwait(false);
}
if (IsInteractive(activationArgs))
@@ -84,22 +84,22 @@ namespace Microsoft.PowerToys.Settings.UI.Services
var defaultHandler = new DefaultActivationHandler(defaultNavItem);
if (defaultHandler.CanHandle(activationArgs))
{
await defaultHandler.HandleAsync(activationArgs);
await defaultHandler.HandleAsync(activationArgs).ConfigureAwait(false);
}
}
}
private async Task StartupAsync()
private static async Task StartupAsync()
{
await Task.CompletedTask;
await Task.CompletedTask.ConfigureAwait(false);
}
private IEnumerable<ActivationHandler> GetActivationHandlers()
private static IEnumerable<ActivationHandler> GetActivationHandlers()
{
yield break;
}
private bool IsInteractive(object args)
private static bool IsInteractive(object args)
{
return args is IActivatedEventArgs;
}

View File

@@ -84,7 +84,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
// More info on tracking issue https://github.com/Microsoft/microsoft-ui-xaml/issues/8
keyboardAccelerators.Add(altLeftKeyboardAccelerator);
keyboardAccelerators.Add(backKeyboardAccelerator);
await Task.CompletedTask;
await Task.CompletedTask.ConfigureAwait(false);
}
private void OnItemInvoked(WinUI.NavigationViewItemInvokedEventArgs args)
@@ -114,7 +114,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
.FirstOrDefault(menuItem => IsMenuItemForPageType(menuItem, e.SourcePageType));
}
private bool IsMenuItemForPageType(WinUI.NavigationViewItem menuItem, Type sourcePageType)
private static bool IsMenuItemForPageType(WinUI.NavigationViewItem menuItem, Type sourcePageType)
{
var pageType = menuItem.GetValue(NavHelper.NavigateToProperty) as Type;
return pageType == sourcePageType;

View File

@@ -3,6 +3,8 @@
// See the LICENSE file in the project root for more information.
using System;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using Microsoft.PowerToys.Settings.UI.Library;
using Microsoft.PowerToys.Settings.UI.Library.Utilities;
using Microsoft.PowerToys.Settings.UI.Library.ViewModels;
@@ -27,6 +29,7 @@ namespace Microsoft.PowerToys.Settings.UI.Views
/// Initializes a new instance of the <see cref="GeneralPage"/> class.
/// General Settings page constructor.
/// </summary>
[SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Exceptions from the IPC response handler should be caught and logged.")]
public GeneralPage()
{
InitializeComponent();
@@ -58,28 +61,30 @@ namespace Microsoft.PowerToys.Settings.UI.Views
{
str = ResourceLoader.GetForCurrentView().GetString("GeneralSettings_VersionIsLatest");
}
else if (version != string.Empty)
else if (!string.IsNullOrEmpty(version))
{
str = ResourceLoader.GetForCurrentView().GetString("GeneralSettings_NewVersionIsAvailable");
if (str != string.Empty)
if (!string.IsNullOrEmpty(str))
{
str += ": " + version;
}
}
ViewModel.LatestAvailableVersion = string.Format(str);
// Using CurrentCulture since this is user-facing
ViewModel.LatestAvailableVersion = string.Format(CultureInfo.CurrentCulture, str);
}
catch (Exception)
catch (Exception e)
{
Logger.LogError("Exception encountered when reading the version.", e);
}
});
DataContext = ViewModel;
}
public int UpdateUIThemeMethod(string themeName)
public static int UpdateUIThemeMethod(string themeName)
{
switch (themeName.ToUpperInvariant())
switch (themeName?.ToUpperInvariant())
{
case "LIGHT":
ShellPage.ShellHandler.RequestedTheme = ElementTheme.Light;
@@ -90,6 +95,9 @@ namespace Microsoft.PowerToys.Settings.UI.Views
case "SYSTEM":
ShellPage.ShellHandler.RequestedTheme = ElementTheme.Default;
break;
default:
Logger.LogError($"Unexpected theme name: {themeName}");
break;
}
return 0;

View File

@@ -2,6 +2,9 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using Microsoft.PowerToys.Settings.UI.Library;
using Microsoft.PowerToys.Settings.UI.Library.Utilities;
@@ -25,28 +28,34 @@ namespace Microsoft.PowerToys.Settings.UI.Views
public void DeleteCustomSize(object sender, RoutedEventArgs e)
{
try
Button deleteRowButton = (Button)sender;
// Using InvariantCulture since this is internal and expected to be numerical
bool success = int.TryParse(deleteRowButton?.CommandParameter?.ToString(), NumberStyles.Integer, CultureInfo.InvariantCulture, out int rowNum);
if (success)
{
Button deleteRowButton = (Button)sender;
int rowNum = int.Parse(deleteRowButton.CommandParameter.ToString());
ViewModel.DeleteImageSize(rowNum);
}
catch
else
{
Logger.LogError("Failed to delete custom image size.");
}
}
[SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "JSON exceptions from saving new settings should be caught and logged.")]
private void AddSizeButton_Click(object sender, RoutedEventArgs e)
{
try
{
ViewModel.AddRow();
}
catch
catch (Exception ex)
{
Logger.LogError("Exception encountered when adding a new image size.", ex);
}
}
[SuppressMessage("Usage", "CA1801:Review unused parameters", Justification = "Params are required for event handler signature requirements.")]
private void ImagesSizesListView_ContainerContentChanging(ListViewBase sender, ContainerContentChangingEventArgs args)
{
if (ViewModel.IsListViewFocusRequested)

View File

@@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using Microsoft.PowerToys.Settings.UI.Library;
using Microsoft.PowerToys.Settings.UI.Library.Utilities;
@@ -56,17 +57,18 @@ namespace Microsoft.PowerToys.Settings.UI.Views
}
}
private void CombineRemappings(List<KeysDataModel> remapKeysList, uint leftKey, uint rightKey, uint combinedKey)
private static void CombineRemappings(List<KeysDataModel> remapKeysList, uint leftKey, uint rightKey, uint combinedKey)
{
KeysDataModel firstRemap = remapKeysList.Find(x => uint.Parse(x.OriginalKeys) == leftKey);
KeysDataModel secondRemap = remapKeysList.Find(x => uint.Parse(x.OriginalKeys) == rightKey);
// Using InvariantCulture for keys as they are internally represented as numerical values
KeysDataModel firstRemap = remapKeysList.Find(x => uint.Parse(x.OriginalKeys, CultureInfo.InvariantCulture) == leftKey);
KeysDataModel secondRemap = remapKeysList.Find(x => uint.Parse(x.OriginalKeys, CultureInfo.InvariantCulture) == rightKey);
if (firstRemap != null && secondRemap != null)
{
if (firstRemap.NewRemapKeys == secondRemap.NewRemapKeys)
{
KeysDataModel combinedRemap = new KeysDataModel
{
OriginalKeys = combinedKey.ToString(),
OriginalKeys = combinedKey.ToString(CultureInfo.InvariantCulture),
NewRemapKeys = firstRemap.NewRemapKeys,
};
remapKeysList.Insert(remapKeysList.IndexOf(firstRemap), combinedRemap);

View File

@@ -92,7 +92,7 @@ namespace Microsoft.PowerToys.Settings.UI.Views
/// Set Default IPC Message callback function.
/// </summary>
/// <param name="implementation">delegate function implementation.</param>
public void SetDefaultSndMessageCallback(IPCMessageCallback implementation)
public static void SetDefaultSndMessageCallback(IPCMessageCallback implementation)
{
DefaultSndMSGCallback = implementation;
}
@@ -101,7 +101,7 @@ namespace Microsoft.PowerToys.Settings.UI.Views
/// Set restart as admin IPC callback function.
/// </summary>
/// <param name="implementation">delegate function implementation.</param>
public void SetRestartAdminSndMessageCallback(IPCMessageCallback implementation)
public static void SetRestartAdminSndMessageCallback(IPCMessageCallback implementation)
{
SndRestartAsAdminMsgCallback = implementation;
}
@@ -110,17 +110,17 @@ namespace Microsoft.PowerToys.Settings.UI.Views
/// Set check for updates IPC callback function.
/// </summary>
/// <param name="implementation">delegate function implementation.</param>
public void SetCheckForUpdatesMessageCallback(IPCMessageCallback implementation)
public static void SetCheckForUpdatesMessageCallback(IPCMessageCallback implementation)
{
CheckForUpdatesMsgCallback = implementation;
}
public void SetElevationStatus(bool isElevated)
public static void SetElevationStatus(bool isElevated)
{
IsElevated = isElevated;
}
public void SetIsUserAnAdmin(bool isAdmin)
public static void SetIsUserAnAdmin(bool isAdmin)
{
IsUserAnAdmin = isAdmin;
}

View File

@@ -13,7 +13,7 @@ namespace Microsoft.PowerToys.Settings.UI.Views
{
public object Convert(object value, Type targetType, object parameter, string language)
{
return (value as IList).Count == 0 ? Visibility.Collapsed : Visibility.Visible;
return (value == null) || (value as IList).Count == 0 ? Visibility.Collapsed : Visibility.Visible;
}
public object ConvertBack(object value, Type targetType, object parameter, string language)