Skip to content

Commit

Permalink
Add setTabColor and openTabColorPicker actions (#6567)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

Adds a pair of `ShortcutAction`s for setting the tab color.
* `setTabColor`: This changes the color of the current tab to the provided color, or can be used to clear the color.
* `openTabColorPicker`: This keybinding immediately activates the tab color picker for the currently focused tab.

## References

## PR Checklist
* [x] scratches my own itch
* [x] I work here
* [x] Tests added/passed
* [x] MicrosoftDocs/terminal#69

## Detailed Description of the Pull Request / Additional comments

## Validation Steps Performed
* hey look there are tests
* Tested with the following:
```json

        // { "command": "setTabColor", "keys": [ "alt+c" ] },
        { "keys": "ctrl+alt+c", "command": { "action": "setTabColor", "color": "#123456" } },
        { "keys": "alt+shift+c", "command": { "action": "setTabColor", "color": null} },
        { "keys": "alt+c", "command": "openTabColorPicker" },
```
  • Loading branch information
zadjii-msft authored Jun 25, 2020
1 parent 9215b52 commit a3a9df8
Show file tree
Hide file tree
Showing 15 changed files with 207 additions and 13 deletions.
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
19 changes: 19 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
"switchToTab",
"toggleFullscreen",
"find",
"setTabColor",
"openTabColorPicker",
"unbound"
],
"type": "string"
Expand Down Expand Up @@ -257,6 +259,22 @@
}
]
},
"SetTabColorAction": {
"description": "Arguments corresponding to a Set Tab Color Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "setTabColor" },
"color": {
"$ref": "#/definitions/Color",
"default": null,
"description": "If provided, will set the tab's color to the given value. If omitted, will reset the tab's color."
}
}
}
]
},
"Keybinding": {
"additionalProperties": false,
"properties": {
Expand All @@ -272,6 +290,7 @@
{ "$ref": "#/definitions/ResizePaneAction" },
{ "$ref": "#/definitions/SplitPaneAction" },
{ "$ref": "#/definitions/OpenSettingsAction" },
{ "$ref": "#/definitions/SetTabColorAction" },
{ "type": "null" }
]
},
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" } },
{ "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" };
static constexpr std::string_view RenameTabKey{ "renameTab" };

namespace winrt::TerminalApp::implementation
Expand Down Expand Up @@ -69,6 +71,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 },
{ RenameTabKey, ShortcutAction::RenameTab }
Expand Down Expand Up @@ -99,6 +103,8 @@ namespace winrt::TerminalApp::implementation

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

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

{ ShortcutAction::RenameTab, winrt::TerminalApp::implementation::RenameTabArgs::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,4 +15,5 @@
#include "AdjustFontSizeArgs.g.cpp"
#include "SplitPaneArgs.g.cpp"
#include "OpenSettingsArgs.g.cpp"
#include "SetTabColorArgs.g.cpp"
#include "RenameTabArgs.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,10 +15,12 @@
#include "AdjustFontSizeArgs.g.h"
#include "SplitPaneArgs.g.h"
#include "OpenSettingsArgs.g.h"
#include "SetTabColorArgs.g.h"
#include "RenameTabArgs.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 @@ -443,6 +445,41 @@ namespace winrt::TerminalApp::implementation
}
};

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, {} };
}
};

struct RenameTabArgs : public RenameTabArgsT<RenameTabArgs>
{
RenameTabArgs() = default;
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 @@ -103,6 +103,11 @@ namespace TerminalApp
SettingsTarget Target { get; };
};

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

[default_interface] runtimeclass RenameTabArgs : IActionArgs
{
String Title { get; };
Expand Down
40 changes: 39 additions & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,45 @@ namespace winrt::TerminalApp::implementation
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);
}

void TerminalPage::_HandleRenameTab(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
Expand All @@ -262,5 +301,4 @@ namespace winrt::TerminalApp::implementation
}
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;
}
case ShortcutAction::RenameTab:
{
_RenameTabHandlers(*this, *eventArgs);
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);
TYPED_EVENT(RenameTab, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
// clang-format on

Expand Down
8 changes: 4 additions & 4 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,
RenameTab
};
Expand Down Expand Up @@ -74,7 +73,8 @@ 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;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> RenameTab;

}
}
21 changes: 16 additions & 5 deletions src/cascadia/TerminalApp/Tab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,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 @@ -530,14 +530,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 @@ -718,7 +718,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 @@ -779,7 +779,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
// Return Value:
// - <none>
void Tab::_ResetTabColor()
void Tab::ResetTabColor()
{
auto weakThis{ get_weak() };

Expand Down Expand Up @@ -817,6 +817,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
Loading

0 comments on commit a3a9df8

Please sign in to comment.