Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preview tab switching with the ATS #7796

Merged
1 commit merged into from
Oct 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/cascadia/TerminalApp/CommandPalette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ namespace winrt::TerminalApp::implementation
}
_sizeChangedRevoker.revoke();
});

_filteredActionsView().SelectionChanged({ this, &CommandPalette::_selectedCommandChanged });
}

// Method Description:
Expand All @@ -117,6 +119,29 @@ namespace winrt::TerminalApp::implementation
_filteredActionsView().ScrollIntoView(_filteredActionsView().SelectedItem());
}

// Method Description:
// - Called when the command selection changes. We'll use this in the tab
// switcher to "preview" tabs as the user navigates the list of tabs. To
// do that, we'll dispatch the switch to tab command for this tab, but not
// dismiss the switcher.
// Arguments:
// - <unused>
// Return Value:
// - <none>
void CommandPalette::_selectedCommandChanged(const IInspectable& /*sender*/,
const Windows::UI::Xaml::RoutedEventArgs& /*args*/)
{
if (_currentMode == CommandPaletteMode::TabSwitchMode)
{
const auto& selectedCommand = _filteredActionsView().SelectedItem();
if (const auto& command = selectedCommand.try_as<TerminalApp::Command>())
{
const auto& actionAndArgs = command.Action();
_dispatch.DoAction(actionAndArgs);
}
}
}
Comment on lines +131 to +143
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems circuitous. TerminalPage is already driving the selection of tabs in the tab switcher, so should we reduce coupling by having TerminalPage do the active terminal change in addition to driving the switching UI?

Maybe having it be in MRU order changes things.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm averse to taking on too many more "are we in mode A, B, C, or D" checks :/)


void CommandPalette::_previewKeyDownHandler(IInspectable const& /*sender*/,
Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/CommandPalette.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ namespace winrt::TerminalApp::implementation
void _keyUpHandler(Windows::Foundation::IInspectable const& sender,
Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);

void _selectedCommandChanged(Windows::Foundation::IInspectable const& sender,
Windows::UI::Xaml::RoutedEventArgs const& args);

void _updateUIForStackChange();

void _rootPointerPressed(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e);
Expand Down
15 changes: 14 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1967,7 +1967,20 @@ namespace winrt::TerminalApp::implementation
_tabContent.Children().Clear();
_tabContent.Children().Append(tab->GetRootElement());

tab->SetFocused(true);
// GH#7409: If the tab switcher is open, then we _don't_ want to
// automatically focus the new tab here. The tab switcher wants
// to be able to "preview" the selected tab as the user tabs
// through the menu, but if we toss the focus to the control
// here, then the user won't be able to navigate the ATS any
// longer.
//
// When the tab swither is eventually dismissed, the focus will
// get tossed back to the focused terminal control, so we don't
// need to worry about focus getting lost.
if (CommandPalette().Visibility() != Visibility::Visible)
{
tab->SetFocused(true);
}

// Raise an event that our title changed
_titleChangeHandlers(*this, tab->GetActiveTitle());
Expand Down