From 042f9ef354f8495af181e49ab9d5c2f20d58d528 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. * [x] I work here * [ ] This is UI so it doesn't have tests * [x] Closes #3609 * [x] Closes #5750 * [x] Closes #6680 * [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 (cherry picked from commit 8564b269c4b757e9da4a20b2613e80d287fcb76d) --- 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/TerminalPage.cpp | 4 ++- src/cascadia/TerminalApp/TerminalTab.cpp | 33 +++++++++++++++++----- src/cascadia/TerminalApp/TerminalTab.h | 3 +- 7 files changed, 68 insertions(+), 15 deletions(-) diff --git a/src/cascadia/TerminalApp/SettingsTab.cpp b/src/cascadia/TerminalApp/SettingsTab.cpp index 3aa39c240edb..f46c04f8487e 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 1c83fc243ba5..908b122f5b15 100644 --- a/src/cascadia/TerminalApp/SettingsTab.h +++ b/src/cascadia/TerminalApp/SettingsTab.h @@ -33,7 +33,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 5b9498bb399a..a69d1a49bac5 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: @@ -225,4 +233,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 d5bdcb34fa2d..77bc41332344 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 SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap); + WINRT_CALLBACK(RequestFocusActiveControl, winrt::delegate); + WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); @@ -51,6 +53,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/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index e7f5b01fa3d1..13708bc00226 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -866,7 +866,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() }) { if (!page->_newTabButton.Flyout().IsOpen()) diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 4c7b7dcd4698..bc69e1925efd 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() }) @@ -832,13 +841,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(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 dbe36e9957eb..0a82882b8fa1 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -89,7 +89,6 @@ namespace winrt::TerminalApp::implementation DECLARE_EVENT(ColorSelected, _colorSelected, winrt::delegate); DECLARE_EVENT(ColorCleared, _colorCleared, winrt::delegate<>); DECLARE_EVENT(TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>); - FORWARDED_TYPED_EVENT(TabRenamerDeactivated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, (&_headerControl), RenameEnded); private: std::shared_ptr _rootPane{ nullptr }; @@ -117,7 +116,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();