Persist selected color scheme on navigation; Don't gray-out color swa…

…tches (microsoft#8799) ## Summary of the Pull Request This PR fixes two of the components of microsoft#8765. > * [ ] Edit a color scheme -> Hit 'apply' -> the selected color scheme resets to the first color scheme in the list (instead of the one just edited) This was fixed by storing the navigation state as a singleton in MainPage, and having the color schemes page update the selected scheme on that singleton. That way, a subsequent navigation to the schemes page could re-use the existing state. > * [ ] The buttons turn gray on rollover covering up what color I'm looking at (I have dark mode) This one was tricky. We're binding the resource for this button, to the color the button is bound to. We're also running a converter on that color, as to change the alpha slightly. This allows us to still have visual feedback on pointerover, without obscuring the color entirely. ## PR Checklist * [x] I work here * [x] Tested manually
mpela81 · Jan 28, 2021 · 1f8e660 · 1f8e660
1 parent bb51c1a
commit 1f8e660
Show file tree

Hide file tree

Showing 10 changed files with 166 additions and 1 deletion.
diff --git a/src/cascadia/TerminalSettingsEditor/ColorLightenConverter.cpp b/src/cascadia/TerminalSettingsEditor/ColorLightenConverter.cpp
@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "pch.h"
+#include "ColorLightenConverter.h"
+#include "ColorLightenConverter.g.cpp"
+
+using namespace winrt::Windows;
+using namespace winrt::Windows::UI;
+using namespace winrt::Windows::UI::Xaml;
+using namespace winrt::Windows::UI::Text;
+
+namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
+{
+    Foundation::IInspectable ColorLightenConverter::Convert(Foundation::IInspectable const& value,
+                                                            Windows::UI::Xaml::Interop::TypeName const& /* targetType */,
+                                                            Foundation::IInspectable const& /* parameter */,
+                                                            hstring const& /* language */)
+    {
+        auto original = winrt::unbox_value_or<Color>(value, Color{ 255, 0, 0, 0 });
+        auto result = original;
+        result.A = 128; // halfway transparent
+        return winrt::box_value(result);
+    }
+
+    Foundation::IInspectable ColorLightenConverter::ConvertBack(Foundation::IInspectable const& /*value*/,
+                                                                Windows::UI::Xaml::Interop::TypeName const& /* targetType */,
+                                                                Foundation::IInspectable const& /*parameter*/,
+                                                                hstring const& /* language */)
+    {
+        throw hresult_not_implemented();
+    }
+}
diff --git a/src/cascadia/TerminalSettingsEditor/ColorLightenConverter.h b/src/cascadia/TerminalSettingsEditor/ColorLightenConverter.h
@@ -0,0 +1,9 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#pragma once
+
+#include "ColorLightenConverter.g.h"
+#include "../inc/cppwinrt_utils.h"
+
+DECLARE_CONVERTER(winrt::Microsoft::Terminal::Settings::Editor, ColorLightenConverter);
diff --git a/src/cascadia/TerminalSettingsEditor/ColorSchemes.cpp b/src/cascadia/TerminalSettingsEditor/ColorSchemes.cpp
@@ -73,6 +73,18 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             auto entry = winrt::make<ColorTableEntry>(i, Windows::UI::Color{ 0, 0, 0, 0 });
             _CurrentColorTable.Append(entry);
         }
+
+        // Try to look up the scheme that was navigated to. If we find it, immediately select it.
+        const std::wstring lastNameFromNav{ _State.LastSelectedScheme() };
+        const auto it = std::find_if(begin(_ColorSchemeList),
+                                     end(_ColorSchemeList),
+                                     [&lastNameFromNav](const auto& scheme) { return scheme.Name() == lastNameFromNav; });
+
+        if (it != end(_ColorSchemeList))
+        {
+            auto scheme = *it;
+            ColorSchemeComboBox().SelectedItem(scheme);
+        }
     }
 
     // Function Description:
@@ -90,6 +102,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         CurrentColorScheme(colorScheme);
         _UpdateColorTable(colorScheme);
 
+        _State.LastSelectedScheme(colorScheme.Name());
+
         // Set the text disclaimer for the text box
         hstring disclaimer{};
         const std::wstring schemeName{ colorScheme.Name() };

diff --git a/src/cascadia/TerminalSettingsEditor/ColorSchemes.h b/src/cascadia/TerminalSettingsEditor/ColorSchemes.h
@@ -17,6 +17,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             _Globals{ settings } {}
 
         GETSET_PROPERTY(Model::GlobalAppSettings, Globals, nullptr);
+        GETSET_PROPERTY(winrt::hstring, LastSelectedScheme, L"");
     };
 
     struct ColorSchemes : ColorSchemesT<ColorSchemes>

diff --git a/src/cascadia/TerminalSettingsEditor/ColorSchemes.idl b/src/cascadia/TerminalSettingsEditor/ColorSchemes.idl
@@ -3,9 +3,11 @@
 
 namespace Microsoft.Terminal.Settings.Editor
 {
+
     runtimeclass ColorSchemesPageNavigationState
     {
         Microsoft.Terminal.Settings.Model.GlobalAppSettings Globals;
+        String LastSelectedScheme;
     };
 
     [default_interface] runtimeclass ColorSchemes : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged

diff --git a/src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml b/src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml
@@ -44,6 +44,8 @@ the MIT License. See LICENSE in the project root for license information. -->
             <local:ColorToBrushConverter x:Key="ColorToBrushConverter"/>
             <local:ColorToHexConverter x:Key="ColorToHexConverter"/>
             <local:InvertedBooleanToVisibilityConverter x:Key="InvertedBooleanToVisibilityConverter"/>
+            <local:ColorLightenConverter x:Key="ColorLightenConverter"/>
+
         </ResourceDictionary>
     </Page.Resources>
 
@@ -139,6 +141,23 @@ the MIT License. See LICENSE in the project root for license information. -->
                             <TextBlock Text="{x:Bind Name, Mode=OneWay}"
                                        Style="{StaticResource ColorHeaderStyle}"/>
                             <Button Background="{x:Bind Color, Converter={StaticResource ColorToBrushConverter}, Mode=OneWay}">
+
+
+                                <Button.Resources>
+                                    <!-- Resources to colorize hover/pressed states based on the color of the button -->
+                                    <ResourceDictionary>
+                                        <ResourceDictionary.ThemeDictionaries>
+                                            <ResourceDictionary x:Key="Light">
+                                                <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{x:Bind Color, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                            </ResourceDictionary>
+                                            <ResourceDictionary x:Key="Dark">
+                                                <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{x:Bind Color, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                            </ResourceDictionary>
+                                            <!-- No High contrast dictionary, let's just leave that unchanged. -->
+                                        </ResourceDictionary.ThemeDictionaries>
+                                    </ResourceDictionary>
+                                </Button.Resources>
+
                                 <Button.Flyout>
                                     <Flyout>
                                         <ColorPicker Tag="{x:Bind Index, Mode=OneWay}"
@@ -159,6 +178,23 @@ the MIT License. See LICENSE in the project root for license information. -->
                     <TextBlock x:Uid="ColorScheme_Foreground"
                                Style="{StaticResource ColorHeaderStyle}"/>
                     <Button Background="{Binding Color, ElementName=ForegroundPicker, Converter={StaticResource ColorToBrushConverter}, Mode=OneWay}">
+
+
+                        <Button.Resources>
+                            <!-- Resources to colorize hover/pressed states based on the color of the button -->
+                            <ResourceDictionary>
+                                <ResourceDictionary.ThemeDictionaries>
+                                    <ResourceDictionary x:Key="Light">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=ForegroundPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <ResourceDictionary x:Key="Dark">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=ForegroundPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <!-- No High contrast dictionary, let's just leave that unchanged. -->
+                                </ResourceDictionary.ThemeDictionaries>
+                            </ResourceDictionary>
+                        </Button.Resources>
+
                         <Button.Flyout>
                             <Flyout>
                                 <ColorPicker x:Name="ForegroundPicker" Color="{x:Bind CurrentColorScheme.Foreground, Mode=TwoWay}"/>
@@ -170,6 +206,23 @@ the MIT License. See LICENSE in the project root for license information. -->
                     <TextBlock x:Uid="ColorScheme_Background"
                                Style="{StaticResource ColorHeaderStyle}"/>
                     <Button Background="{Binding Color, ElementName=BackgroundPicker, Converter={StaticResource ColorToBrushConverter}, Mode=OneWay}">
+
+
+                        <Button.Resources>
+                            <!-- Resources to colorize hover/pressed states based on the color of the button -->
+                            <ResourceDictionary>
+                                <ResourceDictionary.ThemeDictionaries>
+                                    <ResourceDictionary x:Key="Light">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=BackgroundPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <ResourceDictionary x:Key="Dark">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=BackgroundPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <!-- No High contrast dictionary, let's just leave that unchanged. -->
+                                </ResourceDictionary.ThemeDictionaries>
+                            </ResourceDictionary>
+                        </Button.Resources>
+
                         <Button.Flyout>
                             <Flyout>
                                 <ColorPicker x:Name="BackgroundPicker"
@@ -182,6 +235,23 @@ the MIT License. See LICENSE in the project root for license information. -->
                     <TextBlock x:Uid="ColorScheme_CursorColor"
                                Style="{StaticResource ColorHeaderStyle}"/>
                     <Button Background="{Binding Color, ElementName=CursorColorPicker, Converter={StaticResource ColorToBrushConverter}, Mode=OneWay}">
+
+
+                        <Button.Resources>
+                            <!-- Resources to colorize hover/pressed states based on the color of the button -->
+                            <ResourceDictionary>
+                                <ResourceDictionary.ThemeDictionaries>
+                                    <ResourceDictionary x:Key="Light">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=CursorColorPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <ResourceDictionary x:Key="Dark">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=CursorColorPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <!-- No High contrast dictionary, let's just leave that unchanged. -->
+                                </ResourceDictionary.ThemeDictionaries>
+                            </ResourceDictionary>
+                        </Button.Resources>
+
                         <Button.Flyout>
                             <Flyout>
                                 <ColorPicker x:Name="CursorColorPicker"
@@ -194,6 +264,23 @@ the MIT License. See LICENSE in the project root for license information. -->
                     <TextBlock x:Uid="ColorScheme_SelectionBackground"
                                Style="{StaticResource ColorHeaderStyle}"/>
                     <Button Background="{Binding Color, ElementName=SelectionBackgroundPicker, Converter={StaticResource ColorToBrushConverter}, Mode=OneWay}">
+
+
+                        <Button.Resources>
+                            <!-- Resources to colorize hover/pressed states based on the color of the button -->
+                            <ResourceDictionary>
+                                <ResourceDictionary.ThemeDictionaries>
+                                    <ResourceDictionary x:Key="Light">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=SelectionBackgroundPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <ResourceDictionary x:Key="Dark">
+                                        <SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{Binding Color, ElementName=SelectionBackgroundPicker, Converter={StaticResource ColorLightenConverter}, Mode=OneWay}"/>
+                                    </ResourceDictionary>
+                                    <!-- No High contrast dictionary, let's just leave that unchanged. -->
+                                </ResourceDictionary.ThemeDictionaries>
+                            </ResourceDictionary>
+                        </Button.Resources>
+
                         <Button.Flyout>
                             <Flyout>
                                 <ColorPicker x:Name="SelectionBackgroundPicker"

diff --git a/src/cascadia/TerminalSettingsEditor/Converters.idl b/src/cascadia/TerminalSettingsEditor/Converters.idl
@@ -3,6 +3,12 @@
 
 namespace Microsoft.Terminal.Settings.Editor
 {
+
+    runtimeclass ColorLightenConverter : [default] Windows.UI.Xaml.Data.IValueConverter
+    {
+        ColorLightenConverter();
+    };
+
     runtimeclass FontWeightConverter : [default] Windows.UI.Xaml.Data.IValueConverter
     {
         FontWeightConverter();

diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp
@@ -49,6 +49,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         InitializeComponent();
 
         _InitializeProfilesList();
+
+        _colorSchemesNavState = winrt::make<ColorSchemesPageNavigationState>(_settingsClone.GlobalSettings());
     }
 
     // Method Description:
@@ -94,6 +96,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
         _InitializeProfilesList();
 
+        // Update the Nav State with the new version of the settings
+        _colorSchemesNavState.Globals(_settingsClone.GlobalSettings());
+
         _RefreshCurrentPage();
     }
 
@@ -225,7 +230,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
         else if (clickedItemTag == colorSchemesTag)
         {
-            contentFrame().Navigate(xaml_typename<Editor::ColorSchemes>(), winrt::make<ColorSchemesPageNavigationState>(_settingsClone.GlobalSettings()));
+            contentFrame().Navigate(xaml_typename<Editor::ColorSchemes>(), _colorSchemesNavState);
         }
         else if (clickedItemTag == globalAppearanceTag)
         {

diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.h b/src/cascadia/TerminalSettingsEditor/MainPage.h
@@ -41,6 +41,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         void _Navigate(hstring clickedItemTag);
         void _Navigate(const Editor::ProfileViewModel& profile);
         void _RefreshCurrentPage();
+
+        ColorSchemesPageNavigationState _colorSchemesNavState{ nullptr };
     };
 }
 

diff --git a/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj b/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj
@@ -38,6 +38,9 @@
     <ClInclude Include="ColorToBrushConverter.h">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClInclude>
+    <ClInclude Include="ColorLightenConverter.h">
+      <DependentUpon>Converters.idl</DependentUpon>
+    </ClInclude>
     <ClInclude Include="ColorToHexConverter.h">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClInclude>
@@ -117,6 +120,9 @@
     <ClCompile Include="ColorToBrushConverter.cpp">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClCompile>
+    <ClCompile Include="ColorLightenConverter.cpp">
+      <DependentUpon>Converters.idl</DependentUpon>
+    </ClCompile>
     <ClCompile Include="ColorToHexConverter.cpp">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClCompile>