Merge pull request #4134 from robloo/color-picker-fixes

Color picker fixes
2021-08-30 14:12:04 -07:00 · 2021-08-30 14:12:04 -07:00 · aecbcad096
--- a/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ColorPicker/ColorPickerButtonXaml.bind
+++ b/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ColorPicker/ColorPickerButtonXaml.bind
@ -10,19 +10,11 @@
    <SolidColorBrush Color="{ThemeResource SystemChromeLowColor}" x:Key="SystemControlForegroundChromeLowBrush"/>
  </Page.Resources>

-  <Grid>
-    <Grid.RowDefinitions>
-      <RowDefinition Height="*" />
-      <RowDefinition Height="*" />
-      <RowDefinition Height="*" />
-      <RowDefinition Height="*" />
-    </Grid.RowDefinitions>
-    <!-- Example 1 -->
-    <StackPanel Grid.Row="0"
-                Orientation="Vertical"
+  <ScrollViewer>
+    <StackPanel Orientation="Vertical"
                HorizontalAlignment="Center"
-                VerticalAlignment="Center"
                Spacing="20">
+      <!-- Example 1 -->
      <Border Background="{ThemeResource SystemChromeMediumColor}"
              CornerRadius="4"
              Height="100"
@ -44,13 +36,7 @@
          </Style>
        </controls:ColorPickerButton.ColorPickerStyle>
      </controls:ColorPickerButton>
-    </StackPanel>
-    <!-- Example 2 -->
-    <StackPanel Grid.Row="1"
-                Orientation="Vertical"
-                HorizontalAlignment="Center"
-                VerticalAlignment="Center"
-                Spacing="20">
+      <!-- Example 2 -->
      <Border Background="{ThemeResource SystemChromeMediumColor}"
              CornerRadius="4"
              Height="100"
@ -72,13 +58,7 @@
          </Style>
        </controls:ColorPickerButton.ColorPickerStyle>
      </controls:ColorPickerButton>
-    </StackPanel>
-    <!-- Example 3 -->
-    <StackPanel Grid.Row="2"
-                Orientation="Vertical"
-                HorizontalAlignment="Center"
-                VerticalAlignment="Center"
-                Spacing="20">
+      <!-- Example 3 -->
      <Border Background="{ThemeResource SystemChromeMediumColor}"
              CornerRadius="4"
              Height="100"
@ -100,13 +80,7 @@
          </Style>
        </controls:ColorPickerButton.ColorPickerStyle>
      </controls:ColorPickerButton>
-    </StackPanel>
-    <!-- Example 4 -->
-    <StackPanel Grid.Row="3"
-                Orientation="Vertical"
-                HorizontalAlignment="Center"
-                VerticalAlignment="Center"
-                Spacing="20">
+      <!-- Example 4 -->
      <Border Background="{ThemeResource SystemChromeMediumColor}"
              CornerRadius="4"
              Height="100"
@ -130,6 +104,32 @@
          </Style>
        </controls:ColorPickerButton.ColorPickerStyle>
      </controls:ColorPickerButton>
+      <!-- Example 5 -->
+      <Border Background="{ThemeResource SystemChromeMediumColor}"
+              CornerRadius="4"
+              Height="100"
+              Width="300"
+              Padding="10">
+        <TextBlock TextAlignment="Center"
+                   VerticalAlignment="Center">
+          Ring-shaped spectrum <LineBreak />
+          Alpha channel enabled <LineBreak />
+          Only Color Palette Shown
+        </TextBlock>
+      </Border>
+      <controls:ColorPickerButton x:Name="ColorPickerButton5"
+                                  SelectedColor="Teal">
+        <controls:ColorPickerButton.ColorPickerStyle>
+          <Style TargetType="controls:ColorPicker">
+            <Setter Property="ColorSpectrumShape" Value="Ring"/>
+            <Setter Property="IsAlphaEnabled" Value="True"/>
+            <Setter Property="IsHexInputVisible" Value="True"/>
+            <Setter Property="IsColorSpectrumVisible" Value="False"/>
+            <Setter Property="IsColorPaletteVisible" Value="True"/>
+            <Setter Property="IsColorChannelTextInputVisible" Value="False"/>
+          </Style>
+        </controls:ColorPickerButton.ColorPickerStyle>
+      </controls:ColorPickerButton>
    </StackPanel>
-  </Grid>
+  </ScrollViewer>
 </Page>
--- a/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ColorPicker/ColorPickerXaml.bind
+++ b/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ColorPicker/ColorPickerXaml.bind
@ -60,7 +60,7 @@
          Alpha channel enabled
        </TextBlock>
      </Border>
-      <controls:ColorPicker x:Name="ColorPickerButton3"
+      <controls:ColorPicker x:Name="ColorPicker3"
                            Color="Transparent"
                            ColorSpectrumShape="Ring"
                            IsAlphaEnabled="True"
@ -78,12 +78,33 @@
          Saturation+Value spectrum channels
        </TextBlock>
      </Border>
-      <controls:ColorPicker x:Name="ColorPickerButton4"
+      <controls:ColorPicker x:Name="ColorPicker4"
                            Color="Maroon"
                            ColorSpectrumShape="Ring"
                            ColorSpectrumComponents="SaturationValue"
                            IsAlphaEnabled="True"
                            IsHexInputVisible="True"/>
+      <!-- Example 5 -->
+      <Border Background="{ThemeResource SystemChromeMediumColor}"
+              CornerRadius="4"
+              Height="100"
+              Width="300"
+              Padding="10">
+        <TextBlock TextAlignment="Center"
+                   VerticalAlignment="Center">
+          Ring-shaped spectrum <LineBreak />
+          Alpha channel enabled <LineBreak />
+          Only Color Palette Shown
+        </TextBlock>
+      </Border>
+      <controls:ColorPicker x:Name="ColorPicker5"
+                            Color="Teal"
+                            ColorSpectrumShape="Ring"
+                            IsAlphaEnabled="True"
+                            IsHexInputVisible="True"
+                            IsColorSpectrumVisible="False"
+                            IsColorPaletteVisible="True"
+                            IsColorChannelTextInputVisible="False"/>
    </StackPanel>
  </ScrollViewer>
 </Page>
--- a/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.Properties.cs
+++ b/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.Properties.cs
@ -20,7 +20,9 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
                nameof(CustomPaletteColors),
                typeof(ObservableCollection<Windows.UI.Color>),
                typeof(ColorPicker),
-                new PropertyMetadata(null));
+                new PropertyMetadata(
+                    null,
+                    (s, e) => (s as ColorPicker)?.OnDependencyPropertyChanged(s, e)));

        /// <summary>
        /// Gets the list of custom palette colors.
@ -38,7 +40,9 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
                nameof(CustomPaletteColumnCount),
                typeof(int),
                typeof(ColorPicker),
-                new PropertyMetadata(4));
+                new PropertyMetadata(
+                    4,
+                    (s, e) => (s as ColorPicker)?.OnDependencyPropertyChanged(s, e)));

        /// <summary>
        /// Gets or sets the number of colors in each row (section) of the custom color palette.
@ -64,7 +68,9 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
                nameof(CustomPalette),
                typeof(IColorPalette),
                typeof(ColorPicker),
-                new PropertyMetadata(null));
+                new PropertyMetadata(
+                    null,
+                    (s, e) => (s as ColorPicker)?.OnDependencyPropertyChanged(s, e)));

        /// <summary>
        /// Gets or sets the custom color palette.
@ -91,7 +97,9 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
                nameof(IsColorPaletteVisible),
                typeof(bool),
                typeof(ColorPicker),
-                new PropertyMetadata(true));
+                new PropertyMetadata(
+                    true,
+                    (s, e) => (s as ColorPicker)?.OnDependencyPropertyChanged(s, e)));

        /// <summary>
        /// Gets or sets a value indicating whether the color palette is visible.
--- a/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.cs
+++ b/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.cs
@ -40,6 +40,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
    [TemplatePart(Name = nameof(ColorPicker.CheckeredBackground8Border),  Type = typeof(Border))]
    [TemplatePart(Name = nameof(ColorPicker.CheckeredBackground9Border),  Type = typeof(Border))]
    [TemplatePart(Name = nameof(ColorPicker.CheckeredBackground10Border), Type = typeof(Border))]
+    [TemplatePart(Name = nameof(ColorPicker.ColorPanelSelector),          Type = typeof(ListBox))]
    [TemplatePart(Name = nameof(ColorPicker.ColorSpectrumControl),        Type = typeof(ColorSpectrum))]
    [TemplatePart(Name = nameof(ColorPicker.ColorSpectrumAlphaSlider),    Type = typeof(ColorPickerSlider))]
    [TemplatePart(Name = nameof(ColorPicker.ColorSpectrumThirdDimensionSlider), Type = typeof(ColorPickerSlider))]
@ -65,8 +66,6 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
        private const int ColorUpdateInterval = 30; // Milliseconds

        private long tokenColor;
-        private long tokenCustomPalette;
-        private long tokenIsColorPaletteVisible;

        private bool callbacksConnected = false;
        private bool eventsConnected    = false;
@ -78,6 +77,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
        private Color?          updatedRgbColor            = null;
        private DispatcherQueueTimer dispatcherQueueTimer            = null;

+        private ListBox           ColorPanelSelector;
        private ColorSpectrum     ColorSpectrumControl;
        private ColorPickerSlider ColorSpectrumAlphaSlider;
        private ColorPickerSlider ColorSpectrumThirdDimensionSlider;
@ -177,6 +177,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
            // We need to disconnect old events first
            this.ConnectEvents(false);

+            this.ColorPanelSelector = this.GetTemplateChild<ListBox>(nameof(ColorPanelSelector));
+
            this.ColorSpectrumControl              = this.GetTemplateChild<ColorSpectrum>(nameof(ColorSpectrumControl));
            this.ColorSpectrumAlphaSlider          = this.GetTemplateChild<ColorPickerSlider>(nameof(ColorSpectrumAlphaSlider));
            this.ColorSpectrumThirdDimensionSlider = this.GetTemplateChild<ColorPickerSlider>(nameof(ColorSpectrumThirdDimensionSlider));
@ -216,6 +218,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls

            base.OnApplyTemplate();
            this.UpdateVisualState(false);
+            this.ValidateSelectedPanel();
            this.isInitialized = true;
            this.SetActiveColorRepresentation(ColorRepresentation.Rgba);
            this.UpdateColorControlValues(); // TODO: This will also connect events after, can we optimize vs. doing it twice with the ConnectEvents above?
@ -247,23 +250,19 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
        /// <param name="connected">True to connect callbacks, otherwise false.</param>
        private void ConnectCallbacks(bool connected)
        {
-            if ((connected == true) &&
-                (this.callbacksConnected == false))
+            if (connected == true &&
+                this.callbacksConnected == false)
            {
                // Add callbacks for dependency properties
-                this.tokenColor                 = this.RegisterPropertyChangedCallback(ColorProperty,                 OnColorChanged);
-                this.tokenCustomPalette         = this.RegisterPropertyChangedCallback(CustomPaletteProperty,         OnCustomPaletteChanged);
-                this.tokenIsColorPaletteVisible = this.RegisterPropertyChangedCallback(IsColorPaletteVisibleProperty, OnIsColorPaletteVisibleChanged);
+                this.tokenColor = this.RegisterPropertyChangedCallback(ColorProperty, OnColorChanged);

                this.callbacksConnected = true;
            }
-            else if ((connected == false) &&
-                     (this.callbacksConnected == true))
+            else if (connected == false &&
+                     this.callbacksConnected == true)
            {
                // Remove callbacks for dependency properties
-                this.UnregisterPropertyChangedCallback(ColorProperty,                 this.tokenColor);
-                this.UnregisterPropertyChangedCallback(CustomPaletteProperty,         this.tokenCustomPalette);
-                this.UnregisterPropertyChangedCallback(IsColorPaletteVisibleProperty, this.tokenIsColorPaletteVisible);
+                this.UnregisterPropertyChangedCallback(ColorProperty, this.tokenColor);

                this.callbacksConnected = false;
            }
@ -277,8 +276,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
        /// <param name="connected">True to connect event handlers, otherwise false.</param>
        private void ConnectEvents(bool connected)
        {
-            if ((connected == true) &&
-                (this.eventsConnected == false))
+            if (connected == true &&
+                this.eventsConnected == false)
            {
                // Add all events
                if (this.ColorSpectrumControl != null) { this.ColorSpectrumControl.ColorChanged += ColorSpectrum_ColorChanged; }
@ -331,8 +330,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls

                this.eventsConnected = true;
            }
-            else if ((connected == false) &&
-                     (this.eventsConnected == true))
+            else if (connected == false &&
+                     this.eventsConnected == true)
            {
                // Remove all events
                if (this.ColorSpectrumControl != null) { this.ColorSpectrumControl.ColorChanged -= ColorSpectrum_ColorChanged; }
@ -1065,6 +1064,83 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
            return;
        }

+        /// <summary>
+        /// Validates and updates the current 'tab' or 'panel' selection.
+        /// If the currently selected tab is collapsed, the next visible tab will be selected.
+        /// </summary>
+        private void ValidateSelectedPanel()
+        {
+            object selectedItem = null;
+
+            if (this.ColorPanelSelector != null)
+            {
+                if (this.ColorPanelSelector.SelectedItem == null &&
+                    this.ColorPanelSelector.Items.Count > 0)
+                {
+                    // As a failsafe, forcefully select the first item
+                    selectedItem = this.ColorPanelSelector.Items[0];
+                }
+                else
+                {
+                    selectedItem = this.ColorPanelSelector.SelectedItem;
+                }
+
+                if (selectedItem is UIElement selectedElement &&
+                    selectedElement.Visibility == Visibility.Collapsed)
+                {
+                    // Select the first visible item instead
+                    foreach (object item in this.ColorPanelSelector.Items)
+                    {
+                        if (item is UIElement element &&
+                            element.Visibility == Visibility.Visible)
+                        {
+                            selectedItem = item;
+                            break;
+                        }
+                    }
+                }
+
+                this.ColorPanelSelector.SelectedItem = selectedItem;
+            }
+
+            return;
+        }
+
+        private void OnDependencyPropertyChanged(object sender, DependencyPropertyChangedEventArgs args)
+        {
+            DependencyObject senderControl = sender as DependencyObject;
+
+            /* Note: ColorProperty is defined in the base class and cannot be used here
+             * See the OnColorChanged callback below
+             */
+
+            if (object.ReferenceEquals(args.Property, CustomPaletteProperty))
+            {
+                IColorPalette palette = this.CustomPalette;
+
+                if (palette != null)
+                {
+                    this.CustomPaletteColumnCount = palette.ColorCount;
+                    this.CustomPaletteColors.Clear();
+
+                    for (int shadeIndex = 0; shadeIndex < palette.ShadeCount; shadeIndex++)
+                    {
+                        for (int colorIndex = 0; colorIndex < palette.ColorCount; colorIndex++)
+                        {
+                            this.CustomPaletteColors.Add(palette.GetColor(colorIndex, shadeIndex));
+                        }
+                    }
+                }
+            }
+            else if (object.ReferenceEquals(args.Property, IsColorPaletteVisibleProperty))
+            {
+                this.UpdateVisualState(false);
+                this.ValidateSelectedPanel();
+            }
+
+            return;
+        }
+
        /***************************************************************************************
         *
         * Color Update Timer
@ -1147,39 +1223,6 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
            return;
        }

-        /// <summary>
-        /// Callback for when the <see cref="CustomPalette"/> dependency property value changes.
-        /// </summary>
-        private void OnCustomPaletteChanged(DependencyObject d, DependencyProperty e)
-        {
-            IColorPalette palette = this.CustomPalette;
-
-            if (palette != null)
-            {
-                this.CustomPaletteColumnCount = palette.ColorCount;
-                this.CustomPaletteColors.Clear();
-
-                for (int shadeIndex = 0; shadeIndex < palette.ShadeCount; shadeIndex++)
-                {
-                    for (int colorIndex = 0; colorIndex < palette.ColorCount; colorIndex++)
-                    {
-                        this.CustomPaletteColors.Add(palette.GetColor(colorIndex, shadeIndex));
-                    }
-                }
-            }
-
-            return;
-        }
-
-        /// <summary>
-        /// Callback for when the <see cref="IsColorPaletteVisible"/> dependency property value changes.
-        /// </summary>
-        private void OnIsColorPaletteVisibleChanged(DependencyObject d, DependencyProperty e)
-        {
-            this.UpdateVisualState(false);
-            return;
-        }
-
        /***************************************************************************************
         *
         * Event Handling
--- a/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.xaml
+++ b/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.xaml
@ -69,12 +69,13 @@
                    CornerRadius="4" />
            <ListBox x:Name="ColorPanelSelector"
                     Grid.Row="0"
-                     HorizontalAlignment="Center"
+                     HorizontalAlignment="Stretch"
                     Background="Transparent"
+                     ScrollViewer.HorizontalScrollBarVisibility="Hidden"
                     ItemContainerStyle="{StaticResource UnderlineListBoxItemStyle}">
                <ListBox.ItemsPanel>
                    <ItemsPanelTemplate>
-                        <StackPanel Orientation="Horizontal" />
+                        <controls:UniformGrid Rows="1" />
                    </ItemsPanelTemplate>
                </ListBox.ItemsPanel>
                <ListBoxItem x:Name="SpectrumListBoxItem"
@ -1180,9 +1181,6 @@
        <Setter Property="Padding" Value="{StaticResource ListBoxItemPadding}" />
        <Setter Property="HorizontalContentAlignment" Value="Stretch" />
        <Setter Property="VerticalContentAlignment" Value="Stretch" />
-        <Setter Property="Margin" Value="8,0,8,0" />
-        <Setter Property="Padding" Value="24,0,24,0" />
-        <Setter Property="MinWidth" Value="100" />
        <Setter Property="UseSystemFocusVisuals" Value="{StaticResource UseSystemFocusVisuals}" />
        <Setter Property="Template" Value="{StaticResource UnderlineListBoxItemTemplate}" />
    </Style>
--- a/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPickerSlider.Properties.cs
+++ b/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPickerSlider.Properties.cs
@ -104,7 +104,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls.Primitives
                typeof(Brush),
                typeof(ColorPickerSlider),
                new PropertyMetadata(
-                    new SolidColorBrush(Colors.Gray),
+                    null,
                    (s, e) => (s as ColorPickerSlider)?.OnDependencyPropertyChanged(s, e)));

        /// <summary>
--- a/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPickerSlider.xaml
+++ b/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPickerSlider.xaml
@ -9,6 +9,7 @@
           TargetType="controls:ColorPickerSlider">
        <Setter Property="VerticalAlignment" Value="Center" />
        <Setter Property="IsThumbToolTipEnabled" Value="False" />
+        <Setter Property="DefaultForeground" Value="Gray" />
        <Setter Property="Template">
            <Setter.Value>
                <!--  Based on WinUI version 2.4.2  -->