maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Make command palette ephemeral (#8377)

Browse source 
So the implementation is somewhat dirty.
The ideas was nice - add lostFocusHandler

However it broke few things:
* In the TabSwitcher the ListItem must be focusable since otherwise 
the SingleSelectionMode behavior breaks. 
To address this I had to put the lostFocusHandler on the items as well
 
* When you click the flyout, the palette loses focus, 
which makes the terminal page to set the focus on the tab, closing the flyout. 
To address this I had to ensure the tab won't get focused once the flyout is open.
In addition, flyout should fix the focus before opening, 
otherwise alt+tab will put a focus on a tab row rather than on tab

* I also had to close the palette if the tab order changes.
To prevent inconsistencies.

Closes #8355
This commit is contained in:
Don-Vito 2020-12-10 00:01:08 +02:00 committed by GitHub
parent 0bf9dcb63e
commit e1a8657370
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 70 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
src/cascadia/TerminalApp/CommandPalette.cpp
View file

@ -438,6 +438,44 @@ namespace winrt::TerminalApp::implementation
void CommandPalette::_rootPointerPressed(Windows::Foundation::IInspectable const& /*sender*/,
Windows::UI::Xaml::Input::PointerRoutedEventArgs const& /*e*/)
{
if (Visibility() != Visibility::Collapsed)
{
_dismissPalette();
}
}
// Method Description:
// - The purpose of this event handler is to hide the palette if it loses focus.
// We say we lost focus if our root element and all its descendants lost focus.
// This handler is invoked when our root element or some descendant loses focus.
// At this point we need to learn if the newly focused element belongs to this palette.
// To achieve this:
// - We start with the newly focused element and traverse its visual ancestors up to the Xaml root.
// - If one of the ancestors is this CommandPalette, then by our definition the focus is not lost
// - If we reach the Xaml root without meeting this CommandPalette,
// then the focus is not contained in it anymore and it should be dismissed
// Arguments:
// - <unused>
// Return Value:
// - <none>
void CommandPalette::_lostFocusHandler(Windows::Foundation::IInspectable const& /*sender*/,
Windows::UI::Xaml::RoutedEventArgs const& /*args*/)
{
auto focusedElementOrAncestor = Input::FocusManager::GetFocusedElement(this->XamlRoot()).try_as<DependencyObject>();
while (focusedElementOrAncestor)
{
if (focusedElementOrAncestor == *this)
{
// This palette is the focused element or an ancestor of the focused element. No need to dismiss.
return;
}
// Go up to the next ancestor
focusedElementOrAncestor = winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetParent(focusedElementOrAncestor);
}
// We got to the root (the element with no parent) and didn't meet this palette on the path.
// It means that it lost the focus and needs to be dismissed.
_dismissPalette();
}

3
src/cascadia/TerminalApp/CommandPalette.h
View file

@ -85,6 +85,9 @@ namespace winrt::TerminalApp::implementation
void _updateUIForStackChange();
void _rootPointerPressed(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e);
void _lostFocusHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::RoutedEventArgs const& args);
void _backdropPointerPressed(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e);
void _listItemClicked(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Controls::ItemClickEventArgs const& e);

6
src/cascadia/TerminalApp/CommandPalette.xaml
View file

@ -17,6 +17,7 @@ the MIT License. See LICENSE in the project root for license information. -->
PreviewKeyDown="_previewKeyDownHandler"
KeyDown="_keyDownHandler"
PreviewKeyUp="_keyUpHandler"
LostFocus="_lostFocusHandler"
mc:Ignorable="d"
AutomationProperties.Name="{x:Bind ControlName, Mode=OneWay}">
@ -245,8 +246,9 @@ the MIT License. See LICENSE in the project root for license information. -->
<!-- This HorizontalContentAlignment="Stretch" is important
to make sure it takes the entire width of the line -->
<ListViewItem HorizontalContentAlignment="Stretch"
AutomationProperties.Name="{x:Bind Command.Name, Mode=OneWay}"
AutomationProperties.AcceleratorKey="{x:Bind Command.KeyChordText, Mode=OneWay}">
IsTabStop="False"
AutomationProperties.Name="{x:Bind Command.Name, Mode=OneWay}"
AutomationProperties.AcceleratorKey="{x:Bind Command.KeyChordText, Mode=OneWay}">
<Grid HorizontalAlignment="Stretch" ColumnSpacing="8" >
<Grid.ColumnDefinitions>

51
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -585,6 +585,21 @@ namespace winrt::TerminalApp::implementation
newTabFlyout.Items().Append(aboutFlyout);
}
// Before opening the fly-out set focus on the current tab
// so no matter how fly-out is closed later on the focus will return to some tab.
// We cannot do it on closing because if the window loses focus (alt+tab)
// the closing event is not fired.
// It is important to set the focus on the tab
// Since the previous focus location might be discarded in the background,
// e.g., the command palette will be dismissed by the menu,
// and then closing the fly-out will move the focus to wrong location.
newTabFlyout.Opening([this](auto&&, auto&&) {
if (auto index{ _GetFocusedTabIndex() })
{
_tabs.GetAt(*index).Focus(FocusState::Programmatic);
_UpdateMRUTab(index.value());
}
});
_newTabButton.Flyout(newTabFlyout);
}
@ -1072,19 +1087,6 @@ namespace winrt::TerminalApp::implementation
_tabView.TabItems().RemoveAt(tabIndex);
_UpdateTabIndices();
// If the tab switcher is currently open, update it to reflect the
// current list of tabs.
const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode();
const bool commandPaletteIsVisible = CommandPalette().Visibility() == Visibility::Visible;
if (tabSwitchMode == TabSwitcherMode::MostRecentlyUsed && commandPaletteIsVisible)
{
CommandPalette().SetTabActions(_mruTabActions, false);
}
else if (commandPaletteIsVisible)
{
_UpdatePaletteWithInOrderTabs();
}
// To close the window here, we need to close the hosting window.
if (_tabs.Size() == 0)
{
@ -2107,6 +2109,7 @@ namespace winrt::TerminalApp::implementation
}
}
CommandPalette().Visibility(Visibility::Collapsed);
_UpdateTabView();
}
@ -2693,11 +2696,16 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_CommandPaletteClosed(const IInspectable& /*sender*/,
const RoutedEventArgs& /*eventArgs*/)
{
// Return focus to the active control
if (auto index{ _GetFocusedTabIndex() })
// We don't want to set focus on the tab if fly-out is open as it will be closed
// TODO GH#5400: consider checking we are not in the opening state, by hooking both Opening and Open events
if (!_newTabButton.Flyout().IsOpen())
{
_tabs.GetAt(*index).Focus(FocusState::Programmatic);
_UpdateMRUTab(index.value());
// Return focus to the active control
if (auto index{ _GetFocusedTabIndex() })
{
_tabs.GetAt(*index).Focus(FocusState::Programmatic);
_UpdateMRUTab(index.value());
}
}
}
@ -2834,15 +2842,6 @@ namespace winrt::TerminalApp::implementation
{
_mruTabActions.RemoveAt(mruIndex);
_mruTabActions.InsertAt(0, command);
// If the tab switcher is currently open, AND we're using it in
// MRU mode, then update it to reflect the current list of tabs.
const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode();
const bool commandPaletteIsVisible = CommandPalette().Visibility() == Visibility::Visible;
if (tabSwitchMode == TabSwitcherMode::MostRecentlyUsed && commandPaletteIsVisible)
{
CommandPalette().SetTabActions(_mruTabActions, false);
}
}
}
}