maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Hook up the keybindings to the SUI, redux (#10121)

Browse source 
## Summary of the Pull Request

This is a redux of #8882. 

From the original:

>  This is really similar to what we're doing with the `CommandPalette`. We're adding a ~~Preview~~`KeyDown` handler to the SUI `MainPage`, that connects to `TerminalPage::_HandleKey`. That allows the SUI a chance to search the keymap to dispatch actions for keybindings, similar to how the command palette does it. 
> 
> This also means it's now possible for the SUI to invoke _all_ the actions available to the Terminal. This includes the ones like `IncreaseFontSize`, which require a _Terminal_ to actually do something. So we have to make sure all the calls to `_GetActiveControl` actually check that the result is non-null before using it. 
> 
> A bunch of the actions do nothing now from a SUI tab, others behave _weird_. Like "Rename tab" / "Open Tab Renamer" do nothing. "Duplicate Tab" again does nothing - we try making a new settings tab, which just focuses the settings tab again. "Copy text" definitely does nothing, same with paste.

I don't know why I thought this wouldn't work. I thought we'd have to do this in `PreviewKeyDown` or something, which led to [weirdness](https://github.com/microsoft/terminal/pull/8882#issuecomment-767088554). Turns out, we don't need it to be in `PreviewKeyDown`. It can just be in the SUI's `KeyDown`.

## References
* Original: #8882
* Workaround was in #8885


## PR Checklist
* [x] Closes #8767
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

The special case handler from #8885 is no longer needed

## Validation Steps Performed

* Switching tabs with Ctrl+Tab works
* Command palette works
* fullscreen, focus mode works
* close window works
* copy paste on Ctrl+C/V works, even when bound
* Select all text in textboxes works
* tab navigation through UI elements works
This commit is contained in:
Mike Griese 2021-05-24 14:18:38 -05:00 committed by GitHub
parent 0d61466afe
commit 52560ff818
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
src/cascadia/TerminalApp/AppActionHandlers.cpp
View file

@ -117,9 +117,11 @@ namespace winrt::TerminalApp::implementation
}
else if (const auto& realArgs = args.ActionArgs().try_as<SendInputArgs>())
{
const auto termControl = _GetActiveControl();
termControl.SendInput(realArgs.Input());
args.Handled(true);
if (const auto termControl{ _GetActiveControl() })
{
termControl.SendInput(realArgs.Input());
args.Handled(true);
}
}
}
@ -308,9 +310,11 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& realArgs = args.ActionArgs().try_as<AdjustFontSizeArgs>())
{
const auto termControl = _GetActiveControl();
termControl.AdjustFontSize(realArgs.Delta());
args.Handled(true);
if (const auto& termControl{ _GetActiveControl() })
{
termControl.AdjustFontSize(realArgs.Delta());
args.Handled(true);
}
}
}
@ -324,17 +328,21 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleResetFontSize(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
const auto termControl = _GetActiveControl();
termControl.ResetFontSize();
args.Handled(true);
if (const auto& termControl{ _GetActiveControl() })
{
termControl.ResetFontSize();
args.Handled(true);
}
}
void TerminalPage::_HandleToggleShaderEffects(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
const auto termControl = _GetActiveControl();
termControl.ToggleShaderEffects();
args.Handled(true);
if (const auto& termControl{ _GetActiveControl() })
{
termControl.ToggleShaderEffects();
args.Handled(true);
}
}
void TerminalPage::_HandleToggleFocusMode(const IInspectable& /*sender*/,

56
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -897,30 +897,6 @@ namespace winrt::TerminalApp::implementation
}
}
// Method Description:
// Handles preview key on the SUI tab, by handling close tab / next tab / previous tab
// This is a temporary solution - we need to fix all key-bindings work from SUI as long as they don't harm
// the SUI behavior
// Arguments:
// - e: the KeyRoutedEventArgs containing info about the keystroke.
// Return Value:
// - <none>
void TerminalPage::_SUIPreviewKeyDownHandler(Windows::Foundation::IInspectable const& /*sender*/, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
auto key = e.OriginalKey();
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::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast<int32_t>(key) };
const auto cmd{ _settings.ActionMap().GetActionByKeyChord(kc) };
if (cmd && (cmd.ActionAndArgs().Action() == ShortcutAction::CloseTab || cmd.ActionAndArgs().Action() == ShortcutAction::NextTab || cmd.ActionAndArgs().Action() == ShortcutAction::PrevTab || cmd.ActionAndArgs().Action() == ShortcutAction::ClosePane))
{
_actionDispatch->DoAction(cmd.ActionAndArgs());
e.Handled(true);
}
}
// Method Description:
// - Configure the AppKeyBindings to use our ShortcutActionDispatch and the updated ActionMap
// as the object to handle dispatching ShortcutAction events.
@ -1287,10 +1263,12 @@ namespace winrt::TerminalApp::implementation
// Do nothing if for some reason, there's no terminal tab in focus. We don't want to crash.
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
const auto control = _GetActiveControl();
const auto termHeight = control.ViewHeight();
auto scrollDelta = _ComputeScrollDelta(scrollDirection, termHeight);
terminalTab->Scroll(scrollDelta);
if (const auto& control{ _GetActiveControl() })
{
const auto termHeight = control.ViewHeight();
auto scrollDelta = _ComputeScrollDelta(scrollDirection, termHeight);
terminalTab->Scroll(scrollDelta);
}
}
}
@ -1699,8 +1677,11 @@ namespace winrt::TerminalApp::implementation
// - true iff we we able to copy text (if a selection was active)
bool TerminalPage::_CopyText(const bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats)
{
const auto control = _GetActiveControl();
return control.CopySelectionToClipboard(singleLine, formats);
if (const auto& control{ _GetActiveControl() })
{
return control.CopySelectionToClipboard(singleLine, formats);
}
return false;
}
// Method Description:
@ -1718,8 +1699,10 @@ namespace winrt::TerminalApp::implementation
// - Paste text from the Windows Clipboard to the focused terminal
void TerminalPage::_PasteText()
{
const auto control = _GetActiveControl();
control.PasteTextFromClipboard();
if (const auto& control{ _GetActiveControl() })
{
control.PasteTextFromClipboard();
}
}
// Function Description:
@ -2053,8 +2036,10 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalPage::_Find()
{
const auto termControl = _GetActiveControl();
termControl.CreateSearchBoxControl();
if (const auto& control{ _GetActiveControl() })
{
control.CreateSearchBoxControl();
}
}
// Method Description:
@ -2332,7 +2317,8 @@ namespace winrt::TerminalApp::implementation
sui.SetHostingWindow(reinterpret_cast<uint64_t>(*_hostingHwnd));
}
sui.PreviewKeyDown({ this, &TerminalPage::_SUIPreviewKeyDownHandler });
// GH#8767 - let unhandled keys in the SUI try to run commands too.
sui.KeyDown({ this, &TerminalPage::_KeyDownHandler });
sui.OpenJson([weakThis{ get_weak() }](auto&& /*s*/, winrt::Microsoft::Terminal::Settings::Model::SettingsTarget e) {
if (auto page{ weakThis.get() })

1
src/cascadia/TerminalApp/TerminalPage.h
View file

@ -199,7 +199,6 @@ namespace winrt::TerminalApp::implementation
void _ThirdPartyNoticesOnClick(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
void _KeyDownHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);
void _SUIPreviewKeyDownHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);
void _HookupKeyBindings(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) noexcept;
void _RegisterActionCallbacks();