Address feedback + cleanup

src/core/Microsoft.PowerToys.Settings.UI.Library/Enumerations/ColorRepresentationType.cs

@@ -4,51 +4,56 @@
 
 namespace Microsoft.PowerToys.Settings.UI.Library.Enumerations
 {
+    // NOTE: don't change the order (numbers) of the enumeration entires
+
+    /// <summary>
+    /// The type of the color representation
+    /// </summary>
     public enum ColorRepresentationType
     {
-        /// <summary>
-        /// Color presentation as CMYK color value (cyan[0%..100%], magenta[0%..100%], yellow[0%..100%], black key[0%..100%])
-        /// </summary>
-        CMYK = 0,
-
         /// <summary>
         /// Color presentation as hexadecimal color value without the alpha-value (e.g. #0055FF)
         /// </summary>
-        HEX = 1,
-
-        /// <summary>
-        /// Color presentation as HSB color value (hue[0°..360°], saturation[0%..100%], brightness[0%..100%])
-        /// </summary>
-        HSB = 2,
-
-        /// <summary>
-        /// Color presentation as HSI color value (hue[0°..360°], saturation[0%..100%], intensity[0%..100%])
-        /// </summary>
-        HSI = 3,
-
-        /// <summary>
-        /// Color presentation as HSL color value (hue[0°..360°], saturation[0..100%], lightness[0%..100%])
-        /// </summary>
-        HSL = 4,
-
-        /// <summary>
-        /// Color presentation as HSV color value (hue[0°..360°], saturation[0%..100%], value[0%..100%])
-        /// </summary>
-        HSV = 5,
-
-        /// <summary>
-        /// Color presentation as HWB color value (hue[0°..360°], whiteness[0%..100%], blackness[0%..100%])
-        /// </summary>
-        HWB = 6,
-
-        /// <summary>
-        /// Color presentation as natural color (hue, whiteness[0%..100%], blackness[0%..100%])
-        /// </summary>
-        NCol = 7,
+        HEX = 0,
 
         /// <summary>
         /// Color presentation as RGB color value (red[0..255], green[0..255], blue[0..255])
         /// </summary>
-        RGB = 8,
+        RGB = 1,
+
+        /// <summary>
+        /// Color presentation as CMYK color value (cyan[0%..100%], magenta[0%..100%], yellow[0%..100%], black key[0%..100%])
+        /// </summary>
+        CMYK = 2,
+
+        /// <summary>
+        /// Color presentation as HSL color value (hue[0°..360°], saturation[0..100%], lightness[0%..100%])
+        /// </summary>
+        HSL = 3,
+
+        /// <summary>
+        /// Color presentation as HSV color value (hue[0°..360°], saturation[0%..100%], value[0%..100%])
+        /// </summary>
+        HSV = 4,
+
+        /// <summary>
+        /// Color presentation as HSB color value (hue[0°..360°], saturation[0%..100%], brightness[0%..100%])
+        /// </summary>
+        HSB = 5,
+
+        /// <summary>
+        /// Color presentation as HSI color value (hue[0°..360°], saturation[0%..100%], intensity[0%..100%])
+        /// </summary>
+        HSI = 6,
+
+        /// <summary>
+        /// Color presentation as HWB color value (hue[0°..360°], whiteness[0%..100%], blackness[0%..100%])
+        /// </summary>
+        HWB = 7,
+
+        /// <summary>
+        /// Color presentation as natural color (hue, whiteness[0%..100%], blackness[0%..100%])
+        /// </summary>
+        NCol = 8,
     }
 }

src/core/Microsoft.PowerToys.Settings.UI.Library/ViewModels/ColorPickerViewModel.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 using System.Text.Json;
 using Microsoft.PowerToys.Settings.UI.Library.Enumerations;
@@ -17,7 +18,7 @@ namespace Microsoft.PowerToys.Settings.UI.Library.ViewModels
 
         private readonly ISettingsUtils _settingsUtils;
 
-        private ColorPickerSettings _colorPickerSettings;
+        private readonly ColorPickerSettings _colorPickerSettings;
 
         private bool _isEnabled;
 
@@ -31,6 +32,19 @@ namespace Microsoft.PowerToys.Settings.UI.Library.ViewModels
                 throw new ArgumentNullException(nameof(settingsRepository));
             }
 
+            SelectableColorRepresentations = new Dictionary<ColorRepresentationType, string>
+            {
+                { ColorRepresentationType.CMYK, "CMYK - cmyk(100%, 50%, 75%, 0%)" },
+                { ColorRepresentationType.HEX,  "HEX - #FFAA00" },
+                { ColorRepresentationType.HSB,  "HSB - hsb(100, 50%, 75%)" },
+                { ColorRepresentationType.HSI,  "HSI - hsi(100, 50%, 75%)" },
+                { ColorRepresentationType.HSL,  "HSL - hsl(100, 50%, 75%)" },
+                { ColorRepresentationType.HSV,  "HSV - hsv(100, 50%, 75%)" },
+                { ColorRepresentationType.HWB,  "HWB - hwb(100, 50%, 75%)" },
+                { ColorRepresentationType.NCol, "NCol - R10, 50%, 75%" },
+                { ColorRepresentationType.RGB,  "RGB - rgb(100, 50, 75)" },
+            };
+
             GeneralSettingsConfig = settingsRepository.SettingsConfig;
 
             _settingsUtils = settingsUtils ?? throw new ArgumentNullException(nameof(settingsUtils));
@@ -49,80 +63,77 @@ namespace Microsoft.PowerToys.Settings.UI.Library.ViewModels
             SendConfigMSG = ipcMSGCallBackFunc;
         }
 
+        /// <summary>
+        /// Gets a list with all selectable <see cref="ColorRepresentationType"/>s
+        /// </summary>
+        public IReadOnlyDictionary<ColorRepresentationType, string> SelectableColorRepresentations { get; }
+
         public bool IsEnabled
         {
-            get
-            {
-                return _isEnabled;
-            }
-
+            get => _isEnabled;
             set
             {
-                if (_isEnabled != value)
+                if (_isEnabled == value)
                 {
-                    _isEnabled = value;
-                    OnPropertyChanged(nameof(IsEnabled));
-
-                    // Set the status of ColorPicker in the general settings
-                    GeneralSettingsConfig.Enabled.ColorPicker = value;
-                    OutGoingGeneralSettings outgoing = new OutGoingGeneralSettings(GeneralSettingsConfig);
-
-                    SendConfigMSG(outgoing.ToString());
+                    return;
                 }
+
+                _isEnabled = value;
+                OnPropertyChanged(nameof(IsEnabled));
+
+                // Set the status of ColorPicker in the general settings
+                GeneralSettingsConfig.Enabled.ColorPicker = value;
+                var outgoing = new OutGoingGeneralSettings(GeneralSettingsConfig);
+
+                SendConfigMSG(outgoing.ToString());
             }
         }
 
         public bool ChangeCursor
         {
-            get
-            {
-                return _colorPickerSettings.Properties.ChangeCursor;
-            }
-
+            get => _colorPickerSettings.Properties.ChangeCursor;
             set
             {
-                if (_colorPickerSettings.Properties.ChangeCursor != value)
+                if (_colorPickerSettings.Properties.ChangeCursor == value)
                 {
-                    _colorPickerSettings.Properties.ChangeCursor = value;
-                    OnPropertyChanged(nameof(ChangeCursor));
-                    NotifySettingsChanged();
+                    return;
                 }
+
+                _colorPickerSettings.Properties.ChangeCursor = value;
+                OnPropertyChanged(nameof(ChangeCursor));
+                NotifySettingsChanged();
             }
         }
 
         public HotkeySettings ActivationShortcut
         {
-            get
-            {
-                return _colorPickerSettings.Properties.ActivationShortcut;
-            }
-
+            get => _colorPickerSettings.Properties.ActivationShortcut;
             set
             {
-                if (_colorPickerSettings.Properties.ActivationShortcut != value)
+                if (_colorPickerSettings.Properties.ActivationShortcut == value)
                 {
-                    _colorPickerSettings.Properties.ActivationShortcut = value;
-                    OnPropertyChanged(nameof(ActivationShortcut));
-                    NotifySettingsChanged();
+                    return;
                 }
+
+                _colorPickerSettings.Properties.ActivationShortcut = value;
+                OnPropertyChanged(nameof(ActivationShortcut));
+                NotifySettingsChanged();
             }
         }
 
-        public int CopiedColorRepresentationIndex
+        public ColorRepresentationType SelectedColorRepresentationValue
         {
-            get
-            {
-                return (int)_colorPickerSettings.Properties.CopiedColorRepresentation;
-            }
-
+            get => _colorPickerSettings.Properties.CopiedColorRepresentation;
             set
             {
-                if (_colorPickerSettings.Properties.CopiedColorRepresentation != (ColorRepresentationType)value)
+                if (_colorPickerSettings.Properties.CopiedColorRepresentation == value)
                 {
-                    _colorPickerSettings.Properties.CopiedColorRepresentation = (ColorRepresentationType)value;
-                    OnPropertyChanged(nameof(CopiedColorRepresentationIndex));
-                    NotifySettingsChanged();
+                    return;
                 }
+
+                _colorPickerSettings.Properties.CopiedColorRepresentation = value;
+                OnPropertyChanged(nameof(SelectedColorRepresentationValue));
+                NotifySettingsChanged();
             }
         }
 

src/core/Microsoft.PowerToys.Settings.UI/Views/ColorPickerPage.xaml

@@ -13,6 +13,95 @@
     AutomationProperties.LandmarkType="Main">
 
     <Grid RowSpacing="{StaticResource DefaultRowSpacing}">
+        <Grid.ColumnDefinitions>
+            <ColumnDefinition Width="*" />
+            <ColumnDefinition Width="Auto" />
+        </Grid.ColumnDefinitions>
+        <Grid.RowDefinitions>
+            <RowDefinition Height="Auto" />
+            <RowDefinition Height="Auto" />
+        </Grid.RowDefinitions>
+
+        <StackPanel x:Name="ColorPickerView" Orientation="Vertical">
+            <ToggleSwitch x:Uid="ColorPicker_EnableColorPicker" IsOn="{Binding IsEnabled, Mode=TwoWay}" />
+
+            <Custom:HotkeySettingsControl x:Uid="ColorPicker_ActivationShortcut"
+                                          MinWidth="240"
+                                          Margin="{StaticResource MediumTopMargin}"
+                                          HorizontalAlignment="Left"
+                                          Enabled="{x:Bind Mode=OneWay, Path=ViewModel.IsEnabled}"
+                                          HotkeySettings="{x:Bind Path=ViewModel.ActivationShortcut, Mode=TwoWay}"
+                                          Keys="Win, Ctrl, Alt, Shift" />
+
+            <TextBlock x:Uid="ShortcutGuide_Appearance_Behavior"
+                       Foreground="{x:Bind Mode=OneWay, Path=ViewModel.IsEnabled, Converter={StaticResource ModuleEnabledToForegroundConverter}}"
+                       Style="{StaticResource SettingsGroupTitleStyle}" />
+
+            <ComboBox x:Name="ColorPicker_ComboBox"
+                      x:Uid="ColorPicker_CopiedColorRepresentation"
+                      MinWidth="240"
+                      Margin="{StaticResource SmallTopMargin}"
+                      HorizontalAlignment="Left"
+                      DisplayMemberPath="Value"
+                      IsEnabled="{Binding IsEnabled}"
+                      ItemsSource="{Binding SelectableColorRepresentations}"
+                      Loaded="ColorPicker_ComboBox_Loaded"
+                      SelectedValue="{Binding SelectedColorRepresentationValue, Mode=TwoWay}"
+                      SelectedValuePath="Key" />
+
+            <!--
+            Disabling this until we have a safer way to reset cursor as
+            we can hit a state where the cursor doesn't reset
+
+            <CheckBox x:Uid="ColorPicker_ChangeCursor"
+                          IsChecked="{Binding ChangeCursor, Mode=TwoWay}"
+                          Margin="{StaticResource SmallTopMargin}"
+                          IsEnabled="{Binding IsEnabled}"/>
+            -->
+        </StackPanel>
+
+        <RelativePanel x:Name="SidePanel"
+                       Grid.Column="1"
+                       Width="{StaticResource SidePanelWidth}"
+                       HorizontalAlignment="Left">
+            <StackPanel x:Name="DescriptionPanel">
+                <TextBlock x:Name="AboutTitle"
+                           x:Uid="About_ColorPicker"
+                           Grid.ColumnSpan="2"
+                           Margin="{StaticResource XSmallBottomMargin}"
+                           Style="{StaticResource SettingsGroupTitleStyle}" />
+                <TextBlock x:Uid="ColorPicker_Description"
+                           Grid.Row="1"
+                           TextWrapping="Wrap" />
+            </StackPanel>
+
+            <Border x:Name="AboutImage"
+                    Grid.Row="2"
+                    MaxWidth="240"
+                    Margin="{StaticResource SmallTopBottomMargin}"
+                    HorizontalAlignment="Left"
+                    CornerRadius="4"
+                    RelativePanel.Below="DescriptionPanel">
+                <Image x:Uid="ColorPicker_Image" Source="ms-appx:///Assets/Modules/ColorPicker.png" />
+            </Border>
+            <StackPanel x:Name="LinksPanel"
+                        Margin="0,1,0,0"
+                        Orientation="Vertical"
+                        RelativePanel.Below="AboutImage">
+                <HyperlinkButton NavigateUri="https://aka.ms/PowerToysOverview_ColorPicker">
+                    <TextBlock x:Uid="Module_overview" />
+                </HyperlinkButton>
+                <HyperlinkButton NavigateUri="https://aka.ms/powerToysGiveFeedback">
+                    <TextBlock x:Uid="Give_Feedback" />
+                </HyperlinkButton>
+
+                <TextBlock x:Uid="AttributionTitle" Style="{StaticResource SettingsGroupTitleStyle}" />
+
+                <HyperlinkButton Margin="0,-3,0,0" NavigateUri="https://github.com/martinchrzan/ColorPicker/">
+                    <TextBlock Text="Martin Chrzan's Color Picker" TextWrapping="Wrap" />
+                </HyperlinkButton>
+            </StackPanel>
+        </RelativePanel>
         <VisualStateManager.VisualStateGroups>
             <VisualStateGroup x:Name="LayoutVisualStates">
                 <VisualState x:Name="WideLayout">
@@ -26,113 +115,16 @@
                         <AdaptiveTrigger MinWindowWidth="0" />
                     </VisualState.StateTriggers>
                     <VisualState.Setters>
-                        <Setter Target="SidePanel.(Grid.Column)" Value="0"/>
-                        <Setter Target="SidePanel.Width" Value="Auto"/>
+                        <Setter Target="SidePanel.(Grid.Column)" Value="0" />
+                        <Setter Target="SidePanel.Width" Value="Auto" />
                         <Setter Target="ColorPickerView.(Grid.Row)" Value="1" />
-                        <Setter Target="LinksPanel.(RelativePanel.RightOf)" Value="AboutImage"/>
-                        <Setter Target="LinksPanel.(RelativePanel.AlignTopWith)" Value="AboutImage"/>
-                        <Setter Target="AboutImage.Margin" Value="0,12,12,0"/>
+                        <Setter Target="LinksPanel.(RelativePanel.RightOf)" Value="AboutImage" />
+                        <Setter Target="LinksPanel.(RelativePanel.AlignTopWith)" Value="AboutImage" />
+                        <Setter Target="AboutImage.Margin" Value="0,12,12,0" />
                         <Setter Target="AboutTitle.Visibility" Value="Collapsed" />
                     </VisualState.Setters>
                 </VisualState>
             </VisualStateGroup>
         </VisualStateManager.VisualStateGroups>
-        <Grid.ColumnDefinitions>
-            <ColumnDefinition Width="*"/>
-            <ColumnDefinition Width="Auto"/>
-        </Grid.ColumnDefinitions>
-        <Grid.RowDefinitions>
-            <RowDefinition Height="Auto"/>
-            <RowDefinition Height="Auto" />
-        </Grid.RowDefinitions>
-
-        <StackPanel Orientation="Vertical" x:Name="ColorPickerView">
-            <ToggleSwitch x:Uid="ColorPicker_EnableColorPicker" 
-                          IsOn="{Binding IsEnabled, Mode=TwoWay}"/>
-
-            <Custom:HotkeySettingsControl x:Uid="ColorPicker_ActivationShortcut"
-                                          Margin="{StaticResource MediumTopMargin}"
-                                          HotkeySettings="{x:Bind Path=ViewModel.ActivationShortcut, Mode=TwoWay}"
-                                          Keys="Win, Ctrl, Alt, Shift"
-                                          Enabled="{x:Bind Mode=OneWay, Path=ViewModel.IsEnabled}"
-                                          HorizontalAlignment="Left"
-                                          MinWidth="240"
-                                          />
-
-            <TextBlock x:Uid="ShortcutGuide_Appearance_Behavior"
-                       Style="{StaticResource SettingsGroupTitleStyle}"
-                       Foreground="{x:Bind Mode=OneWay, Path=ViewModel.IsEnabled, Converter={StaticResource ModuleEnabledToForegroundConverter}}"/>
-        
-            <ComboBox x:Uid="ColorPicker_CopiedColorRepresentation"
-                      SelectedIndex="{Binding CopiedColorRepresentationIndex, Mode=TwoWay}"
-                      Margin="{StaticResource SmallTopMargin}"
-                      IsEnabled="{Binding IsEnabled}"
-                      HorizontalAlignment="Left"
-                      MinWidth="240">
-                <ComboBoxItem Content="CMYK - cmyk(100%, 50%, 75%, 0%)"/>
-                <ComboBoxItem Content="HEX - #FFAA00"/>
-                <ComboBoxItem Content="HSB - hsb(100, 50%, 75%)"/>
-                <ComboBoxItem Content="HSI - hsi(100, 50%, 75%)"/>
-                <ComboBoxItem Content="HSL - hsl(100, 50%, 75%)"/>
-                <ComboBoxItem Content="HSV - hsv(100, 50%, 75%)"/>
-                <ComboBoxItem Content="HWB - hwb(100, 50%, 75%)"/>
-                <ComboBoxItem Content="NCol - R10, 50%, 75%"/>
-                <ComboBoxItem Content="RGB - rgb(100, 50, 75)"/>
-            </ComboBox>
-
-               <!--
-            Disabling this until we have a safer way to reset cursor as 
-            we can hit a state where the cursor doesn't reset
-           
-            <CheckBox x:Uid="ColorPicker_ChangeCursor" 
-                          IsChecked="{Binding ChangeCursor, Mode=TwoWay}"
-                          Margin="{StaticResource SmallTopMargin}"
-                          IsEnabled="{Binding IsEnabled}"/>
-                          -->
-        </StackPanel>
-
-        <RelativePanel x:Name="SidePanel" 
-                    HorizontalAlignment="Left"
-                    Width="{StaticResource SidePanelWidth}"
-                    Grid.Column="1">
-            <StackPanel x:Name="DescriptionPanel">
-                <TextBlock x:Uid="About_ColorPicker" x:Name="AboutTitle" Grid.ColumnSpan="2"
-                       Style="{StaticResource SettingsGroupTitleStyle}"
-                       Margin="{StaticResource XSmallBottomMargin}"/>
-                <TextBlock x:Uid="ColorPicker_Description"
-                           TextWrapping="Wrap"
-                           Grid.Row="1" />
-            </StackPanel>
-
-            <Border x:Name="AboutImage"
-                    CornerRadius="4"
-                    Grid.Row="2"
-                    MaxWidth="240"
-                    HorizontalAlignment="Left"
-                    Margin="{StaticResource SmallTopBottomMargin}"
-                    RelativePanel.Below="DescriptionPanel">
-                <Image x:Uid="ColorPicker_Image" Source="ms-appx:///Assets/Modules/ColorPicker.png" />
-            </Border>
-            <StackPanel x:Name="LinksPanel"
-                        Margin="0,1,0,0"
-                        RelativePanel.Below="AboutImage"
-                        Orientation="Vertical" >
-                <HyperlinkButton NavigateUri="https://aka.ms/PowerToysOverview_ColorPicker">
-                    <TextBlock x:Uid="Module_overview" />
-                </HyperlinkButton>
-                <HyperlinkButton NavigateUri="https://aka.ms/powerToysGiveFeedback">
-                    <TextBlock x:Uid="Give_Feedback" />
-                </HyperlinkButton>
-
-                <TextBlock 
-                x:Uid="AttributionTitle"
-                Style="{StaticResource SettingsGroupTitleStyle}" />
-
-                <HyperlinkButton Margin="0,-3,0,0"
-                                NavigateUri="https://github.com/martinchrzan/ColorPicker/">
-                    <TextBlock Text="Martin Chrzan's Color Picker" TextWrapping="Wrap" />
-                </HyperlinkButton>
-            </StackPanel>
-        </RelativePanel>
     </Grid>
 </Page>

src/core/Microsoft.PowerToys.Settings.UI/Views/ColorPickerPage.xaml.cs

@@ -21,5 +21,33 @@ namespace Microsoft.PowerToys.Settings.UI.Views
             DataContext = ViewModel;
             InitializeComponent();
         }
+
+        /// <summary>
+        /// Event is called when the <see cref="ComboBox"/> is completly loaded, inclusive the ItemSource
+        /// </summary>
+        /// <param name="sender">The sender of this event</param>
+        /// <param name="e">The arguments of this event</param>
+        private void ColorPicker_ComboBox_Loaded(object sender, Windows.UI.Xaml.RoutedEventArgs e)
+        {
+           /**
+            * UWP hack
+            * because UWP load the binded ItemSource of the ComboBox asyncronous,
+            * so after InitializeComponent() the ItemSource is still empty and can't automatically select a entry.
+            * Selection via SelectedItem and SelectedValue is still not working too
+            */
+            var index = 0;
+
+            foreach (var item in ViewModel.SelectableColorRepresentations)
+            {
+                if (item.Key == ViewModel.SelectedColorRepresentationValue)
+                {
+                    break;
+                }
+
+                index++;
+            }
+
+            ColorPicker_ComboBox.SelectedIndex = index;
+        }
     }
 }