Skip to content

Commit

Permalink
Redesign color schemes page (#9196)
Browse files Browse the repository at this point in the history 
This PR performs a large overall polish of the color schemes page:
- Ensures keyboard navigation is holistically improved (i.e. fully
  accessible, no lost focus, etc...)
- Adds tooltips and automation properties to all controls
- Redesigns the page according to @mdtauk's approved design
  ([link](#8997 (comment))).
  Note, there are some minor modifications to the design that were
  approved by @cinnamon-msft.
- Automatically reflow's the color buttons when they do not fit in
  horizontal mode

## Detailed Description of the Pull Request / Additional comments
- Redesign
  - a data template was introduced to make color representation
    consistent and straightforward
  - `ContentControl` is used to hold a reference to the
    `ColorTableEntry` and represent it properly using the aforementioned
    data template.
  - The design is mainly a StackPanel holding two grids: color table &
    functional colors.
  - The color table is populated via code. After much thought, this
    seems to be the easiest way to correctly bind 16 controls that are
    very similar.
  - The functional colors are populated via XAML manually.
  - We need a grid to separate the text and the buttons. This allows for
    scenarios like "selection background is the longest string" to force
    the buttons and text to be aligned.
- Reflow
  - A `VisualStateManager` uses an `AdaptiveTrigger` to change the
    orientation of the color tables' stack panel. The adaptive trigger
    was carefully calculated to align with the navigation view's
    breakpoint.
- Keyboard Navigation
  - (a lesson from `SettingContainer`) `ContentControl` can be focused
    as opposed to the content inside of it. We don't want that, so we
    set `IsTabStop` to false on it. That basically fixes all of our
    keyboard navigation issues in this new design.
- Automation Properties and ToolTips
  - As in my previous PRs, I can't seem to figure out how to bind to a
    control's automation property to its own tooltip. So I had to do
    this in the code and add a few `x:Name` around.

## Validation Steps Performed
- Manually tested...
  - tab navigation
  - accessibility insights
  - NVDA
  - changing color schemes updates color table
- specific scenario:
  - change a color table color and a functional color
  - navigate to a different color scheme
  - navigate back to the first color scheme
  - if the colors persist, the changes were propagated to the settings model

References #8997 - Based on the work from @Chips1234
References #6800 - Settings UI Epic
Closes #8765 - Polish Color Schemes page
Closes #8899 - Automation Properties
Closes #8768 - Keyboard Navigation
  • Loading branch information
@carlos-zamora
carlos-zamora committed Feb 19, 2021
1 parent 2c22b68 commit bb0e1d3
Show file tree
Hide file tree
Showing 5 changed files with 353 additions and 239 deletions.
142 changes: 131 additions & 11 deletions src/cascadia/TerminalSettingsEditor/ColorSchemes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ using namespace winrt::Windows::Foundation::Collections;

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
// The first 8 entries of the color table are non-bright colors, whereas the rest are bright.
static constexpr uint8_t ColorTableDivider{ 8 };

static constexpr std::wstring_view ForegroundColorTag{ L"Foreground" };
static constexpr std::wstring_view BackgroundColorTag{ L"Background" };
static constexpr std::wstring_view CursorColorTag{ L"CursorColor" };
static constexpr std::wstring_view SelectionBackgroundColorTag{ L"SelectionBackground" };

static const std::array<hstring, 16> TableColorNames = {
RS_(L"ColorScheme_Black/Header"),
RS_(L"ColorScheme_Red/Header"),
Expand Down Expand Up @@ -53,9 +61,25 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

ColorSchemes::ColorSchemes() :
_ColorSchemeList{ single_threaded_observable_vector<Model::ColorScheme>() },
_CurrentColorTable{ single_threaded_observable_vector<Editor::ColorTableEntry>() }
_CurrentNonBrightColorTable{ single_threaded_observable_vector<Editor::ColorTableEntry>() },
_CurrentBrightColorTable{ single_threaded_observable_vector<Editor::ColorTableEntry>() }
{
InitializeComponent();

Automation::AutomationProperties::SetName(ColorSchemeComboBox(), RS_(L"ColorScheme_Name/Header"));
Automation::AutomationProperties::SetFullDescription(ColorSchemeComboBox(), RS_(L"ColorScheme_Name/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip"));
ToolTipService::SetToolTip(ColorSchemeComboBox(), box_value(RS_(L"ColorScheme_Name/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip")));

Automation::AutomationProperties::SetName(RenameButton(), RS_(L"Rename/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip"));

Automation::AutomationProperties::SetName(NameBox(), RS_(L"ColorScheme_Name/Header"));
Automation::AutomationProperties::SetFullDescription(NameBox(), RS_(L"ColorScheme_Name/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip"));
ToolTipService::SetToolTip(NameBox(), box_value(RS_(L"ColorScheme_Name/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip")));

Automation::AutomationProperties::SetName(RenameAcceptButton(), RS_(L"RenameAccept/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip"));
Automation::AutomationProperties::SetName(RenameCancelButton(), RS_(L"RenameCancel/[using:Windows.UI.Xaml.Controls]ToolTipService/ToolTip"));
Automation::AutomationProperties::SetName(AddNewButton(), RS_(L"ColorScheme_AddNewButton/Text"));
Automation::AutomationProperties::SetName(DeleteButton(), RS_(L"ColorScheme_DeleteButton/Text"));
}

void ColorSchemes::OnNavigatedTo(const NavigationEventArgs& e)
Expand All @@ -70,9 +94,20 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// very accurately.
for (uint8_t i = 0; i < TableColorNames.size(); ++i)
{
auto entry = winrt::make<ColorTableEntry>(i, Windows::UI::Color{ 0, 0, 0, 0 });
_CurrentColorTable.Append(entry);
const auto& entry{ winrt::make<ColorTableEntry>(i, Windows::UI::Color{ 0, 0, 0, 0 }) };
if (i < ColorTableDivider)
{
_CurrentNonBrightColorTable.Append(entry);
}
else
{
_CurrentBrightColorTable.Append(entry);
}
}
_CurrentForegroundColor = winrt::make<ColorTableEntry>(ForegroundColorTag, Windows::UI::Color{ 0, 0, 0, 0 });
_CurrentBackgroundColor = winrt::make<ColorTableEntry>(BackgroundColorTag, Windows::UI::Color{ 0, 0, 0, 0 });
_CurrentCursorColor = winrt::make<ColorTableEntry>(CursorColorTag, Windows::UI::Color{ 0, 0, 0, 0 });
_CurrentSelectionBackgroundColor = winrt::make<ColorTableEntry>(SelectionBackgroundColorTag, Windows::UI::Color{ 0, 0, 0, 0 });

// Try to look up the scheme that was navigated to. If we find it, immediately select it.
const std::wstring lastNameFromNav{ _State.LastSelectedScheme() };
Expand All @@ -85,6 +120,41 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
auto scheme = *it;
ColorSchemeComboBox().SelectedItem(scheme);
}

// populate color table grid
const auto colorLabelStyle{ Resources().Lookup(winrt::box_value(L"ColorLabelStyle")).as<Windows::UI::Xaml::Style>() };
const auto colorControlStyle{ Resources().Lookup(winrt::box_value(L"ColorControlStyle")).as<Windows::UI::Xaml::Style>() };
const auto colorTableEntryTemplate{ Resources().Lookup(winrt::box_value(L"ColorTableEntryTemplate")).as<DataTemplate>() };
auto setupColorControl = [colorTableEntryTemplate, colorControlStyle, colorTableGrid{ ColorTableGrid() }](const auto&& colorRef, const uint32_t& row, const uint32_t& col) {
ContentControl colorControl{};
colorControl.ContentTemplate(colorTableEntryTemplate);
colorControl.Style(colorControlStyle);

Data::Binding binding{};
binding.Source(colorRef);
binding.Mode(Data::BindingMode::TwoWay);
colorControl.SetBinding(ContentControl::ContentProperty(), binding);

colorTableGrid.Children().Append(colorControl);
Grid::SetRow(colorControl, row);
Grid::SetColumn(colorControl, col);
};
for (uint32_t row = 0; row < ColorTableGrid().RowDefinitions().Size(); ++row)
{
// color label
TextBlock label{};
label.Text(TableColorNames[row]);
label.Style(colorLabelStyle);
ColorTableGrid().Children().Append(label);
Grid::SetRow(label, row);
Grid::SetColumn(label, 0);

// regular color
setupColorControl(_CurrentNonBrightColorTable.GetAt(row), row, 1);

// bright color
setupColorControl(_CurrentBrightColorTable.GetAt(row), row, 2);
}
}

// Function Description:
Expand Down Expand Up @@ -149,20 +219,52 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void ColorSchemes::ColorPickerChanged(IInspectable const& sender,
ColorChangedEventArgs const& args)
{
if (auto picker = sender.try_as<ColorPicker>())
if (const auto& picker{ sender.try_as<ColorPicker>() })
{
if (auto tag = picker.Tag())
if (const auto& tag{ picker.Tag() })
{
auto index = winrt::unbox_value<uint8_t>(tag);
CurrentColorScheme().SetColorTableEntry(index, args.NewColor());
_CurrentColorTable.GetAt(index).Color(args.NewColor());
if (const auto index{ tag.try_as<uint8_t>() })
{
CurrentColorScheme().SetColorTableEntry(*index, args.NewColor());
if (index < ColorTableDivider)
{
_CurrentNonBrightColorTable.GetAt(*index).Color(args.NewColor());
}
else
{
_CurrentBrightColorTable.GetAt(*index - ColorTableDivider).Color(args.NewColor());
}
}
else if (const auto stringTag{ tag.try_as<hstring>() })
{
if (stringTag == ForegroundColorTag)
{
CurrentColorScheme().Foreground(args.NewColor());
_CurrentForegroundColor.Color(args.NewColor());
}
else if (stringTag == BackgroundColorTag)
{
CurrentColorScheme().Background(args.NewColor());
_CurrentBackgroundColor.Color(args.NewColor());
}
else if (stringTag == CursorColorTag)
{
CurrentColorScheme().CursorColor(args.NewColor());
_CurrentCursorColor.Color(args.NewColor());
}
else if (stringTag == SelectionBackgroundColorTag)
{
CurrentColorScheme().SelectionBackground(args.NewColor());
_CurrentSelectionBackgroundColor.Color(args.NewColor());
}
}
}
}
}

bool ColorSchemes::CanDeleteCurrentScheme() const
{
if (const auto scheme{ CurrentColorScheme() })
if (const auto& scheme{ CurrentColorScheme() })
{
// Only allow this color scheme to be deleted if it's not provided in-box
const std::wstring myName{ scheme.Name() };
Expand Down Expand Up @@ -295,14 +397,32 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
for (uint8_t i = 0; i < TableColorNames.size(); ++i)
{
_CurrentColorTable.GetAt(i).Color(colorScheme.Table()[i]);
if (i < ColorTableDivider)
{
_CurrentNonBrightColorTable.GetAt(i).Color(colorScheme.Table()[i]);
}
else
{
_CurrentBrightColorTable.GetAt(i - ColorTableDivider).Color(colorScheme.Table()[i]);
}
}
_CurrentForegroundColor.Color(colorScheme.Foreground());
_CurrentBackgroundColor.Color(colorScheme.Background());
_CurrentCursorColor.Color(colorScheme.CursorColor());
_CurrentSelectionBackgroundColor.Color(colorScheme.SelectionBackground());
}

ColorTableEntry::ColorTableEntry(uint8_t index, Windows::UI::Color color)
{
Name(TableColorNames[index]);
Index(winrt::box_value<uint8_t>(index));
Tag(winrt::box_value<uint8_t>(index));
Color(color);
}

ColorTableEntry::ColorTableEntry(std::wstring_view tag, Windows::UI::Color color)
{
Name(LocalizedNameForEnumName(L"ColorScheme_", tag, L"Text"));
Tag(winrt::box_value(tag));
Color(color);
}
}
12 changes: 9 additions & 3 deletions src/cascadia/TerminalSettingsEditor/ColorSchemes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void DeleteConfirmation_Click(winrt::Windows::Foundation::IInspectable const& sender, winrt::Windows::UI::Xaml::RoutedEventArgs const& e);

GETSET_PROPERTY(Editor::ColorSchemesPageNavigationState, State, nullptr);
GETSET_PROPERTY(Windows::Foundation::Collections::IObservableVector<winrt::Microsoft::Terminal::Settings::Editor::ColorTableEntry>, CurrentColorTable, nullptr);
GETSET_PROPERTY(Model::ColorScheme, CurrentColorScheme, nullptr);
GETSET_PROPERTY(Windows::Foundation::Collections::IVector<Editor::ColorTableEntry>, CurrentNonBrightColorTable, nullptr);
GETSET_PROPERTY(Windows::Foundation::Collections::IVector<Editor::ColorTableEntry>, CurrentBrightColorTable, nullptr);
GETSET_PROPERTY(Windows::Foundation::Collections::IObservableVector<Model::ColorScheme>, ColorSchemeList, nullptr);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
OBSERVABLE_GETSET_PROPERTY(winrt::Microsoft::Terminal::Settings::Model::ColorScheme, CurrentColorScheme, _PropertyChangedHandlers, nullptr);
OBSERVABLE_GETSET_PROPERTY(bool, IsRenaming, _PropertyChangedHandlers, nullptr);
OBSERVABLE_GETSET_PROPERTY(Editor::ColorTableEntry, CurrentForegroundColor, _PropertyChangedHandlers, nullptr);
OBSERVABLE_GETSET_PROPERTY(Editor::ColorTableEntry, CurrentBackgroundColor, _PropertyChangedHandlers, nullptr);
OBSERVABLE_GETSET_PROPERTY(Editor::ColorTableEntry, CurrentCursorColor, _PropertyChangedHandlers, nullptr);
OBSERVABLE_GETSET_PROPERTY(Editor::ColorTableEntry, CurrentSelectionBackgroundColor, _PropertyChangedHandlers, nullptr);

private:
void _UpdateColorTable(const winrt::Microsoft::Terminal::Settings::Model::ColorScheme& colorScheme);
Expand All @@ -56,10 +61,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
public:
ColorTableEntry(uint8_t index, Windows::UI::Color color);
ColorTableEntry(std::wstring_view tag, Windows::UI::Color color);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Name, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(IInspectable, Index, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(IInspectable, Tag, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(Windows::UI::Color, Color, _PropertyChangedHandlers);
};
}
Expand Down
15 changes: 12 additions & 3 deletions src/cascadia/TerminalSettingsEditor/ColorSchemes.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,24 @@ namespace Microsoft.Terminal.Settings.Editor
Boolean CanDeleteCurrentScheme { get; };
Boolean IsRenaming { get; };

Microsoft.Terminal.Settings.Model.ColorScheme CurrentColorScheme { get; };
Windows.Foundation.Collections.IObservableVector<ColorTableEntry> CurrentColorTable;
// Terminal Colors
Windows.Foundation.Collections.IVector<ColorTableEntry> CurrentNonBrightColorTable { get; };
Windows.Foundation.Collections.IVector<ColorTableEntry> CurrentBrightColorTable { get; };

// System Colors
ColorTableEntry CurrentForegroundColor;
ColorTableEntry CurrentBackgroundColor;
ColorTableEntry CurrentCursorColor;
ColorTableEntry CurrentSelectionBackgroundColor;


Windows.Foundation.Collections.IObservableVector<Microsoft.Terminal.Settings.Model.ColorScheme> ColorSchemeList { get; };
}

[default_interface] runtimeclass ColorTableEntry : Windows.UI.Xaml.Data.INotifyPropertyChanged
{
String Name { get; };
IInspectable Index;
IInspectable Tag;
Windows.UI.Color Color;
}
}
Loading

0 comments on commit bb0e1d3

Please sign in to comment.