From 8564b269c4b757e9da4a20b2613e80d287fcb76d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 11 May 2021 18:55:49 -0500 Subject: [PATCH] In specific scenarios, focus the active control (#10048) A redo of #6290. That PR was overkill. In that one, we'd toss focus back to the active control any time that the tab view item got focus. That's maybe not the _best_ solution. Instead, this PR is precision strikes. We're re-using a lot of what we already have from #9260. * When the context menu is closed, yeet focus to the control. * When the renamer is dismissed, yeet focus to the control. * When the TabViewItem is tapped (meaning no one else handled it), yeet focus to the control. ### checklist * [x] I work here * [ ] This is UI so it doesn't have tests * [x] Closes #3609 * [x] Closes #5750 * [x] Closes #6680 ### scenarios: * [x] focus the window by clicking on the tab -> Control is focused. * [x] Open the color picker with the context menu, can move the focus inside the picker with the arrow keys. * [x] Dismiss the picker with esc -> Control is focused. * [x] Dismiss the picker with enter -> Control is focused. * [x] Dismiss the renamer with esc -> Control is focused. * [x] Dismiss the renamer with enter -> Control is focused. * [x] Dismiss the context menu with esc -> Control is focused. * [x] Start renaming, then click on the tab -> Rename is committed, Control is focused. * [x] Start renaming, then click on the text box -> focus is still in the text box --- src/cascadia/TerminalApp/SettingsTab.cpp | 3 +- src/cascadia/TerminalApp/SettingsTab.h | 2 +- src/cascadia/TerminalApp/TabBase.cpp | 34 ++++++++++++++++++++-- src/cascadia/TerminalApp/TabBase.h | 4 +++ src/cascadia/TerminalApp/TabManagement.cpp | 4 ++- src/cascadia/TerminalApp/TerminalTab.cpp | 34 +++++++++++++++++----- src/cascadia/TerminalApp/TerminalTab.h | 3 +- 7 files changed, 68 insertions(+), 16 deletions(-) diff --git a/src/cascadia/TerminalApp/SettingsTab.cpp b/src/cascadia/TerminalApp/SettingsTab.cpp index df2cf7a869..b0319c4d4e 100644 --- a/src/cascadia/TerminalApp/SettingsTab.cpp +++ b/src/cascadia/TerminalApp/SettingsTab.cpp @@ -62,7 +62,8 @@ namespace winrt::TerminalApp::implementation // - void SettingsTab::_MakeTabViewItem() { - TabViewItem(::winrt::MUX::Controls::TabViewItem{}); + TabBase::_MakeTabViewItem(); + Title(RS_(L"SettingsTab")); TabViewItem().Header(winrt::box_value(Title())); } diff --git a/src/cascadia/TerminalApp/SettingsTab.h b/src/cascadia/TerminalApp/SettingsTab.h index 221214eff4..acf4249445 100644 --- a/src/cascadia/TerminalApp/SettingsTab.h +++ b/src/cascadia/TerminalApp/SettingsTab.h @@ -31,7 +31,7 @@ namespace winrt::TerminalApp::implementation void Focus(winrt::Windows::UI::Xaml::FocusState focusState) override; private: - void _MakeTabViewItem(); + void _MakeTabViewItem() override; winrt::fire_and_forget _CreateIcon(); }; } diff --git a/src/cascadia/TerminalApp/TabBase.cpp b/src/cascadia/TerminalApp/TabBase.cpp index a7a973801d..ef17fe3eb9 100644 --- a/src/cascadia/TerminalApp/TabBase.cpp +++ b/src/cascadia/TerminalApp/TabBase.cpp @@ -46,9 +46,17 @@ namespace winrt::TerminalApp::implementation auto weakThis{ get_weak() }; // Build the menu - Controls::MenuFlyout newTabFlyout; - _AppendCloseMenuItems(newTabFlyout); - TabViewItem().ContextFlyout(newTabFlyout); + Controls::MenuFlyout contextMenuFlyout; + // GH#5750 - When the context menu is dismissed with ESC, toss the focus + // back to our control. + contextMenuFlyout.Closed([weakThis](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_RequestFocusActiveControlHandlers(); + } + }); + _AppendCloseMenuItems(contextMenuFlyout); + TabViewItem().ContextFlyout(contextMenuFlyout); } // Method Description: @@ -223,4 +231,24 @@ namespace winrt::TerminalApp::implementation toolTip.Content(textBlock); WUX::Controls::ToolTipService::SetToolTip(TabViewItem(), toolTip); } + + // Method Description: + // - Initializes a TabViewItem for this Tab instance. + // Arguments: + // - + // Return Value: + // - + void TabBase::_MakeTabViewItem() + { + TabViewItem(::winrt::MUX::Controls::TabViewItem{}); + + // GH#3609 If the tab was tapped, and no one else was around to handle + // it, then ask our parent to toss focus into the active control. + TabViewItem().Tapped([weakThis{ get_weak() }](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_RequestFocusActiveControlHandlers(); + } + }); + } } diff --git a/src/cascadia/TerminalApp/TabBase.h b/src/cascadia/TerminalApp/TabBase.h index 5129513333..8efa12550d 100644 --- a/src/cascadia/TerminalApp/TabBase.h +++ b/src/cascadia/TerminalApp/TabBase.h @@ -25,6 +25,8 @@ namespace winrt::TerminalApp::implementation void UpdateTabViewIndex(const uint32_t idx, const uint32_t numTabs); void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap); + WINRT_CALLBACK(RequestFocusActiveControl, winrt::delegate); + WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); WINRT_CALLBACK(CloseRequested, winrt::Windows::Foundation::EventHandler); WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); @@ -52,6 +54,8 @@ namespace winrt::TerminalApp::implementation virtual void _CreateContextMenu(); virtual winrt::hstring _CreateToolTipTitle(); + virtual void _MakeTabViewItem(); + void _AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout); void _EnableCloseMenuItems(); void _CloseTabsAfter(); diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index e774935ab7..699c3f5bff 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -215,7 +215,9 @@ namespace winrt::TerminalApp::implementation } }); - newTabImpl->TabRenamerDeactivated([weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) { + // The tab might want us to toss focus into the control, especially when + // transient UIs (like the context menu, or the renamer) are dismissed. + newTabImpl->RequestFocusActiveControl([weakThis{ get_weak() }]() { if (const auto page{ weakThis.get() }) { page->_FocusCurrentTab(false); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index b61a2eb605..b807315148 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -53,6 +53,15 @@ namespace winrt::TerminalApp::implementation } }); + // GH#9162 - when the header is done renaming, ask for focus to be + // tossed back to the control, rather into ourselves. + _headerControl.RenameEnded([weakThis = get_weak()](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_RequestFocusActiveControlHandlers(); + } + }); + _UpdateHeaderControlMaxWidth(); // Use our header control as the TabViewItem's header @@ -83,7 +92,7 @@ namespace winrt::TerminalApp::implementation // - void TerminalTab::_MakeTabViewItem() { - TabViewItem(::winrt::MUX::Controls::TabViewItem{}); + TabBase::_MakeTabViewItem(); TabViewItem().DoubleTapped([weakThis = get_weak()](auto&& /*s*/, auto&& /*e*/) { if (auto tab{ weakThis.get() }) @@ -878,14 +887,23 @@ namespace winrt::TerminalApp::implementation } // Build the menu - Controls::MenuFlyout newTabFlyout; + Controls::MenuFlyout contextMenuFlyout; Controls::MenuFlyoutSeparator menuSeparator; - newTabFlyout.Items().Append(chooseColorMenuItem); - newTabFlyout.Items().Append(renameTabMenuItem); - newTabFlyout.Items().Append(duplicateTabMenuItem); - newTabFlyout.Items().Append(menuSeparator); - _AppendCloseMenuItems(newTabFlyout); - TabViewItem().ContextFlyout(newTabFlyout); + contextMenuFlyout.Items().Append(chooseColorMenuItem); + contextMenuFlyout.Items().Append(renameTabMenuItem); + contextMenuFlyout.Items().Append(duplicateTabMenuItem); + contextMenuFlyout.Items().Append(menuSeparator); + + // GH#5750 - When the context menu is dismissed with ESC, toss the focus + // back to our control. + contextMenuFlyout.Closed([weakThis](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_RequestFocusActiveControlHandlers(); + } + }); + _AppendCloseMenuItems(contextMenuFlyout); + TabViewItem().ContextFlyout(contextMenuFlyout); } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index b248dab350..6aba27dbda 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -91,7 +91,6 @@ namespace winrt::TerminalApp::implementation DECLARE_EVENT(ColorCleared, _colorCleared, winrt::delegate<>); DECLARE_EVENT(TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>); DECLARE_EVENT(DuplicateRequested, _DuplicateRequestedHandlers, winrt::delegate<>); - FORWARDED_TYPED_EVENT(TabRenamerDeactivated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, (&_headerControl), RenameEnded); TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable); private: @@ -120,7 +119,7 @@ namespace winrt::TerminalApp::implementation std::optional _bellIndicatorTimer; void _BellIndicatorTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e); - void _MakeTabViewItem(); + void _MakeTabViewItem() override; winrt::fire_and_forget _UpdateHeaderControlMaxWidth();