From 38d2fdad5f7549aa10d5a0974bc2b805cf011dec Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 24 Nov 2025 16:21:49 -0800 Subject: [PATCH] Replace NullableColorPicker ContentDialog with Flyout (#19572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request Updates the NullableColorPicker to use a flyout instead of a content dialog. Frankly, it should've been this way from the start. #19561 is an issue regarding the rectangle on the right side of the picker. The complaint being that it should be something more useful than a preview, an idea being that it could be a lightness gradient. Unfortunately, the WinUI color picker doesn't let you do that. It's just a plain preview. That said, there's a lot of customizations that can be added still to increase value here. To name a few: - IsColorSliderVisible --> a color slider to adjust the lightness of the color (as desired in #19561) - IsHexInputVisible --> an input field to see and adjust the hex value directly - IsColorChannelTextInputVisible --> several input fields to adjust individual RGB channels or switch over to HSV However, the content dialog doesn't allow for text input due to a WinUI bug and it's too small to display all of those controls. Instead, I just discarded the content dialog altogether and opted into a flyout. This makes it a more consistent experience with the other color pickers (i.e. tab color, edit color scheme page). This also adds space for all of the functionality mentioned above (those properties are enabled by default). ## Validation Steps Performed ✅ selecting a color still works Closes #19561 --- .../NullableColorPicker.cpp | 9 ++---- .../NullableColorPicker.h | 6 ++-- .../NullableColorPicker.xaml | 29 +++++-------------- 3 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp index 4cb9f655b7..7996535c97 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp @@ -159,12 +159,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation CurrentColor(nullptr); } - safe_void_coroutine NullableColorPicker::MoreColors_Clicked(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/) - { - co_await ColorPickerDialog().ShowAsync(); - } - - void NullableColorPicker::ColorPickerDialog_Opened(const IInspectable& /*sender*/, const ContentDialogOpenedEventArgs& /*args*/) + void NullableColorPicker::Flyout_Opening(const IInspectable& /*sender*/, const IInspectable& /*args*/) { // Initialize color picker with current color if (CurrentColor()) @@ -185,7 +180,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } } - void NullableColorPicker::ColorPickerDialog_PrimaryButtonClick(const IInspectable& /*sender*/, const ContentDialogButtonClickEventArgs& /*args*/) + void NullableColorPicker::Flyout_Closing(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::Controls::Primitives::FlyoutBaseClosingEventArgs& /*args*/) { const auto& selectedColor = ColorPickerControl().Color(); const Microsoft::Terminal::Core::Color terminalColor{ selectedColor.R, selectedColor.G, selectedColor.B, selectedColor.A }; diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h index 876ce23d7f..04ce05aa32 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h @@ -22,10 +22,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void ColorChip_DataContextChanged(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::DataContextChangedEventArgs& args); void NullColorButton_Clicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args); - safe_void_coroutine MoreColors_Clicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args); - - void ColorPickerDialog_Opened(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::ContentDialogOpenedEventArgs& args); - void ColorPickerDialog_PrimaryButtonClick(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs& args); + void Flyout_Opening(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args); + void Flyout_Closing(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::Primitives::FlyoutBaseClosingEventArgs& args); DEPENDENCY_PROPERTY(Editor::ColorSchemeViewModel, ColorSchemeVM); DEPENDENCY_PROPERTY(Windows::Foundation::IReference, CurrentColor); diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml index 6755543311..941f29b5e3 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml @@ -97,25 +97,6 @@ - - - - @@ -146,8 +127,14 @@