Rejuvenate the color schemes page

src/cascadia/TerminalSettingsEditor/ColorSchemeViewModel.cpp

@@ -51,7 +51,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
     bool ColorSchemeViewModel::RequestRename(winrt::hstring newName)
     {
-        return _parentPageVM.RequestRenameCurrentScheme(newName);
+        if (const auto parentPageVM{ _parentPageVM.get() })
+        {
+            return parentPageVM.RequestRenameCurrentScheme(newName);
+        }
+        return false;
     }
 
     void ColorSchemeViewModel::Name(winrt::hstring newName)

src/cascadia/TerminalSettingsEditor/ColorSchemeViewModel.h

@@ -49,7 +49,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
     private:
         winrt::hstring _Name;
         Model::ColorScheme _scheme;
-        Editor::ColorSchemesPageViewModel _parentPageVM{ nullptr };
+        weak_ref<Editor::ColorSchemesPageViewModel> _parentPageVM{ nullptr };
 
         void _ColorEntryChangedHandler(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Data::PropertyChangedEventArgs& args);
     };
@@ -69,8 +69,3 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         Windows::UI::Color _color;
     };
 };
-
-namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation
-{
-    BASIC_FACTORY(ColorSchemeViewModel);
-}

src/cascadia/TerminalSettingsEditor/ColorSchemeViewModel.idl

@@ -7,12 +7,10 @@ namespace Microsoft.Terminal.Settings.Editor
 {
     runtimeclass ColorSchemeViewModel : Windows.UI.Xaml.Data.INotifyPropertyChanged, Windows.Foundation.IStringable
     {
-        ColorSchemeViewModel(Microsoft.Terminal.Settings.Model.ColorScheme scheme, ColorSchemesPageViewModel parentPageVM);
-
         String Name;
         Boolean IsInBoxScheme;
 
-        Boolean RequestRename(String newName);
+        Boolean RequestRename(String newName);
 
         // Terminal Colors
         Windows.Foundation.Collections.IVector<ColorTableEntry> NonBrightColorTable;

src/cascadia/TerminalSettingsEditor/ColorSchemes.cpp

@@ -31,7 +31,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         InitializeComponent();
 
         Automation::AutomationProperties::SetName(AddNewButton(), RS_(L"ColorScheme_AddNewButton/Text"));
-        Automation::AutomationProperties::SetName(DeleteButton(), RS_(L"ColorScheme_DeleteButton/Text"));
     }
 
     void ColorSchemes::OnNavigatedTo(const NavigationEventArgs& e)
@@ -64,7 +63,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         itemContainer.as<ContentControl>().Focus(FocusState::Programmatic);
     }
 
-    void ColorSchemes::AddNew_Click(const IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::RoutedEventArgs& /*e*/)
+    void ColorSchemes::AddNew_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*e*/)
     {
         if (const auto newSchemeVM{ _ViewModel.RequestAddNew() })
         {
@@ -73,12 +72,17 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
     }
 
+    void ColorSchemes::Edit_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*e*/)
+    {
+        _ViewModel.RequestEditSelectedScheme();
+    }
+
     void ColorSchemes::ListView_PreviewKeyDown(const IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs& e)
     {
         if (e.OriginalKey() == winrt::Windows::System::VirtualKey::Enter)
         {
             // Treat this as if 'edit' was clicked
-            _ViewModel.Edit_Click(nullptr, nullptr);
+            _ViewModel.RequestEditSelectedScheme();
             e.Handled(true);
         }
         else if (e.OriginalKey() == winrt::Windows::System::VirtualKey::Delete)

src/cascadia/TerminalSettingsEditor/ColorSchemes.h

@@ -21,6 +21,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         void ListView_PreviewKeyDown(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs& e);
         void ListView_SelectionChanged(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::SelectionChangedEventArgs& e);
         void DeleteConfirmation_Click(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e);
+        void Edit_Click(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e);
 
         WINRT_PROPERTY(Model::ColorScheme, CurrentColorScheme, nullptr);
         WINRT_OBSERVABLE_PROPERTY(Editor::ColorSchemesPageViewModel, ViewModel, _PropertyChangedHandlers, nullptr);

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml

@@ -182,28 +182,23 @@
                         Orientation="Horizontal"
                         XYFocusKeyboardNavigation="Enabled">
                 <Button x:Name="EditButton"
-                        Click="{x:Bind ViewModel.Edit_Click}"
+                        Click="Edit_Click"
                         IsEnabled="{x:Bind ViewModel.HasCurrentScheme, Mode=OneWay}"
                         Style="{StaticResource BrowseButtonStyle}">
-                    <TextBlock x:Uid="ColorScheme_EditButton"
-                               Style="{StaticResource IconButtonTextBlockStyle}" />
+                    <TextBlock x:Uid="ColorScheme_EditButton" />
                 </Button>
 
                 <Button x:Name="AddNewButton"
                         Click="AddNew_Click"
                         Style="{StaticResource BrowseButtonStyle}">
-                    <TextBlock x:Uid="ColorScheme_AddNewButton"
-                               Style="{StaticResource IconButtonTextBlockStyle}" />
+                    <TextBlock x:Uid="ColorScheme_AddNewButton" />
                 </Button>
 
                 <Button x:Name="DeleteButton"
+                        x:Uid="ColorScheme_DeleteButton"
                         Margin="10,0,0,0"
                         IsEnabled="{x:Bind ViewModel.CanDeleteCurrentScheme, Mode=OneWay}"
                         Style="{StaticResource DeleteButtonStyle}">
-                    <Button.Content>
-                        <TextBlock x:Uid="ColorScheme_DeleteButton"
-                                   Style="{StaticResource IconButtonTextBlockStyle}" />
-                    </Button.Content>
                     <Button.Flyout>
                         <Flyout>
                             <StackPanel>

src/cascadia/TerminalSettingsEditor/ColorSchemesPageViewModel.cpp

@@ -77,11 +77,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         return _CurrentScheme != nullptr;
     }
 
-    void ColorSchemesPageViewModel::Edit_Click(const IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::RoutedEventArgs& /*e*/)
-    {
-        CurrentPage(ColorSchemesSubPage::EditColorScheme);
-    }
-
     void ColorSchemesPageViewModel::_MakeColorSchemeVMsHelper()
     {
         std::vector<Editor::ColorSchemeViewModel> allColorSchemes;
@@ -90,7 +85,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         for (const auto& pair : colorSchemeMap)
         {
             const auto scheme = pair.Value();
-            Editor::ColorSchemeViewModel viewModel{ scheme, *this };
+            auto viewModel{ winrt::make<ColorSchemeViewModel>(scheme, *this) };
             viewModel.IsInBoxScheme(std::find(std::begin(InBoxSchemes), std::end(InBoxSchemes), scheme.Name()) != std::end(InBoxSchemes));
             allColorSchemes.emplace_back(viewModel);
 
@@ -113,7 +108,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         _settings.GlobalSettings().AddColorScheme(scheme);
 
         // Construct the new color scheme VM
-        const Editor::ColorSchemeViewModel schemeVM{ scheme, *this };
+        const auto schemeVM{ winrt::make<ColorSchemeViewModel>(scheme, *this) };
         _AllColorSchemes.Append(schemeVM);
         _viewModelToSchemeMap.Insert(schemeVM, scheme);
         return schemeVM;
@@ -173,12 +168,20 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         _settings.UpdateColorSchemeReferences(name, L"Campbell");
     }
 
+    void ColorSchemesPageViewModel::RequestEditSelectedScheme()
+    {
+        if (_CurrentScheme)
+        {
+            CurrentPage(ColorSchemesSubPage::EditColorScheme);
+        }
+    }
+
     bool ColorSchemesPageViewModel::CanDeleteCurrentScheme() const
     {
         if (_CurrentScheme)
         {
             // Only allow this color scheme to be deleted if it's not provided in-box
-            return std::find(std::begin(InBoxSchemes), std::end(InBoxSchemes), _CurrentScheme.Name()) == std::end(InBoxSchemes);
+            return !_CurrentScheme.IsInBoxScheme();
         }
         return false;
     }

src/cascadia/TerminalSettingsEditor/ColorSchemesPageViewModel.h

@@ -24,8 +24,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         Editor::ColorSchemeViewModel RequestAddNew();
         bool RequestRenameCurrentScheme(winrt::hstring newName);
         void RequestDeleteCurrentScheme();
-
-        void Edit_Click(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e);
+        void RequestEditSelectedScheme();
 
         bool CanDeleteCurrentScheme() const;
 

src/cascadia/TerminalSettingsEditor/ColorSchemesPageViewModel.idl

@@ -19,13 +19,12 @@ namespace Microsoft.Terminal.Settings.Editor
         ColorSchemeViewModel RequestAddNew();
         Boolean RequestRenameCurrentScheme(String newName);
         void RequestDeleteCurrentScheme();
+        void RequestEditSelectedScheme();
 
         ColorSchemeViewModel CurrentScheme;
         Boolean HasCurrentScheme { get; };
         Boolean CanDeleteCurrentScheme { get; };
         ColorSchemesSubPage CurrentPage;
         Windows.Foundation.Collections.IObservableVector<ColorSchemeViewModel> AllColorSchemes { get; };
-
-        void Edit_Click(IInspectable sender, Windows.UI.Xaml.RoutedEventArgs e);
     }
 }

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

@@ -1173,7 +1173,7 @@
     <value>Edit</value>
     <comment>Button label that edits the currently selected color scheme.</comment>
   </data>
-  <data name="ColorScheme_DeleteButton.Text" xml:space="preserve">
+  <data name="ColorScheme_DeleteButton.Content" xml:space="preserve">
     <value>Delete</value>
     <comment>Button label that deletes the selected color scheme.</comment>
   </data>
@@ -1233,7 +1233,7 @@
     <value>Rename</value>
     <comment>Text label for a button that can be used to begin the renaming process.</comment>
   </data>
-  <data name="ColorScheme_DeleteButtonDisclaimerInBox" xml:space="preserve">
+  <data name="ColorScheme_DeleteButtonDisclaimerInBox.Text" xml:space="preserve">
     <value>This color scheme cannot be deleted or renamed because it is included by default.</value>
     <comment>Disclaimer presented next to the delete button when it is disabled.</comment>
   </data>
@@ -1345,6 +1345,14 @@
     <value>Reset to inherited value.</value>
     <comment>This button will remove a user's customization from a given setting, restoring it to the value that the profile inherited. This is a text label on a button.</comment>
   </data>
+  <data name="ColorScheme_TerminalColorsHeader.Text" xml:space="preserve">
+    <value>Terminal colors</value>
+    <comment>A header for a grouping of colors in the color scheme. These colors are used for basic parts of the terminal.</comment>
+  </data>
+  <data name="ColorScheme_SystemColorsHeader.Text" xml:space="preserve">
+    <value>System colors</value>
+    <comment>A header for a grouping of colors in the color scheme. These colors are used for functional parts of the terminal.</comment>
+  </data>
   <data name="ColorScheme_ColorsHeader.Header" xml:space="preserve">
     <value>Colors</value>
     <comment>A header for the grouping of colors in the color scheme.</comment>