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

Provide global setting to use ATS for nextTab and prevTab #7321

Merged
22 commits merged into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b8e9b3b
for science - nt/pt hijacked, unanchored ats now tabSearch
leonMSFT Aug 13, 2020
e4de51a
change keyup to select on any modifier release
leonMSFT Aug 13, 2020
4fd1862
add global setting to swap between boring tab switching and exciting …
leonMSFT Aug 13, 2020
43241cf
now that we don't have a designated anchor key, change the var to anc…
leonMSFT Aug 13, 2020
e15672e
go back to anchorKey, priority set anchorkey at appactionhandlers level
leonMSFT Aug 14, 2020
33a513c
detect modifiers pressed at handlenT/handlepT, do boring switch if no…
leonMSFT Aug 14, 2020
9d188f0
let OnDirectKeyEvent find parents of ItemTemplated elements, give cmd…
leonMSFT Aug 17, 2020
c62bfb2
separate mode into tabswitch and tabsearch
leonMSFT Aug 17, 2020
9e8d92a
Merge branch 'master' into dev/lelian/im-the-captain-now
leonMSFT Aug 17, 2020
433ec74
cue don't let your dreams be dreams meme
leonMSFT Aug 17, 2020
3799be2
a bit more cleanup
leonMSFT Aug 17, 2020
9306f0d
schema updated
leonMSFT Aug 17, 2020
c293055
a whole lotta thinking resulting in just one line changed
leonMSFT Aug 18, 2020
b43342e
i didn't realize i was in the spelling bee
leonMSFT Aug 18, 2020
ebff42c
removing setting from control
leonMSFT Aug 19, 2020
859544a
merge master
leonMSFT Aug 19, 2020
c74f29c
might as well move this here, it's a bit cleaner
leonMSFT Aug 19, 2020
5f9a25f
value or
leonMSFT Aug 19, 2020
1553354
pr updates
leonMSFT Aug 20, 2020
e820bc5
merge conflicts
leonMSFT Aug 21, 2020
054219a
missed this one
leonMSFT Aug 21, 2020
7f1adff
Merge remote-tracking branch 'origin/master' into dev/lelian/im-the-c…
zadjii-msft Aug 21, 2020
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
19 changes: 1 addition & 18 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"wt",
"closeOtherTabs",
"closeTabsAfter",
"tabSwitcher",
Copy link
Member

Choose a reason for hiding this comment

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

useTabSwitcher probably needs to get added to this file too

"tabSearch",
"unbound"
],
"type": "string"
Expand Down Expand Up @@ -422,22 +422,6 @@
],
"required": [ "index" ]
},
"TabSwitcherAction": {
"description": "Arguments corresponding to a Tab Switcher Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "tabSwitcher" },
"anchorKey": {
"$ref": "#/definitions/AnchorKey",
"default": null,
"description": "If provided, the tab switcher will stay open as long as the anchor key is held down. The anchor key should be part of the keybinding that opens the switcher."
}
}
}
]
},
"Keybinding": {
"additionalProperties": false,
"properties": {
Expand All @@ -459,7 +443,6 @@
{ "$ref": "#/definitions/WtAction" },
{ "$ref": "#/definitions/CloseOtherTabsAction" },
{ "$ref": "#/definitions/CloseTabsAfterAction" },
{ "$ref": "#/definitions/TabSwitcherAction" },
{ "type": "null" }
]
},
Expand Down
7 changes: 3 additions & 4 deletions src/cascadia/TerminalApp/ActionAndArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static constexpr std::string_view ExecuteCommandlineKey{ "wt" };
static constexpr std::string_view ToggleCommandPaletteKey{ "commandPalette" };
static constexpr std::string_view CloseOtherTabsKey{ "closeOtherTabs" };
static constexpr std::string_view CloseTabsAfterKey{ "closeTabsAfter" };
static constexpr std::string_view ToggleTabSwitcherKey{ "tabSwitcher" };
static constexpr std::string_view TabSearchKey{ "tabSearch" };

static constexpr std::string_view ActionKey{ "action" };

Expand Down Expand Up @@ -103,7 +103,7 @@ namespace winrt::TerminalApp::implementation
{ ToggleCommandPaletteKey, ShortcutAction::ToggleCommandPalette },
{ CloseOtherTabsKey, ShortcutAction::CloseOtherTabs },
{ CloseTabsAfterKey, ShortcutAction::CloseTabsAfter },
{ ToggleTabSwitcherKey, ShortcutAction::ToggleTabSwitcher },
{ TabSearchKey, ShortcutAction::TabSearch },
};

using ParseResult = std::tuple<IActionArgs, std::vector<::TerminalApp::SettingsLoadWarnings>>;
Expand Down Expand Up @@ -145,8 +145,6 @@ namespace winrt::TerminalApp::implementation

{ ShortcutAction::CloseTabsAfter, winrt::TerminalApp::implementation::CloseTabsAfterArgs::FromJson },

{ ShortcutAction::ToggleTabSwitcher, winrt::TerminalApp::implementation::ToggleTabSwitcherArgs::FromJson },

carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{ ShortcutAction::Invalid, nullptr },
};

Expand Down Expand Up @@ -301,6 +299,7 @@ namespace winrt::TerminalApp::implementation
{ ShortcutAction::ToggleCommandPalette, RS_(L"ToggleCommandPaletteCommandKey") },
{ ShortcutAction::CloseOtherTabs, L"" }, // Intentionally omitted, must be generated by GenerateName
{ ShortcutAction::CloseTabsAfter, L"" }, // Intentionally omitted, must be generated by GenerateName
{ ShortcutAction::TabSearch, RS_(L"TabSearchCommandKey") },
};
}();

Expand Down
19 changes: 0 additions & 19 deletions src/cascadia/TerminalApp/ActionArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "SetTabColorArgs.g.cpp"
#include "RenameTabArgs.g.cpp"
#include "ExecuteCommandlineArgs.g.cpp"
#include "ToggleTabSwitcherArgs.g.h"

#include "Utils.h"

Expand Down Expand Up @@ -355,22 +354,4 @@ namespace winrt::TerminalApp::implementation
_Index)
};
}

winrt::hstring ToggleTabSwitcherArgs::GenerateName() const
{
// If there's an anchor key set, don't generate a name so that
// it won't show up in the command palette. Only an unanchored
// tab switcher should be able to be toggled from the palette.
// TODO: GH#7179 - once this goes in, make sure to hide the
// anchor mode command that was given a name in settings.
if (_AnchorKey != Windows::System::VirtualKey::None)
{
return L"";
}
else
{
return RS_(L"ToggleTabSwitcherCommandKey");
}
}

}
29 changes: 0 additions & 29 deletions src/cascadia/TerminalApp/ActionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "ExecuteCommandlineArgs.g.h"
#include "CloseOtherTabsArgs.g.h"
#include "CloseTabsAfterArgs.g.h"
#include "ToggleTabSwitcherArgs.g.h"

#include "../../cascadia/inc/cppwinrt_utils.h"
#include "Utils.h"
Expand Down Expand Up @@ -548,34 +547,6 @@ namespace winrt::TerminalApp::implementation
return { *args, {} };
}
};

struct ToggleTabSwitcherArgs : public ToggleTabSwitcherArgsT<ToggleTabSwitcherArgs>
{
ToggleTabSwitcherArgs() = default;
GETSET_PROPERTY(Windows::System::VirtualKey, AnchorKey, Windows::System::VirtualKey::None);

static constexpr std::string_view AnchorJsonKey{ "anchorKey" };

public:
hstring GenerateName() const;

bool Equals(const IActionArgs& other)
{
auto otherAsUs = other.try_as<ToggleTabSwitcherArgs>();
if (otherAsUs)
{
return otherAsUs->_AnchorKey == _AnchorKey;
}
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<ToggleTabSwitcherArgs>();
JsonUtils::GetValueForKey(json, AnchorJsonKey, args->_AnchorKey);
return { *args, {} };
}
};
}

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

[default_interface] runtimeclass ToggleTabSwitcherArgs : IActionArgs
{
Windows.System.VirtualKey AnchorKey { get; };
};
}
24 changes: 6 additions & 18 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,27 +456,15 @@ namespace winrt::TerminalApp::implementation
}
}

void TerminalPage::_HandleToggleTabSwitcher(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
void TerminalPage::_HandleOpenTabSearch(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::ToggleTabSwitcherArgs>())
{
auto anchorKey = realArgs.AnchorKey();
auto opt = _GetFocusedTabIndex();
uint32_t startIdx = opt.value_or(0);

auto opt = _GetFocusedTabIndex();
uint32_t startIdx = opt ? *opt : 0;
CommandPalette().EnableTabSwitcherMode(true, startIdx);
CommandPalette().Visibility(Visibility::Visible);

if (anchorKey != VirtualKey::None)
{
// TODO: GH#7178 - delta should also have the option of being -1, in the case when
// a user decides to open the tab switcher going to the prev tab.
int delta = 1;
startIdx = (startIdx + _tabs.Size() + delta) % _tabs.Size();
}

CommandPalette().EnableTabSwitcherMode(anchorKey, startIdx);
CommandPalette().Visibility(Visibility::Visible);
}
args.Handled(true);
}
}
7 changes: 7 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,13 @@ namespace winrt::TerminalApp::implementation
if (auto focusedElement{ focusedObject.try_as<Windows::UI::Xaml::FrameworkElement>() })
{
focusedObject = focusedElement.Parent();

// Parent() seems to return null when the focusedElement is created from an ItemTemplate.
// Use the VisualTreeHelper's GetParent as a fallback.
if (!focusedObject)
{
focusedObject = winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetParent(focusedElement);
Copy link
Member

Choose a reason for hiding this comment

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

clever!

Copy link
Member

Choose a reason for hiding this comment

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

i'm surprised this mostly just worked -- binding the direct handler, that is :)

}
}
else
{
Expand Down
Loading