From 41190214b078ce51b66fdd3bee243a63a43060e8 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 28 Aug 2025 17:10:21 -0700 Subject: [PATCH] [bugged] add support for string when theme color is null --- .../NullableColorPicker.cpp | 33 +++++++++++++++-- .../NullableColorPicker.h | 16 +++++++- .../NullableColorPicker.idl | 11 +++++- .../NullableColorPicker.xaml | 5 ++- .../ProfileViewModel.cpp | 37 ++++--------------- .../TerminalSettingsEditor/ProfileViewModel.h | 4 +- .../ProfileViewModel.idl | 4 +- .../TerminalSettingsEditor/Profiles_Base.cpp | 12 ++++++ .../TerminalSettingsEditor/Profiles_Base.xaml | 26 ++++++++++--- .../SettingContainer.cpp | 10 +++++ .../TerminalSettingsEditor/SettingContainer.h | 1 + .../SettingContainer.idl | 3 ++ .../SettingContainerStyle.xaml | 3 +- 13 files changed, 116 insertions(+), 49 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp index 4cb9f655b7..adecff7c61 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp @@ -4,6 +4,7 @@ #include "pch.h" #include "NullableColorPicker.h" #include "NullableColorPicker.g.cpp" +#include "NullableColorTemplateSelector.g.cpp" #include @@ -88,9 +89,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _NullColorPreviewProperty = DependencyProperty::Register( L"NullColorPreview", - xaml_typename(), + xaml_typename>(), xaml_typename(), - PropertyMetadata{ box_value(Windows::UI::Colors::Transparent()) }); + PropertyMetadata{ nullptr }); } } @@ -154,6 +155,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation return color == nullptr; } + Visibility NullableColorPicker::IsNullToVisibility(Windows::Foundation::IReference color) + { + return color == nullptr ? Visibility::Collapsed : Visibility::Visible; + } + void NullableColorPicker::NullColorButton_Clicked(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/) { CurrentColor(nullptr); @@ -178,10 +184,15 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation }; ColorPickerControl().Color(winuiColor); } - else + else if (const auto nullColor = NullColorPreview()) { // No current color (null), use the deduced value for null - ColorPickerControl().Color(NullColorPreview()); + ColorPickerControl().Color(nullColor.Value()); + } + else + { + // NullColorPreview is undefined, use a fallback value + ColorPickerControl().Color(Colors::Transparent()); } } @@ -223,4 +234,18 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } } } + + Windows::UI::Xaml::DataTemplate NullableColorTemplateSelector::SelectTemplateCore(const winrt::Windows::Foundation::IInspectable& item, const winrt::Windows::UI::Xaml::DependencyObject& /*container*/) + { + return SelectTemplateCore(item); + } + + Windows::UI::Xaml::DataTemplate NullableColorTemplateSelector::SelectTemplateCore(const winrt::Windows::Foundation::IInspectable& item) + { + if (const auto nullableColor = item.try_as>()) + { + return ColorTemplate(); + } + return NullColorTemplate(); + } } diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h index 876ce23d7f..36833501c1 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h @@ -4,6 +4,7 @@ #pragma once #include "NullableColorPicker.g.h" +#include "NullableColorTemplateSelector.g.h" #include "Utils.h" namespace winrt::Microsoft::Terminal::Settings::Editor::implementation @@ -15,6 +16,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation static winrt::Windows::UI::Xaml::Media::SolidColorBrush CalculateBorderBrush(const Windows::UI::Color& color); static bool IsNull(Windows::Foundation::IReference color); + static Windows::UI::Xaml::Visibility IsNullToVisibility(Windows::Foundation::IReference color); void ColorChip_Loaded(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args); void ColorChip_Unloaded(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args); @@ -31,7 +33,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation DEPENDENCY_PROPERTY(Windows::Foundation::IReference, CurrentColor); DEPENDENCY_PROPERTY(bool, ShowNullColorButton); DEPENDENCY_PROPERTY(hstring, NullColorButtonLabel); - DEPENDENCY_PROPERTY(Windows::UI::Color, NullColorPreview); + DEPENDENCY_PROPERTY(Windows::Foundation::IReference, NullColorPreview); private: static void _InitializeProperties(); @@ -41,9 +43,21 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation std::vector _colorChips; }; + + struct NullableColorTemplateSelector : NullableColorTemplateSelectorT + { + NullableColorTemplateSelector() = default; + + Windows::UI::Xaml::DataTemplate SelectTemplateCore(const winrt::Windows::Foundation::IInspectable&, const winrt::Windows::UI::Xaml::DependencyObject&); + Windows::UI::Xaml::DataTemplate SelectTemplateCore(const winrt::Windows::Foundation::IInspectable&); + + WINRT_PROPERTY(winrt::Windows::UI::Xaml::DataTemplate, ColorTemplate); + WINRT_PROPERTY(winrt::Windows::UI::Xaml::DataTemplate, NullColorTemplate); + }; } namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation { BASIC_FACTORY(NullableColorPicker); + BASIC_FACTORY(NullableColorTemplateSelector); } diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.idl b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.idl index 4722906543..71ada8a0ed 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.idl +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.idl @@ -21,10 +21,19 @@ namespace Microsoft.Terminal.Settings.Editor String NullColorButtonLabel; static Windows.UI.Xaml.DependencyProperty NullColorButtonLabelProperty { get; }; - Windows.UI.Color NullColorPreview; + Windows.Foundation.IReference NullColorPreview; static Windows.UI.Xaml.DependencyProperty NullColorPreviewProperty { get; }; static Windows.UI.Xaml.Media.SolidColorBrush CalculateBorderBrush(Windows.UI.Color color); static Boolean IsNull(Windows.Foundation.IReference color); + static Windows.UI.Xaml.Visibility IsNullToVisibility(Windows.Foundation.IReference color); + } + + [default_interface] runtimeclass NullableColorTemplateSelector : Windows.UI.Xaml.Controls.DataTemplateSelector + { + NullableColorTemplateSelector(); + + Windows.UI.Xaml.DataTemplate ColorTemplate; + Windows.UI.Xaml.DataTemplate NullColorTemplate; } } diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml index 6755543311..dfff02ac3a 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml @@ -136,9 +136,10 @@ + CornerRadius="{StaticResource ControlCornerRadius}" + Visibility="{x:Bind IsNullToVisibility(NullColorPreview), Mode=OneWay}" /> diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp index 08954045df..76fd5c1d2e 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp @@ -151,6 +151,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { _NotifyChanges(L"AnswerbackMessagePreview"); } + else if (viewModelProperty == L"TabColor") + { + _NotifyChanges(L"TabColorPreview"); + } else if (viewModelProperty == L"TabThemeColorPreview") { _NotifyChanges(L"TabColorPreview"); @@ -449,7 +453,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation return RS_(L"Profile_AnswerbackMessageNone"); } - Windows::UI::Color ProfileViewModel::TabColorPreview() const + Windows::Foundation::IReference ProfileViewModel::TabColorPreview() const { if (const auto modelVal = _profile.TabColor()) { @@ -465,7 +469,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation return TabThemeColorPreview(); } - Windows::UI::Color ProfileViewModel::TabThemeColorPreview() const + Windows::Foundation::IReference ProfileViewModel::TabThemeColorPreview() const { if (const auto currentTheme = _appSettings.GlobalSettings().CurrentTheme()) { @@ -489,34 +493,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } // XAML default color for tab - // TODO CARLOS: I keep getting #FFFFFFFF - // - TabViewItemHeaderBackground: gives value above, is incorrect - // - LayerOnMicaBaseAltFillColorTransparentBrush: resolved value for TabViewItemHeaderBackground (from XAML GH); same issue - // - // I think the problem is that TabViewItemHeaderBackground is set on the TabViewItem, but I don't have access to that - - // NEXT STEPS: allow NullableColorPicker to show color preview OR just text - // - add template selector - // - 2 templates: - // - color? --> ColorPreviewTemplate (in CommonResources.xaml) - // - else --> String/TextBlock - // - pass in string for when template selector fails? - - - if (const auto tabHeaderBackground = Application::Current().Resources().Lookup(box_value(L"TabViewItemHeaderBackground"))) - { - if (const auto brush = tabHeaderBackground.try_as()) - { - // Fill the alpha so that the color can actually be displayed in the preview - auto brushColor = brush.Color(); - brushColor.A = static_cast(255); - return brushColor; - } - } - - // This should never happen, but if it does, return Transparent so it's obvious - assert(false); - return Windows::UI::Colors::Transparent(); + return nullptr; } Editor::AppearanceViewModel ProfileViewModel::DefaultAppearance() const diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h index 72395995a5..5024ec5bfc 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h @@ -124,8 +124,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation bool Orphaned() const; hstring TabTitlePreview() const; hstring AnswerbackMessagePreview() const; - Windows::UI::Color TabColorPreview() const; - Windows::UI::Color TabThemeColorPreview() const; + Windows::Foundation::IReference TabColorPreview() const; + Windows::Foundation::IReference TabThemeColorPreview() const; til::typed_event DeleteProfileRequested; diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl index 2830e0b35c..41a9d1c68c 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl @@ -123,8 +123,8 @@ namespace Microsoft.Terminal.Settings.Editor String TabTitlePreview { get; }; String AnswerbackMessagePreview { get; }; - Windows.UI.Color TabColorPreview { get; }; - Windows.UI.Color TabThemeColorPreview { get; }; + Windows.Foundation.IReference TabColorPreview { get; }; + Windows.Foundation.IReference TabThemeColorPreview { get; }; void CreateUnfocusedAppearance(); void DeleteUnfocusedAppearance(); diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp b/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp index 88ba50e386..72937fc05f 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp @@ -34,6 +34,18 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _Profile = args.Profile(); _windowRoot = args.WindowRoot(); + _Profile.PropertyChanged([this](auto&&, const PropertyChangedEventArgs& args) { + const auto viewModelProperty{ args.PropertyName() }; + if (viewModelProperty == L"TabColorPreview") + { + // TODO CARLOS: When the CurrentValue changes to null (aka "use theme color" resolves to default XAML colors), + // the CurrentValueTemplateSelector should switch from using the ColorTemplate to the NullColorTemplate. + // Breakpoints in SelectTemplateCore() are not hit in this scenario (they are hit when set to not-null). + // Reloading the app with CurrentValue being null works fine. The issue is purely when swapping from a color to null. + TabColor().UpdateLayout(); + } + }); + // Check the use parent directory box if the starting directory is empty if (_Profile.StartingDirectory().empty()) { diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml b/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml index 813cada4c5..1d77771db8 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml @@ -209,19 +209,33 @@ + - + + + + + + + + + + + + + + diff --git a/src/cascadia/TerminalSettingsEditor/SettingContainer.cpp b/src/cascadia/TerminalSettingsEditor/SettingContainer.cpp index 2ab1614123..54670738b7 100644 --- a/src/cascadia/TerminalSettingsEditor/SettingContainer.cpp +++ b/src/cascadia/TerminalSettingsEditor/SettingContainer.cpp @@ -15,6 +15,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation DependencyProperty SettingContainer::_FontIconGlyphProperty{ nullptr }; DependencyProperty SettingContainer::_CurrentValueProperty{ nullptr }; DependencyProperty SettingContainer::_CurrentValueTemplateProperty{ nullptr }; + DependencyProperty SettingContainer::_CurrentValueTemplateSelectorProperty{ nullptr }; DependencyProperty SettingContainer::_CurrentValueAccessibleNameProperty{ nullptr }; DependencyProperty SettingContainer::_HasSettingValueProperty{ nullptr }; DependencyProperty SettingContainer::_SettingOverrideSourceProperty{ nullptr }; @@ -75,6 +76,15 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation xaml_typename(), PropertyMetadata{ nullptr }); } + if (!_CurrentValueTemplateSelectorProperty) + { + _CurrentValueTemplateSelectorProperty = + DependencyProperty::Register( + L"CurrentValueTemplateSelector", + xaml_typename(), + xaml_typename(), + PropertyMetadata{ nullptr }); + } if (!_CurrentValueAccessibleNameProperty) { _CurrentValueAccessibleNameProperty = diff --git a/src/cascadia/TerminalSettingsEditor/SettingContainer.h b/src/cascadia/TerminalSettingsEditor/SettingContainer.h index 9c77a838e0..0459b30270 100644 --- a/src/cascadia/TerminalSettingsEditor/SettingContainer.h +++ b/src/cascadia/TerminalSettingsEditor/SettingContainer.h @@ -38,6 +38,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation DEPENDENCY_PROPERTY(hstring, FontIconGlyph); DEPENDENCY_PROPERTY(Windows::Foundation::IInspectable, CurrentValue); DEPENDENCY_PROPERTY(Windows::UI::Xaml::DataTemplate, CurrentValueTemplate); + DEPENDENCY_PROPERTY(Windows::UI::Xaml::Controls::DataTemplateSelector, CurrentValueTemplateSelector); DEPENDENCY_PROPERTY(hstring, CurrentValueAccessibleName); DEPENDENCY_PROPERTY(bool, HasSettingValue); DEPENDENCY_PROPERTY(bool, StartExpanded); diff --git a/src/cascadia/TerminalSettingsEditor/SettingContainer.idl b/src/cascadia/TerminalSettingsEditor/SettingContainer.idl index 143d49b919..4c241313e9 100644 --- a/src/cascadia/TerminalSettingsEditor/SettingContainer.idl +++ b/src/cascadia/TerminalSettingsEditor/SettingContainer.idl @@ -24,6 +24,9 @@ namespace Microsoft.Terminal.Settings.Editor Windows.UI.Xaml.DataTemplate CurrentValueTemplate; static Windows.UI.Xaml.DependencyProperty CurrentValueTemplateProperty { get; }; + Windows.UI.Xaml.Controls.DataTemplateSelector CurrentValueTemplateSelector; + static Windows.UI.Xaml.DependencyProperty CurrentValueTemplateSelectorProperty { get; }; + String CurrentValueAccessibleName; static Windows.UI.Xaml.DependencyProperty CurrentValueAccessibleNameProperty { get; }; diff --git a/src/cascadia/TerminalSettingsEditor/SettingContainerStyle.xaml b/src/cascadia/TerminalSettingsEditor/SettingContainerStyle.xaml index 280dba68ad..246ae22769 100644 --- a/src/cascadia/TerminalSettingsEditor/SettingContainerStyle.xaml +++ b/src/cascadia/TerminalSettingsEditor/SettingContainerStyle.xaml @@ -461,7 +461,8 @@ HorizontalAlignment="Right" VerticalAlignment="Center" Content="{TemplateBinding CurrentValue}" - ContentTemplate="{TemplateBinding CurrentValueTemplate}" /> + ContentTemplate="{TemplateBinding CurrentValueTemplate}" + ContentTemplateSelector="{TemplateBinding CurrentValueTemplateSelector}" />