From 988dad37e5f41ab40fc73b8876492a8f5378e917 Mon Sep 17 00:00:00 2001 From: Floris Westerman Date: Thu, 29 Jul 2021 00:05:32 +0200 Subject: [PATCH] Passing through moveFocus keys when moving to another pane failed (#10806) Implementation of #6219 with a small tweak, not just passing the keys when no panes are present, but passing on the keys when there is no other pane to move to. This enables another usecase: 2 panes in terminal split vertically; in one of these panes running tmux with two panes that are split horizontally. This allows the user to still navigate between tmux panes even though they have terminal panes open. Not that I know of * [x] Closes #6219 * [x] CLA signed. * [x] Tests added/passed * [x] Documentation updated. I don't think that's necessary * [x] Schema updated. N/A * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Implementation by propagating the boolean indicating success of moving focus all the way to the action handler, where this result will determine whether the action will be considered handled or not. When the action is not handled, the keychord will be propagated to the terminal. Manual testing; all relevant unit tests still work --- src/cascadia/TerminalApp/AppActionHandlers.cpp | 7 +++++-- src/cascadia/TerminalApp/TerminalPage.cpp | 8 +++++--- src/cascadia/TerminalApp/TerminalPage.h | 2 +- src/cascadia/TerminalApp/TerminalTab.cpp | 13 +++++++++---- src/cascadia/TerminalApp/TerminalTab.h | 2 +- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index cdc92aaf6ae..7ad847b1d24 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -307,8 +307,11 @@ namespace winrt::TerminalApp::implementation } else { - _MoveFocus(realArgs.FocusDirection()); - args.Handled(true); + // Mark as handled only when the move succeeded (e.g. when there + // is a pane to move to), otherwise mark as unhandled so the + // keychord can propagate to the terminal (GH#6129) + const auto moveSucceeded = _MoveFocus(realArgs.FocusDirection()); + args.Handled(moveSucceeded); } } } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index a5eb4c5ddf7..fa3372f169d 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1101,14 +1101,16 @@ namespace winrt::TerminalApp::implementation // Arguments: // - direction: The direction to move the focus in. // Return Value: - // - - void TerminalPage::_MoveFocus(const FocusDirection& direction) + // - Whether changing the focus succeeded. This allows a keychord to propagate + // to the terminal when no other panes are present (GH#6219) + bool TerminalPage::_MoveFocus(const FocusDirection& direction) { if (const auto terminalTab{ _GetFocusedTabImpl() }) { _UnZoomIfNeeded(); - terminalTab->NavigateFocus(direction); + return terminalTab->NavigateFocus(direction); } + return false; } TermControl TerminalPage::_GetActiveControl() diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 306e44c7335..b68e765b530 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -230,7 +230,7 @@ namespace winrt::TerminalApp::implementation void _SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference& customTabSwitcherMode); bool _SelectTab(const uint32_t tabIndex); - void _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); + bool _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); winrt::Microsoft::Terminal::Control::TermControl _GetActiveControl(); std::optional _GetFocusedTabIndex() const noexcept; diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 4a09839407a..dc3d17f2ac6 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -481,19 +481,24 @@ namespace winrt::TerminalApp::implementation // Arguments: // - direction: The direction to move the focus in. // Return Value: - // - - void TerminalTab::NavigateFocus(const FocusDirection& direction) + // - Whether changing the focus succeeded. This allows a keychord to propagate + // to the terminal when no other panes are present (GH#6219) + bool TerminalTab::NavigateFocus(const FocusDirection& direction) { if (direction == FocusDirection::Previous) { + if (_mruPanes.size() < 2) + { + return false; + } // To get to the previous pane, get the id of the previous pane and focus to that - _rootPane->FocusPane(_mruPanes.at(1)); + return _rootPane->FocusPane(_mruPanes.at(1)); } else { // NOTE: This _must_ be called on the root pane, so that it can propagate // throughout the entire tree. - _rootPane->NavigateFocus(direction); + return _rootPane->NavigateFocus(direction); } } diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index 7167f74a67c..225b03857b9 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -52,7 +52,7 @@ namespace winrt::TerminalApp::implementation void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction); - void NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); + bool NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); bool FocusPane(const uint32_t id); void UpdateSettings(const Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings, const GUID& profile);