Skip to content

Commit 38d2fda

Browse files
Replace NullableColorPicker ContentDialog with Flyout (#19572)
## 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
1 parent 1ca0c76 commit 38d2fda

File tree

3 files changed

+12
-32
lines changed

3 files changed

+12
-32
lines changed

src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
159159
CurrentColor(nullptr);
160160
}
161161

162-
safe_void_coroutine NullableColorPicker::MoreColors_Clicked(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/)
163-
{
164-
co_await ColorPickerDialog().ShowAsync();
165-
}
166-
167-
void NullableColorPicker::ColorPickerDialog_Opened(const IInspectable& /*sender*/, const ContentDialogOpenedEventArgs& /*args*/)
162+
void NullableColorPicker::Flyout_Opening(const IInspectable& /*sender*/, const IInspectable& /*args*/)
168163
{
169164
// Initialize color picker with current color
170165
if (CurrentColor())
@@ -185,7 +180,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
185180
}
186181
}
187182

188-
void NullableColorPicker::ColorPickerDialog_PrimaryButtonClick(const IInspectable& /*sender*/, const ContentDialogButtonClickEventArgs& /*args*/)
183+
void NullableColorPicker::Flyout_Closing(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::Controls::Primitives::FlyoutBaseClosingEventArgs& /*args*/)
189184
{
190185
const auto& selectedColor = ColorPickerControl().Color();
191186
const Microsoft::Terminal::Core::Color terminalColor{ selectedColor.R, selectedColor.G, selectedColor.B, selectedColor.A };

src/cascadia/TerminalSettingsEditor/NullableColorPicker.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
2222
void ColorChip_DataContextChanged(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::DataContextChangedEventArgs& args);
2323

2424
void NullColorButton_Clicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args);
25-
safe_void_coroutine MoreColors_Clicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args);
26-
27-
void ColorPickerDialog_Opened(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::ContentDialogOpenedEventArgs& args);
28-
void ColorPickerDialog_PrimaryButtonClick(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs& args);
25+
void Flyout_Opening(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args);
26+
void Flyout_Closing(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::Primitives::FlyoutBaseClosingEventArgs& args);
2927

3028
DEPENDENCY_PROPERTY(Editor::ColorSchemeViewModel, ColorSchemeVM);
3129
DEPENDENCY_PROPERTY(Windows::Foundation::IReference<Microsoft::Terminal::Core::Color>, CurrentColor);

src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -97,25 +97,6 @@
9797
<StackPanel x:Name="ContentStackPanel"
9898
Orientation="Horizontal"
9999
Spacing="5">
100-
<ContentDialog x:Name="ColorPickerDialog"
101-
x:Uid="NullableColorPicker_ColorPickerContentDialog"
102-
DefaultButton="Primary"
103-
Opened="ColorPickerDialog_Opened"
104-
PrimaryButtonClick="ColorPickerDialog_PrimaryButtonClick"
105-
TabFocusNavigation="Cycle">
106-
<muxc:ColorPicker x:Name="ColorPickerControl"
107-
Margin="0,0,0,-40"
108-
ColorSpectrumShape="Box"
109-
IsAlphaEnabled="False"
110-
IsAlphaSliderVisible="True"
111-
IsAlphaTextInputVisible="True"
112-
IsColorChannelTextInputVisible="False"
113-
IsColorSliderVisible="False"
114-
IsHexInputVisible="False"
115-
IsMoreButtonVisible="False"
116-
Orientation="Horizontal" />
117-
</ContentDialog>
118-
119100
<ContentPresenter Content="{x:Bind ColorSchemeVM, Mode=OneWay}"
120101
ContentTemplate="{StaticResource ColorSchemeTemplate}" />
121102

@@ -146,8 +127,14 @@
146127
</ToggleButton>
147128

148129
<Button x:Uid="NullableColorPicker_MoreColorsButton"
149-
HorizontalAlignment="Stretch"
150-
Click="MoreColors_Clicked" />
130+
HorizontalAlignment="Stretch">
131+
<Button.Flyout>
132+
<Flyout Closing="Flyout_Closing"
133+
Opening="Flyout_Opening">
134+
<muxc:ColorPicker x:Name="ColorPickerControl" />
135+
</Flyout>
136+
</Button.Flyout>
137+
</Button>
151138
</StackPanel>
152139
</Grid>
153140

0 commit comments

Comments
 (0)