Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setTabColor and openTabColorPicker actions #6567

Merged
5 commits merged into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spell-check/expect/alphabet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ abcdefghijklmnop
ABCDEFGHIJKLMNOPQRST
abcdefghijklmnopqrstuvwxyz
ABE
BBGGRR
BBBBBBBBBBBBBBDDDD
QQQQQQQQQQABCDEFGHIJ
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQ
Expand Down
59 changes: 59 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ namespace TerminalAppLocalTests

TEST_METHOD(TestStringOverload);

TEST_METHOD(TestSetTabColorArgs);

TEST_CLASS_SETUP(ClassSetup)
{
InitializeJsonReader();
Expand Down Expand Up @@ -400,6 +402,63 @@ namespace TerminalAppLocalTests
}
}

void KeyBindingsTests::TestSetTabColorArgs()
{
const std::string bindings0String{ R"([
{ "keys": ["ctrl+c"], "command": { "action": "setTabColor", "color": null } },
{ "keys": ["ctrl+d"], "command": { "action": "setTabColor", "color": "#123456" } },
{ "keys": ["ctrl+e"], "command": { "action": "setTabColor", "color": "thisStringObviouslyWontWork" } },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsonUtilsNew will make this an Exception (!)

{ "keys": ["ctrl+f"], "command": "setTabColor" },
])" };

const auto bindings0Json = VerifyParseSucceeded(bindings0String);

auto appKeyBindings = winrt::make_self<implementation::AppKeyBindings>();
VERIFY_IS_NOT_NULL(appKeyBindings);
VERIFY_ARE_EQUAL(0u, appKeyBindings->_keyShortcuts.size());
appKeyBindings->LayerJson(bindings0Json);
VERIFY_ARE_EQUAL(4u, appKeyBindings->_keyShortcuts.size());

{
KeyChord kc{ true, false, false, static_cast<int32_t>('C') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SetTabColorArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_NULL(realArgs.TabColor());
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('D') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SetTabColorArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_NOT_NULL(realArgs.TabColor());
// Remember that COLORREFs are actually BBGGRR order, while the string is in #RRGGBB order
VERIFY_ARE_EQUAL(static_cast<uint32_t>(til::color(0x563412)), realArgs.TabColor().Value());
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('E') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SetTabColorArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_NULL(realArgs.TabColor());
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('F') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SetTabColorArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_NULL(realArgs.TabColor());
}
}

void KeyBindingsTests::TestStringOverload()
{
const std::string bindings0String{ R"([
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalApp/ActionAndArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ static constexpr std::string_view ResizePaneKey{ "resizePane" };
static constexpr std::string_view MoveFocusKey{ "moveFocus" };
static constexpr std::string_view FindKey{ "find" };
static constexpr std::string_view ToggleFullscreenKey{ "toggleFullscreen" };
static constexpr std::string_view SetTabColorKey{ "setTabColor" };
static constexpr std::string_view OpenTabColorPickerKey{ "openTabColorPicker" };

namespace winrt::TerminalApp::implementation
{
Expand Down Expand Up @@ -68,6 +70,8 @@ namespace winrt::TerminalApp::implementation
{ OpenSettingsKey, ShortcutAction::OpenSettings },
{ ToggleFullscreenKey, ShortcutAction::ToggleFullscreen },
{ SplitPaneKey, ShortcutAction::SplitPane },
{ SetTabColorKey, ShortcutAction::SetTabColor },
{ OpenTabColorPickerKey, ShortcutAction::OpenTabColorPicker },
{ UnboundKey, ShortcutAction::Invalid },
{ FindKey, ShortcutAction::Find }
};
Expand Down Expand Up @@ -97,6 +101,8 @@ namespace winrt::TerminalApp::implementation

{ ShortcutAction::OpenSettings, winrt::TerminalApp::implementation::OpenSettingsArgs::FromJson },

{ ShortcutAction::SetTabColor, winrt::TerminalApp::implementation::SetTabColorArgs::FromJson },

{ ShortcutAction::Invalid, nullptr },
};

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/ActionArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
#include "AdjustFontSizeArgs.g.cpp"
#include "SplitPaneArgs.g.cpp"
#include "OpenSettingsArgs.g.cpp"
#include "SetTabColorArgs.g.cpp"
37 changes: 37 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
#include "AdjustFontSizeArgs.g.h"
#include "SplitPaneArgs.g.h"
#include "OpenSettingsArgs.g.h"
#include "SetTabColorArgs.g.h"

#include "../../cascadia/inc/cppwinrt_utils.h"
#include "Utils.h"
#include "JsonUtils.h"
#include "TerminalWarnings.h"

// Notes on defining ActionArgs and ActionEventArgs:
Expand Down Expand Up @@ -441,6 +443,41 @@ namespace winrt::TerminalApp::implementation
return { *args, {} };
}
};

struct SetTabColorArgs : public SetTabColorArgsT<SetTabColorArgs>
{
SetTabColorArgs() = default;
GETSET_PROPERTY(Windows::Foundation::IReference<uint32_t>, TabColor, nullptr);

static constexpr std::string_view ColorKey{ "color" };

public:
bool Equals(const IActionArgs& other)
{
auto otherAsUs = other.try_as<SetTabColorArgs>();
if (otherAsUs)
{
return otherAsUs->_TabColor == _TabColor;
}
return false;
};
static FromJsonResult FromJson(const Json::Value& json)
{
// LOAD BEARING: Not using make_self here _will_ break you in the future!
auto args = winrt::make_self<SetTabColorArgs>();
std::optional<til::color> temp;
try
{
::TerminalApp::JsonUtils::GetOptionalColor(json, ColorKey, temp);
if (temp.has_value())
{
args->_TabColor = static_cast<uint32_t>(temp.value());
}
}
CATCH_LOG();
return { *args, {} };
}
};
}

namespace winrt::TerminalApp::factory_implementation
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,9 @@ namespace TerminalApp
{
SettingsTarget Target { get; };
};

[default_interface] runtimeclass SetTabColorArgs : IActionArgs
{
Windows.Foundation.IReference<UInt32> TabColor { get; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh uint32 this is gonna make JsonUtilsNew fun 😦

};
}
39 changes: 39 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,43 @@ namespace winrt::TerminalApp::implementation
ToggleFullscreen();
args.Handled(true);
}

void TerminalPage::_HandleSetTabColor(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
std::optional<til::color> tabColor;

if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::SetTabColorArgs>())
{
if (realArgs.TabColor() != nullptr)
{
tabColor = realArgs.TabColor().Value();
}
}

auto activeTab = _GetFocusedTab();
if (activeTab)
{
if (tabColor.has_value())
{
activeTab->SetTabColor(tabColor.value());
}
else
{
activeTab->ResetTabColor();
}
}
args.Handled(true);
}

void TerminalPage::_HandleOpenTabColorPicker(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
auto activeTab = _GetFocusedTab();
if (activeTab)
{
activeTab->ActivateColorPicker();
}
args.Handled(true);
}
}
10 changes: 10 additions & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,16 @@ namespace winrt::TerminalApp::implementation
_ToggleFullscreenHandlers(*this, *eventArgs);
break;
}
case ShortcutAction::SetTabColor:
{
_SetTabColorHandlers(*this, *eventArgs);
break;
}
case ShortcutAction::OpenTabColorPicker:
{
_OpenTabColorPickerHandlers(*this, *eventArgs);
break;
}
default:
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ namespace winrt::TerminalApp::implementation
TYPED_EVENT(Find, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(MoveFocus, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(ToggleFullscreen, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(SetTabColor, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(OpenTabColorPicker,TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
// clang-format on

private:
Expand Down
7 changes: 4 additions & 3 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.idl
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ import "../ActionArgs.idl";

namespace TerminalApp
{
// TODO: GH#1069 - Many of these shortcut actions are "legacy" now that we
// have support for arbitrary args (#1142). We should remove them, and our
// legacy deserializers.
enum ShortcutAction
{
Invalid = 0,
Expand Down Expand Up @@ -35,6 +32,8 @@ namespace TerminalApp
MoveFocus,
Find,
ToggleFullscreen,
SetTabColor,
OpenTabColorPicker,
OpenSettings
};

Expand Down Expand Up @@ -73,5 +72,7 @@ namespace TerminalApp
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> Find;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> MoveFocus;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> ToggleFullscreen;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> SetTabColor;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> OpenTabColorPicker;
}
}
21 changes: 16 additions & 5 deletions src/cascadia/TerminalApp/Tab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ namespace winrt::TerminalApp::implementation
chooseColorMenuItem.Click([weakThis](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_tabColorPickup.ShowAt(tab->_tabViewItem);
tab->ActivateColorPicker();
}
});
chooseColorMenuItem.Text(RS_(L"TabColorChoose"));
Expand All @@ -509,14 +509,14 @@ namespace winrt::TerminalApp::implementation
_tabColorPickup.ColorSelected([weakThis](auto newTabColor) {
if (auto tab{ weakThis.get() })
{
tab->_SetTabColor(newTabColor);
tab->SetTabColor(newTabColor);
}
});

_tabColorPickup.ColorCleared([weakThis]() {
if (auto tab{ weakThis.get() })
{
tab->_ResetTabColor();
tab->ResetTabColor();
}
});

Expand Down Expand Up @@ -697,7 +697,7 @@ namespace winrt::TerminalApp::implementation
// - color: the shiny color the user picked for their tab
// Return Value:
// - <none>
void Tab::_SetTabColor(const winrt::Windows::UI::Color& color)
void Tab::SetTabColor(const winrt::Windows::UI::Color& color)
{
auto weakThis{ get_weak() };

Expand Down Expand Up @@ -758,7 +758,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
// Return Value:
// - <none>
void Tab::_ResetTabColor()
void Tab::ResetTabColor()
{
auto weakThis{ get_weak() };

Expand Down Expand Up @@ -796,6 +796,17 @@ namespace winrt::TerminalApp::implementation
});
}

// Method Description:
// - Display the tab color picker at the location of the TabViewItem for this tab.
// Arguments:
// - <none>
// Return Value:
// - <none>
void Tab::ActivateColorPicker()
{
_tabColorPickup.ShowAt(_tabViewItem);
}

// Method Description:
// Toggles the visual state of the tab view item,
// so that changes to the tab color are reflected immediately
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/Tab.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ namespace winrt::TerminalApp::implementation

std::optional<winrt::Windows::UI::Color> GetTabColor();

void SetTabColor(const winrt::Windows::UI::Color& color);
void ResetTabColor();
void ActivateColorPicker();

WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
DECLARE_EVENT(ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>);
Expand Down Expand Up @@ -80,8 +84,6 @@ namespace winrt::TerminalApp::implementation
void _Focus();

void _CreateContextMenu();
void _SetTabColor(const winrt::Windows::UI::Color& color);
void _ResetTabColor();
void _RefreshVisualState();

void _BindEventHandlers(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control) noexcept;
Expand Down
14 changes: 14 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,8 @@ namespace winrt::TerminalApp::implementation
_actionDispatch->Find({ this, &TerminalPage::_HandleFind });
_actionDispatch->ResetFontSize({ this, &TerminalPage::_HandleResetFontSize });
_actionDispatch->ToggleFullscreen({ this, &TerminalPage::_HandleToggleFullscreen });
_actionDispatch->SetTabColor({ this, &TerminalPage::_HandleSetTabColor });
_actionDispatch->OpenTabColorPicker({ this, &TerminalPage::_HandleOpenTabColorPicker });
}

// Method Description:
Expand Down Expand Up @@ -1096,6 +1098,18 @@ namespace winrt::TerminalApp::implementation
return std::nullopt;
}

// Method Description:
// - returns a com_ptr to the currently focused tab. This might return null,
// so make sure to check the result!
winrt::com_ptr<Tab> TerminalPage::_GetFocusedTab()
{
if (auto index{ _GetFocusedTabIndex() })
{
return _GetStrongTabImpl(*index);
}
return nullptr;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}

// Method Description:
// - An async method for changing the focused tab on the UI thread. This
// method will _only_ set the selected item of the TabView, which will
Expand Down
Loading