Skip to content

Commit

Permalink
In specific scenarios, focus the active control (#10048)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
zadjii-msft authored and DHowett committed May 14, 2021
1 parent 02aca0b commit 042f9ef
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 15 deletions.
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/SettingsTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ namespace winrt::TerminalApp::implementation
// - <none>
void SettingsTab::_MakeTabViewItem()
{
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
TabBase::_MakeTabViewItem();

Title(RS_(L"SettingsTab"));
TabViewItem().Header(winrt::box_value(Title()));
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/SettingsTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
}
34 changes: 31 additions & 3 deletions src/cascadia/TerminalApp/TabBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
// - <none>
// Return Value:
// - <none>
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();
}
});
}
}
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/TabBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<void()>);

WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);

Expand All @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
33 changes: 26 additions & 7 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -83,7 +92,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalTab::_MakeTabViewItem()
{
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
TabBase::_MakeTabViewItem();

TabViewItem().DoubleTapped([weakThis = get_weak()](auto&& /*s*/, auto&& /*e*/) {
if (auto tab{ weakThis.get() })
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalApp/TerminalTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ namespace winrt::TerminalApp::implementation
DECLARE_EVENT(ColorSelected, _colorSelected, winrt::delegate<winrt::Windows::UI::Color>);
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<Pane> _rootPane{ nullptr };
Expand Down Expand Up @@ -117,7 +116,7 @@ namespace winrt::TerminalApp::implementation
std::optional<Windows::UI::Xaml::DispatcherTimer> _bellIndicatorTimer;
void _BellIndicatorTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);

void _MakeTabViewItem();
void _MakeTabViewItem() override;

winrt::fire_and_forget _UpdateHeaderControlMaxWidth();

Expand Down

0 comments on commit 042f9ef

Please sign in to comment.