Progressbar: Fix issue with color changing when updating value while …

…error state (#3201) * Fix issue with color changing when updating value while error state * Switch back to path binding * Cleanup * Remove duplicate storyboard * Split into testsuites * Add polyfill * Update theme resource file * Update vcx file * CR feedback
microsoft · Dec 14, 2020 · 3a176d2 · 3a176d2
1 parent 31ca62b
commit 3a176d2
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 30 deletions.
diff --git a/dev/CommonStyles/CommonStyles.vcxitems b/dev/CommonStyles/CommonStyles.vcxitems
@@ -87,11 +87,6 @@
       <Version>RS1</Version>
       <Type>ThemeResources</Type>
     </Page>
-    <Page Include="$(MSBuildThisFileDirectory)ProgressBar_themeresources.xaml">
-      <Version>RS1</Version>
-      <Type>ThemeResources</Type>
-      <UseNonstandardCondtionalXAML>true</UseNonstandardCondtionalXAML>
-    </Page>
     <Page Include="$(MSBuildThisFileDirectory)MediaTransportControls_themeresources.xaml">
       <Version>RS1</Version>
       <Type>ThemeResources</Type>

diff --git a/dev/ProgressBar/InteractionTests/ProgressBarTests.cs b/dev/ProgressBar/InteractionTests/ProgressBarTests.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
 using System;
@@ -42,6 +42,7 @@ public void TestCleanup()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "A")]
         public void ChangeValueTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))
@@ -67,6 +68,7 @@ public void ChangeValueTest()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "A")]
         public void UpdateIndicatorWidthTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))
@@ -128,6 +130,7 @@ public void UpdateIndicatorWidthTest()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "A")]
         public void UpdateMinMaxTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))
@@ -191,6 +194,7 @@ public void UpdateMinMaxTest()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "B")]
         public void ChangeStateTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))
@@ -253,6 +257,7 @@ public void ChangeStateTest()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "B")]
         public void PaddingOffsetTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))
@@ -297,6 +302,7 @@ public void PaddingOffsetTest()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "C")]
         public void RetemplateUpdateIndicatorWidthTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))
@@ -362,6 +368,7 @@ public void RetemplateUpdateIndicatorWidthTest()
         }
 
         [TestMethod]
+        [TestProperty("TestSuite", "C")]
         public void ReTemplateChangeStateTest()
         {
             using (var setup = new TestSetupHelper("ProgressBar Tests"))

diff --git a/dev/ProgressBar/ProgressBar.cpp b/dev/ProgressBar/ProgressBar.cpp
@@ -78,31 +78,38 @@ void ProgressBar::OnShowErrorPropertyChanged(const winrt::DependencyPropertyChan
 
 void ProgressBar::UpdateStates()
 {
-    if (ShowError() && IsIndeterminate())
-    {
-        winrt::VisualStateManager::GoToState(*this, s_IndeterminateErrorStateName, true);
-    }
-    else if (ShowError())
-    {
-        winrt::VisualStateManager::GoToState(*this, s_ErrorStateName, true);
-    }
-    else if (ShowPaused() && IsIndeterminate())
-    {
-        winrt::VisualStateManager::GoToState(*this, s_IndeterminatePausedStateName, true);
-    }
-    else if (ShowPaused())
-    {
-        winrt::VisualStateManager::GoToState(*this, s_PausedStateName, true);
-    }
-    else if (IsIndeterminate())
+    if (IsIndeterminate())
     {
+        if (ShowError())
+        {
+            winrt::VisualStateManager::GoToState(*this, s_IndeterminateErrorStateName, true);
+        }
+        else if (ShowPaused())
+        {
+            winrt::VisualStateManager::GoToState(*this, s_IndeterminatePausedStateName, true);
+        }
+        else
+        {
+            winrt::VisualStateManager::GoToState(*this, s_IndeterminateStateName, true);
+        }
         UpdateWidthBasedTemplateSettings();
-        winrt::VisualStateManager::GoToState(*this, s_IndeterminateStateName, true);
     }
-    else if (!IsIndeterminate())
+    else
     {
-        winrt::VisualStateManager::GoToState(*this, s_DeterminateStateName, true);
+        if (ShowError())
+        {
+            winrt::VisualStateManager::GoToState(*this, s_ErrorStateName, true);
+        }
+        else if(ShowPaused())
+        {
+            winrt::VisualStateManager::GoToState(*this, s_PausedStateName, true);
+        }
+        else
+        {
+            winrt::VisualStateManager::GoToState(*this, s_DeterminateStateName, true);
+        }
     }
+
 }
 
 void ProgressBar::SetProgressBarIndicatorWidth()
@@ -121,7 +128,14 @@ void ProgressBar::SetProgressBarIndicatorWidth()
 
             // Adds "Updating" state in between to trigger RepositionThemeAnimation Visual Transition
             // in ProgressBar.xaml when reverting back to previous state
-            winrt::VisualStateManager::GoToState(*this, s_UpdatingStateName, true);
+            if (ShowError())
+            {
+                winrt::VisualStateManager::GoToState(*this, s_UpdatingWithErrorStateName, true);
+            }
+            else
+            {
+                winrt::VisualStateManager::GoToState(*this, s_UpdatingStateName, true);
+            }
 
             if (IsIndeterminate())
             {

diff --git a/dev/ProgressBar/ProgressBar.h b/dev/ProgressBar/ProgressBar.h
@@ -55,4 +55,5 @@ class ProgressBar :
     static constexpr wstring_view s_IndeterminatePausedStateName{ L"IndeterminatePaused" };
     static constexpr wstring_view s_DeterminateStateName{ L"Determinate" };
     static constexpr wstring_view s_UpdatingStateName{ L"Updating" };
+    static constexpr wstring_view s_UpdatingWithErrorStateName{ L"UpdatingError" };
 };
diff --git a/dev/ProgressBar/ProgressBar.vcxitems b/dev/ProgressBar/ProgressBar.vcxitems
@@ -33,11 +33,16 @@
       <Version>RS1</Version>
       <Type>DefaultStyle</Type>
     </Page>
+    <Page Include="$(MSBuildThisFileDirectory)ProgressBar_themeresources.xaml">
+      <Version>RS1</Version>
+      <Type>ThemeResources</Type>
+      <UseNonstandardCondtionalXAML>true</UseNonstandardCondtionalXAML>
+    </Page>
   </ItemGroup>
   <ItemGroup>
     <Midl Include="$(MSBuildThisFileDirectory)ProgressBar.idl" />
   </ItemGroup>
   <ItemGroup>
     <PRIResource Include="$(MSBuildThisFileDirectory)Strings\en-us\Resources.resw" />
   </ItemGroup>
-</Project>
+</Project>
diff --git a/dev/ProgressBar/ProgressBar.xaml b/dev/ProgressBar/ProgressBar.xaml
@@ -30,6 +30,11 @@
                                             <RepositionThemeAnimation TargetName="DeterminateProgressBarIndicator" FromHorizontalOffset="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.IndicatorLengthDelta}" />
                                         </Storyboard>
                                     </VisualTransition>
+                                    <VisualTransition From="UpdatingError" To="Error">
+                                        <Storyboard>
+                                            <RepositionThemeAnimation TargetName="DeterminateProgressBarIndicator" FromHorizontalOffset="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.IndicatorLengthDelta}" />
+                                        </Storyboard>
+                                    </VisualTransition>
                                     <VisualTransition From="Paused" To="Determinate">
                                         <Storyboard>
                                             <DoubleAnimation
@@ -65,6 +70,20 @@
                                 <VisualState x:Name="Normal" />
                                 <VisualState x:Name="Determinate" />
                                 <VisualState x:Name="Updating" />
+                                <VisualState x:Name="UpdatingError">
+                                    <Storyboard>
+                                        <ColorAnimation
+                                        Storyboard.TargetName="DeterminateProgressBarIndicator"
+                                        Storyboard.TargetProperty="(Shape.Fill).(SolidColorBrush.Color)"
+                                        To="{Binding Source={ThemeResource SystemControlErrorTextForegroundBrush},Path=Color}"
+                                        Duration="0:0:0.0"/>
+                                        <ColorAnimation
+                                        Storyboard.TargetName="ProgressBarRoot"
+                                        Storyboard.TargetProperty="(Border.Background).(SolidColorBrush.Color)"
+                                        To="{StaticResource SystemControlErrorBackgroundColor}"
+                                        Duration="0:0:0.0"/>
+                                    </Storyboard>
+                                </VisualState>
                                 <VisualState x:Name="Indeterminate">
                                     <Storyboard RepeatBehavior="Forever">
                                         <DoubleAnimationUsingKeyFrames
@@ -119,7 +138,7 @@
                                             Storyboard.TargetName="IndeterminateProgressBarIndicator2"
                                             Storyboard.TargetProperty="(Shape.Fill).(SolidColorBrush.Color)">
                                             <LinearColorKeyFrame Value="{ThemeResource SystemAccentColor}" KeyTime="0:0:2.75" />
-                                            <LinearColorKeyFrame Value="{ThemeResource SystemErrorTextColor}" KeyTime="0:0:3" />
+                                            <LinearColorKeyFrame Value="{Binding Source={ThemeResource SystemControlErrorTextForegroundBrush},Path=Color}" KeyTime="0:0:3" />
                                         </ColorAnimationUsingKeyFrames>
                                         <ColorAnimationUsingKeyFrames
                                             Storyboard.TargetName="ProgressBarRoot"
@@ -135,7 +154,7 @@
                                         <ColorAnimation
                                             Storyboard.TargetName="DeterminateProgressBarIndicator"
                                             Storyboard.TargetProperty="(Shape.Fill).(SolidColorBrush.Color)"
-                                            To="{ThemeResource SystemErrorTextColor}"
+                                            To="{Binding Source={ThemeResource SystemControlErrorTextForegroundBrush},Path=Color}"
                                             Duration="0:0:0.25"/>
                                         <ColorAnimation
                                             Storyboard.TargetName="ProgressBarRoot"

diff --git a/...monStyles/ProgressBar_themeresources.xaml → ...ogressBar/ProgressBar_themeresources.xaml b/...monStyles/ProgressBar_themeresources.xaml → ...ogressBar/ProgressBar_themeresources.xaml
@@ -11,6 +11,7 @@
             <Thickness x:Key="ProgressBarBorderThemeThickness">0</Thickness>
             <Color x:Key="SystemControlErrorBackgroundColor">#33FFFFFF</Color>
             <contract7NotPresent:Color x:Key="SystemErrorTextColor">#FFF000</contract7NotPresent:Color>
+            <contract7NotPresent:SolidColorBrush x:Key="SystemControlErrorTextForegroundBrush" Color="{ThemeResource SystemErrorTextColor}"/>
             <SolidColorBrush x:Key="ProgressBarBackgroundThemeBrush" Color="#59FFFFFF" />
             <SolidColorBrush x:Key="ProgressBarBorderThemeBrush" Color="Transparent" />
             <SolidColorBrush x:Key="ProgressBarForegroundThemeBrush" Color="#FF5B2EC5" />
@@ -20,6 +21,8 @@
             <x:Double x:Key="ProgressBarIndicatorPauseOpacity">1</x:Double>
             <x:Double x:Key="ProgressBarThemeMinHeight">4</x:Double>
             <Thickness x:Key="ProgressBarBorderThemeThickness">1</Thickness>
+            <contract7NotPresent:Color x:Key="SystemErrorTextColor">#FFF000</contract7NotPresent:Color>
+            <contract7NotPresent:SolidColorBrush x:Key="SystemControlErrorTextForegroundBrush" Color="{ThemeResource SystemErrorTextColor}"/>
             <SolidColorBrush x:Key="ProgressBarBackgroundThemeBrush" Color="{ThemeResource SystemColorButtonFaceColor}" />
             <SolidColorBrush x:Key="ProgressBarBorderThemeBrush" Color="{ThemeResource SystemColorButtonTextColor}" />
             <SolidColorBrush x:Key="ProgressBarForegroundThemeBrush" Color="{ThemeResource SystemColorHighlightColor}" />
@@ -32,6 +35,7 @@
             <Color x:Key="SystemControlErrorBackgroundColor">#29C50500</Color>
             <!-- Hex value for SystemErrorText at 0.16 opacity -->
             <contract7NotPresent:Color x:Key="SystemErrorTextColor">#C50500</contract7NotPresent:Color>
+            <contract7NotPresent:SolidColorBrush x:Key="SystemControlErrorTextForegroundBrush" Color="{ThemeResource SystemErrorTextColor}"/>
             <SolidColorBrush x:Key="ProgressBarBackgroundThemeBrush" Color="#30000000" />
             <SolidColorBrush x:Key="ProgressBarBorderThemeBrush" Color="Transparent" />
             <SolidColorBrush x:Key="ProgressBarForegroundThemeBrush" Color="#FF4617B4" />