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.

Sign up for GitHub

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

Completely remove action dispatching from Command Palette #8628

Merged
3 commits merged into from
Jan 7, 2021
Merged

Completely remove action dispatching from Command Palette #8628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad28b1c
Remove action dispatching from Command Palette
Don-Vito Dec 20, 2020
9cd1df7
Cleanup redundant palette visibility checks
Don-Vito Dec 20, 2020
312028f
Decouple TerminalPage from AppLogic
Don-Vito Dec 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
56 changes: 20 additions & 36 deletions src/cascadia/TerminalApp/CommandPalette.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,18 +245,26 @@ namespace winrt::TerminalApp::implementation
// Only give anchored tab switcher the ability to cycle through tabs with the tab button.
// For unanchored mode, accessibility becomes an issue when we try to hijack tab since it's
// a really widely used keyboard navigation key.
if (_currentMode == CommandPaletteMode::TabSwitchMode && key == VirtualKey::Tab)
if (_currentMode == CommandPaletteMode::TabSwitchMode && _keymap)
{
auto const state = CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift);
if (WI_IsFlagSet(state, CoreVirtualKeyStates::Down))
{
SelectNextItem(false);
e.Handled(true);
}
else
auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

winrt::Microsoft::Terminal::TerminalControl::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast<int32_t>(key) };
const auto action = _keymap.TryLookup(kc);
if (action)
{
SelectNextItem(true);
e.Handled(true);
if (action.Action() == ShortcutAction::PrevTab)
{
SelectNextItem(false);
e.Handled(true);
}
else if (action.Action() == ShortcutAction::NextTab)
{
SelectNextItem(true);
e.Handled(true);
}
}
}
else if (key == VirtualKey::Home)
Expand Down Expand Up @@ -350,30 +358,6 @@ namespace winrt::TerminalApp::implementation

e.Handled(true);
}
else
{
const auto vkey = ::gsl::narrow_cast<WORD>(e.OriginalKey());

// In the interest of not telling all modes to check for keybindings, limit to TabSwitch mode for now.
if (_currentMode == CommandPaletteMode::TabSwitchMode)
{
auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

auto success = _bindings.TryKeyChord({
ctrlDown,
altDown,
shiftDown,
vkey,
});

if (success)
{
e.Handled(true);
}
}
}
}

// Method Description:
Expand Down Expand Up @@ -852,9 +836,9 @@ namespace winrt::TerminalApp::implementation
return _filteredActions;
}

void CommandPalette::SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings)
void CommandPalette::SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap)
{
_bindings = bindings;
_keymap = keymap;
}

void CommandPalette::SetCommands(Collections::IVector<Command> const& actions)
Expand Down
5 changes: 2 additions & 3 deletions src/cascadia/TerminalApp/CommandPalette.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace winrt::TerminalApp::implementation

void SetCommands(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& actions);
void SetTabs(Windows::Foundation::Collections::IVector<winrt::TerminalApp::TabBase> const& tabs, const bool clearList);
void SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings);
void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap);

void EnableCommandPaletteMode(Microsoft::Terminal::Settings::Model::CommandPaletteLaunchMode const launchMode);

Expand All @@ -47,7 +47,6 @@ namespace winrt::TerminalApp::implementation

// Tab Switcher
void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx);
void SetTabSwitchOrder(const Microsoft::Terminal::Settings::Model::TabSwitcherMode order);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
OBSERVABLE_GETSET_PROPERTY(winrt::hstring, NoMatchesText, _PropertyChangedHandlers);
Expand Down Expand Up @@ -109,7 +108,7 @@ namespace winrt::TerminalApp::implementation
std::wstring _getTrimmedInput();
void _evaluatePrefix();

Microsoft::Terminal::TerminalControl::IKeyBindings _bindings;
Microsoft::Terminal::Settings::Model::KeyMapping _keymap{ nullptr };

// Tab Switcher
Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> _tabActions{ nullptr };
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CommandPalette.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace TerminalApp

void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
void SetTabs(Windows.Foundation.Collections.IVector<TabBase> tabs, Boolean clearList);
void SetKeyBindings(Microsoft.Terminal.TerminalControl.IKeyBindings bindings);
void SetKeyMap(Microsoft.Terminal.Settings.Model.KeyMapping keymap);
void EnableCommandPaletteMode(Microsoft.Terminal.Settings.Model.CommandPaletteLaunchMode launchMode);

void SelectNextItem(Boolean moveDown);
Expand Down
17 changes: 3 additions & 14 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "pch.h"
#include "TerminalPage.h"
#include "Utils.h"
#include "AppLogic.h"
#include "../../types/inc/utils.hpp"

#include <LibraryResources.h>
Expand Down Expand Up @@ -108,7 +107,7 @@ namespace winrt::TerminalApp::implementation
if (auto page{ weakThis.get() })
{
_UpdateCommandsForPalette();
CommandPalette().SetKeyBindings(*_bindings);
CommandPalette().SetKeyMap(_settings.KeyMap());
}
}

Expand Down Expand Up @@ -943,9 +942,7 @@ namespace winrt::TerminalApp::implementation
auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

winrt::Microsoft::Terminal::TerminalControl::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast<int32_t>(key) };
auto setting = AppLogic::CurrentAppSettings();
auto keymap = setting.GlobalSettings().KeyMap();
const auto actionAndArgs = keymap.TryLookup(kc);
const auto actionAndArgs = _settings.KeyMap().TryLookup(kc);
if (actionAndArgs)
{
if (CommandPalette().Visibility() == Visibility::Visible && actionAndArgs.Action() != ShortcutAction::ToggleCommandPalette)
Expand Down Expand Up @@ -1319,14 +1316,6 @@ namespace winrt::TerminalApp::implementation
// - Sets focus to the tab to the right or left the currently selected tab.
void TerminalPage::_SelectNextTab(const bool bMoveRight)
{
if (CommandPalette().Visibility() == Visibility::Visible)
{
// If the tab switcher is currently open, don't change its mode.
// Just select the new tab.
CommandPalette().SelectNextItem(bMoveRight);
return;
}

const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode();
const bool useInOrderTabIndex = tabSwitchMode != TabSwitcherMode::MostRecentlyUsed;

Expand Down Expand Up @@ -2234,7 +2223,7 @@ namespace winrt::TerminalApp::implementation
// here, then the user won't be able to navigate the ATS any
// longer.
//
// When the tab swither is eventually dismissed, the focus will
// When the tab switcher is eventually dismissed, the focus will
// get tossed back to the focused terminal control, so we don't
// need to worry about focus getting lost.
if (CommandPalette().Visibility() != Visibility::Visible)
Expand Down