Skip to content

Commit aead759

Browse files
zadjii-msftDHowett
authored andcommitted
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 8564b26)
1 parent 674c918 commit aead759

File tree

7 files changed

+67
-15
lines changed

7 files changed

+67
-15
lines changed

src/cascadia/TerminalApp/SettingsTab.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ namespace winrt::TerminalApp::implementation
6262
// - <none>
6363
void SettingsTab::_MakeTabViewItem()
6464
{
65-
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
65+
TabBase::_MakeTabViewItem();
66+
6667
Title(RS_(L"SettingsTab"));
6768
TabViewItem().Header(winrt::box_value(Title()));
6869
}

src/cascadia/TerminalApp/SettingsTab.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace winrt::TerminalApp::implementation
3333
void Focus(winrt::Windows::UI::Xaml::FocusState focusState) override;
3434

3535
private:
36-
void _MakeTabViewItem();
36+
void _MakeTabViewItem() override;
3737
winrt::fire_and_forget _CreateIcon();
3838
};
3939
}

src/cascadia/TerminalApp/TabBase.cpp

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,17 @@ namespace winrt::TerminalApp::implementation
4646
auto weakThis{ get_weak() };
4747

4848
// Build the menu
49-
Controls::MenuFlyout newTabFlyout;
50-
_AppendCloseMenuItems(newTabFlyout);
51-
TabViewItem().ContextFlyout(newTabFlyout);
49+
Controls::MenuFlyout contextMenuFlyout;
50+
// GH#5750 - When the context menu is dismissed with ESC, toss the focus
51+
// back to our control.
52+
contextMenuFlyout.Closed([weakThis](auto&&, auto&&) {
53+
if (auto tab{ weakThis.get() })
54+
{
55+
tab->_RequestFocusActiveControlHandlers();
56+
}
57+
});
58+
_AppendCloseMenuItems(contextMenuFlyout);
59+
TabViewItem().ContextFlyout(contextMenuFlyout);
5260
}
5361

5462
// Method Description:
@@ -225,4 +233,24 @@ namespace winrt::TerminalApp::implementation
225233
toolTip.Content(textBlock);
226234
WUX::Controls::ToolTipService::SetToolTip(TabViewItem(), toolTip);
227235
}
236+
237+
// Method Description:
238+
// - Initializes a TabViewItem for this Tab instance.
239+
// Arguments:
240+
// - <none>
241+
// Return Value:
242+
// - <none>
243+
void TabBase::_MakeTabViewItem()
244+
{
245+
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
246+
247+
// GH#3609 If the tab was tapped, and no one else was around to handle
248+
// it, then ask our parent to toss focus into the active control.
249+
TabViewItem().Tapped([weakThis{ get_weak() }](auto&&, auto&&) {
250+
if (auto tab{ weakThis.get() })
251+
{
252+
tab->_RequestFocusActiveControlHandlers();
253+
}
254+
});
255+
}
228256
}

src/cascadia/TerminalApp/TabBase.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ namespace winrt::TerminalApp::implementation
2525
void UpdateTabViewIndex(const uint32_t idx, const uint32_t numTabs);
2626
void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap);
2727

28+
WINRT_CALLBACK(RequestFocusActiveControl, winrt::delegate<void()>);
29+
2830
WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
2931
WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
3032

@@ -51,6 +53,8 @@ namespace winrt::TerminalApp::implementation
5153
virtual void _CreateContextMenu();
5254
virtual winrt::hstring _CreateToolTipTitle();
5355

56+
virtual void _MakeTabViewItem();
57+
5458
void _AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout);
5559
void _EnableCloseMenuItems();
5660
void _CloseTabsAfter();

src/cascadia/TerminalApp/TerminalPage.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,9 @@ namespace winrt::TerminalApp::implementation
866866
}
867867
});
868868

869-
newTabImpl->TabRenamerDeactivated([weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
869+
// The tab might want us to toss focus into the control, especially when
870+
// transient UIs (like the context menu, or the renamer) are dismissed.
871+
newTabImpl->RequestFocusActiveControl([weakThis{ get_weak() }]() {
870872
if (const auto page{ weakThis.get() })
871873
{
872874
if (!page->_newTabButton.Flyout().IsOpen())

src/cascadia/TerminalApp/TerminalTab.cpp

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ namespace winrt::TerminalApp::implementation
5353
}
5454
});
5555

56+
// GH#9162 - when the header is done renaming, ask for focus to be
57+
// tossed back to the control, rather into ourselves.
58+
_headerControl.RenameEnded([weakThis = get_weak()](auto&&, auto&&) {
59+
if (auto tab{ weakThis.get() })
60+
{
61+
tab->_RequestFocusActiveControlHandlers();
62+
}
63+
});
64+
5665
_UpdateHeaderControlMaxWidth();
5766

5867
// Use our header control as the TabViewItem's header
@@ -83,7 +92,7 @@ namespace winrt::TerminalApp::implementation
8392
// - <none>
8493
void TerminalTab::_MakeTabViewItem()
8594
{
86-
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
95+
TabBase::_MakeTabViewItem();
8796

8897
TabViewItem().DoubleTapped([weakThis = get_weak()](auto&& /*s*/, auto&& /*e*/) {
8998
if (auto tab{ weakThis.get() })
@@ -832,13 +841,22 @@ namespace winrt::TerminalApp::implementation
832841
}
833842

834843
// Build the menu
835-
Controls::MenuFlyout newTabFlyout;
844+
Controls::MenuFlyout contextMenuFlyout;
836845
Controls::MenuFlyoutSeparator menuSeparator;
837-
newTabFlyout.Items().Append(chooseColorMenuItem);
838-
newTabFlyout.Items().Append(renameTabMenuItem);
839-
newTabFlyout.Items().Append(menuSeparator);
840-
_AppendCloseMenuItems(newTabFlyout);
841-
TabViewItem().ContextFlyout(newTabFlyout);
846+
contextMenuFlyout.Items().Append(chooseColorMenuItem);
847+
contextMenuFlyout.Items().Append(renameTabMenuItem);
848+
contextMenuFlyout.Items().Append(menuSeparator);
849+
850+
// GH#5750 - When the context menu is dismissed with ESC, toss the focus
851+
// back to our control.
852+
contextMenuFlyout.Closed([weakThis](auto&&, auto&&) {
853+
if (auto tab{ weakThis.get() })
854+
{
855+
tab->_RequestFocusActiveControlHandlers();
856+
}
857+
});
858+
_AppendCloseMenuItems(contextMenuFlyout);
859+
TabViewItem().ContextFlyout(contextMenuFlyout);
842860
}
843861

844862
// Method Description:

src/cascadia/TerminalApp/TerminalTab.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ namespace winrt::TerminalApp::implementation
8989
DECLARE_EVENT(ColorSelected, _colorSelected, winrt::delegate<winrt::Windows::UI::Color>);
9090
DECLARE_EVENT(ColorCleared, _colorCleared, winrt::delegate<>);
9191
DECLARE_EVENT(TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>);
92-
FORWARDED_TYPED_EVENT(TabRenamerDeactivated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, (&_headerControl), RenameEnded);
9392

9493
private:
9594
std::shared_ptr<Pane> _rootPane{ nullptr };
@@ -117,7 +116,7 @@ namespace winrt::TerminalApp::implementation
117116
std::optional<Windows::UI::Xaml::DispatcherTimer> _bellIndicatorTimer;
118117
void _BellIndicatorTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);
119118

120-
void _MakeTabViewItem();
119+
void _MakeTabViewItem() override;
121120

122121
winrt::fire_and_forget _UpdateHeaderControlMaxWidth();
123122

0 commit comments

Comments
 (0)