Skip to content

Commit

Permalink
Move Tab Switcher mode handling into CommandPalette (microsoft#8656)
Browse files Browse the repository at this point in the history 
A part of the microsoft#8415.
Includes:
* Moving `TabSwitcherMode` related decisions into `CommandPalette`
(simplifying the logic of `TerminalPage::SelectNextTab`)
* Fix a bug where the index of first tab switch is incorrect
(since bindings are not updated)
* Removing redundant `CommandPalette` updates
* Preparations for tabs binding
  • Loading branch information
@Don-Vito @mpela81
Don-Vito authored and mpela81 committed Jan 28, 2021
1 parent cf76bb0 commit ad42fd0
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 114 deletions.
9 changes: 2 additions & 7 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,13 +521,8 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleOpenTabSearch(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
// Tab search is always in-order.
CommandPalette().SetTabs(_tabs, true);

auto opt = _GetFocusedTabIndex();
uint32_t startIdx = opt.value_or(0);

CommandPalette().EnableTabSwitcherMode(true, startIdx);
CommandPalette().SetTabs(_tabs, _mruTabs);
CommandPalette().EnableTabSearchMode();
CommandPalette().Visibility(Visibility::Visible);

args.Handled(true);
Expand Down
136 changes: 74 additions & 62 deletions src/cascadia/TerminalApp/CommandPalette.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace winrt::TerminalApp::implementation
_currentNestedCommands = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_allCommands = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_tabActions = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_mruTabActions = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_commandLineHistory = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();

_switchToMode(CommandPaletteMode::ActionMode);
Expand All @@ -51,6 +52,12 @@ namespace winrt::TerminalApp::implementation
RegisterPropertyChangedCallback(UIElement::VisibilityProperty(), [this](auto&&, auto&&) {
if (Visibility() == Visibility::Visible)
{
if (_filteredActionsView().Items().Size() == 0 && _filteredActions.Size() > 0)
{
// Force immediate binding update so we can select an item
Bindings->Update();
}

if (_currentMode == CommandPaletteMode::TabSwitchMode)
{
_searchBox().Visibility(Visibility::Collapsed);
Expand All @@ -66,7 +73,6 @@ namespace winrt::TerminalApp::implementation
{
_filteredActionsView().SelectedIndex(0);
_searchBox().Focus(FocusState::Programmatic);
_updateFilteredActions();
}

TraceLoggingWrite(
Expand Down Expand Up @@ -112,7 +118,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
void CommandPalette::SelectNextItem(const bool moveDown)
{
const auto selected = _filteredActionsView().SelectedIndex();
auto selected = _filteredActionsView().SelectedIndex();
const int numItems = ::base::saturated_cast<int>(_filteredActionsView().Items().Size());

// Do not try to select an item if
Expand Down Expand Up @@ -561,7 +567,7 @@ namespace winrt::TerminalApp::implementation
case CommandPaletteMode::TabSearchMode:
return _tabActions;
case CommandPaletteMode::TabSwitchMode:
return _tabActions;
return _tabSwitcherMode == TabSwitcherMode::MostRecentlyUsed ? _mruTabActions : _tabActions;
case CommandPaletteMode::CommandlineMode:
return _commandLineHistory;
default:
Expand Down Expand Up @@ -854,31 +860,40 @@ namespace winrt::TerminalApp::implementation
_allCommands.Append(filteredCommand);
}

_updateFilteredActions();
if (Visibility() == Visibility::Visible && _currentMode == CommandPaletteMode::ActionMode)
{
_updateFilteredActions();
}
}

void CommandPalette::SetTabs(Collections::IVector<TabBase> const& tabs, const bool clearList)
// Method Description:
// - Replaces a list of filtered commands in the target collection with new
// commands based on the tabs in the source collection.
// Although the source observable we still don't register on it,
// so the palette user will need to reset the binding manually every time
// the source collection changes
// Arguments:
// - source: the tabs to use for creation filtered commands
// - target: the collection to store newly created filtered commands
// Return Value:
// - <none>
void CommandPalette::_bindTabs(
Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase> const& source,
Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> const& target)
{
_tabActions.Clear();
for (const auto& tab : tabs)
target.Clear();
for (const auto& tab : source)
{
auto tabPaletteItem{ winrt::make<winrt::TerminalApp::implementation::TabPaletteItem>(tab) };
auto filteredCommand{ winrt::make<FilteredCommand>(tabPaletteItem) };
_tabActions.Append(filteredCommand);
target.Append(filteredCommand);
}
}

// The smooth remove/add animations that happen during
// UpdateFilteredActions don't work very well with changing the tab
// order, because of the sheer amount of remove/adds. So, let's just
// clear & rebuild the list when we change the set of tabs.
//
// Some callers might actually want smooth updating, like when the list
// of tabs changes.
if (clearList && _currentMode == CommandPaletteMode::TabSwitchMode)
{
_filteredActions.Clear();
}
_updateFilteredActions();
void CommandPalette::SetTabs(Collections::IObservableVector<TabBase> const& tabs, Collections::IObservableVector<TabBase> const& mruTabs)
{
_bindTabs(tabs, _tabActions);
_bindTabs(mruTabs, _mruTabActions);
}

void CommandPalette::EnableCommandPaletteMode(CommandPaletteLaunchMode const launchMode)
Expand All @@ -888,26 +903,11 @@ namespace winrt::TerminalApp::implementation
CommandPaletteMode::ActionMode;

_switchToMode(mode);
_updateFilteredActions();
}

void CommandPalette::_switchToMode(CommandPaletteMode mode)
{
// The smooth remove/add animations that happen during
// UpdateFilteredActions don't work very well when switching between
// modes because of the sheer amount of remove/adds. So, let's just
// clear + append when switching between modes.
if (mode != _currentMode)
{
_currentMode = mode;
_filteredActions.Clear();
auto commandsToFilter = _commandsToFilter();

for (auto action : commandsToFilter)
{
_filteredActions.Append(action);
}
}
_currentMode = mode;

ParsedCommandLineText(L"");
_searchBox().Text(L"");
Expand Down Expand Up @@ -940,6 +940,13 @@ namespace winrt::TerminalApp::implementation
PrefixCharacter(L">");
break;
}

// The smooth remove/add animations that happen during
// UpdateFilteredActions don't work very well when switching between
// modes because of the sheer amount of remove/adds. So, let's just
// clear + append when switching between modes.
_filteredActions.Clear();
_updateFilteredActions();
}

// Method Description:
Expand All @@ -956,27 +963,34 @@ namespace winrt::TerminalApp::implementation
winrt::hstring searchText{ _getTrimmedInput() };

auto commandsToFilter = _commandsToFilter();
for (const auto& action : commandsToFilter)
{
// Update filter for all commands
// This will modify the highlighting but will also lead to re-computation of weight (and consequently sorting).
// Pay attention that it already updates the highlighting in the UI
action.UpdateFilter(searchText);

// if there is active search we skip commands with 0 weight
if (searchText.empty() || action.Weight() > 0)
if (_currentMode == CommandPaletteMode::TabSwitchMode)
{
std::copy(begin(commandsToFilter), end(commandsToFilter), std::back_inserter(actions));
}
else if (_currentMode == CommandPaletteMode::TabSearchMode || _currentMode == CommandPaletteMode::ActionMode || _currentMode == CommandPaletteMode::CommandlineMode)
{
for (const auto& action : commandsToFilter)
{
actions.push_back(action);
// Update filter for all commands
// This will modify the highlighting but will also lead to re-computation of weight (and consequently sorting).
// Pay attention that it already updates the highlighting in the UI
action.UpdateFilter(searchText);

// if there is active search we skip commands with 0 weight
if (searchText.empty() || action.Weight() > 0)
{
actions.push_back(action);
}
}
}

// We want to present the commands sorted,
// unless we are in the TabSwitcherMode and TabSearchMode,
// in which we want to preserve the original order (to be aligned with the tab view)
if (_currentMode != CommandPaletteMode::TabSearchMode && _currentMode != CommandPaletteMode::TabSwitchMode && _currentMode != CommandPaletteMode::CommandlineMode)
// We want to present the commands sorted
if (_currentMode == CommandPaletteMode::ActionMode)
{
std::sort(actions.begin(), actions.end(), FilteredCommand::Compare);
}

return actions;
}

Expand Down Expand Up @@ -1051,20 +1065,18 @@ namespace winrt::TerminalApp::implementation
_currentNestedCommands.Clear();
}

void CommandPalette::EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx)
void CommandPalette::EnableTabSwitcherMode(const uint32_t startIdx, TabSwitcherMode tabSwitcherMode)
{
_switcherStartIdx = startIdx;

if (searchMode)
{
_switchToMode(CommandPaletteMode::TabSearchMode);
}
else
{
_switchToMode(CommandPaletteMode::TabSwitchMode);
}

_updateFilteredActions();
// The _switcherStartIdx field allows us to select the current tab
// We need to take it into account only in the in-order mode,
// as an MRU mode the current tab is on top by definition.
_switcherStartIdx = tabSwitcherMode == TabSwitcherMode::InOrder ? startIdx : 0;
_tabSwitcherMode = tabSwitcherMode;
_switchToMode(CommandPaletteMode::TabSwitchMode);
}

void CommandPalette::EnableTabSearchMode()
{
_switchToMode(CommandPaletteMode::TabSearchMode);
}
}
13 changes: 8 additions & 5 deletions src/cascadia/TerminalApp/CommandPalette.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,9 @@ namespace winrt::TerminalApp::implementation
Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::FilteredCommand> FilteredActions();

void SetCommands(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& actions);
void SetTabs(Windows::Foundation::Collections::IVector<winrt::TerminalApp::TabBase> const& tabs, const bool clearList);
void SetTabs(Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase> const& tabs, Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase> const& mruTabs);
void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap);

void EnableCommandPaletteMode(Microsoft::Terminal::Settings::Model::CommandPaletteLaunchMode const launchMode);

bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);

void SelectNextItem(const bool moveDown);
Expand All @@ -45,8 +43,9 @@ namespace winrt::TerminalApp::implementation
void ScrollToTop();
void ScrollToBottom();

// Tab Switcher
void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx);
void EnableCommandPaletteMode(Microsoft::Terminal::Settings::Model::CommandPaletteLaunchMode const launchMode);
void EnableTabSwitcherMode(const uint32_t startIdx, Microsoft::Terminal::Settings::Model::TabSwitcherMode tabSwitcherMode);
void EnableTabSearchMode();

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
OBSERVABLE_GETSET_PROPERTY(winrt::hstring, NoMatchesText, _PropertyChangedHandlers);
Expand Down Expand Up @@ -112,7 +111,11 @@ namespace winrt::TerminalApp::implementation

// Tab Switcher
Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> _tabActions{ nullptr };
Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> _mruTabActions{ nullptr };
Microsoft::Terminal::Settings::Model::TabSwitcherMode _tabSwitcherMode;
uint32_t _switcherStartIdx;

void _bindTabs(Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase> const& source, Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> const& target);
void _anchorKeyUpHandler();

winrt::Windows::UI::Xaml::Controls::ListView::SizeChanged_revoker _sizeChangedRevoker;
Expand Down
9 changes: 6 additions & 3 deletions src/cascadia/TerminalApp/CommandPalette.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ namespace TerminalApp
Windows.Foundation.Collections.IObservableVector<FilteredCommand> FilteredActions { get; };

void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
void SetTabs(Windows.Foundation.Collections.IVector<TabBase> tabs, Boolean clearList);

void SetTabs(Windows.Foundation.Collections.IObservableVector<TabBase> tabs, Windows.Foundation.Collections.IObservableVector<TabBase> mruTabs);

void SetKeyMap(Microsoft.Terminal.Settings.Model.KeyMapping keymap);
void EnableCommandPaletteMode(Microsoft.Terminal.Settings.Model.CommandPaletteLaunchMode launchMode);

void SelectNextItem(Boolean moveDown);

void EnableTabSwitcherMode(Boolean searchMode, UInt32 startIdx);
void EnableCommandPaletteMode(Microsoft.Terminal.Settings.Model.CommandPaletteLaunchMode launchMode);
void EnableTabSwitcherMode(UInt32 startIdx, Microsoft.Terminal.Settings.Model.TabSwitcherMode tabSwitcherMode);
void EnableTabSearchMode();

event Windows.Foundation.TypedEventHandler<CommandPalette, TabBase> SwitchToTabRequested;
event Windows.Foundation.TypedEventHandler<CommandPalette, Microsoft.Terminal.Settings.Model.Command> DispatchCommandRequested;
Expand Down
50 changes: 14 additions & 36 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ namespace winrt::TerminalApp::implementation
{
TerminalPage::TerminalPage() :
_tabs{ winrt::single_threaded_observable_vector<TerminalApp::TabBase>() },
_mruTabs{ winrt::single_threaded_vector<TerminalApp::TabBase>() },
_mruTabs{ winrt::single_threaded_observable_vector<TerminalApp::TabBase>() },
_startupActions{ winrt::single_threaded_vector<ActionAndArgs>() },
_hostingHwnd{}
{
Expand Down Expand Up @@ -1323,49 +1323,27 @@ namespace winrt::TerminalApp::implementation
// - Sets focus to the tab to the right or left the currently selected tab.
void TerminalPage::_SelectNextTab(const bool bMoveRight)
{
const auto index{ _GetFocusedTabIndex().value_or(0) };
const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode();
const bool useInOrderTabIndex = tabSwitchMode != TabSwitcherMode::MostRecentlyUsed;

// First, determine what the index of the newly selected tab should be.
// This changes if we're doing an in-order traversal vs a MRU traversal.
auto newTabIndex = 0;
if (useInOrderTabIndex)
{
// Determine what the next in-order tab index is
if (auto index{ _GetFocusedTabIndex() })
{
uint32_t tabCount = _tabs.Size();
// Wraparound math. By adding tabCount and then calculating
// modulo tabCount, we clamp the values to the range [0,
// tabCount) while still supporting moving leftward from 0 to
// tabCount - 1.
newTabIndex = ((tabCount + *index + (bMoveRight ? 1 : -1)) % tabCount);
}
if (tabSwitchMode == TabSwitcherMode::Disabled)
{
uint32_t tabCount = _tabs.Size();
// Wraparound math. By adding tabCount and then calculating
// modulo tabCount, we clamp the values to the range [0,
// tabCount) while still supporting moving leftward from 0 to
// tabCount - 1.
const auto newTabIndex = ((tabCount + index + (bMoveRight ? 1 : -1)) % tabCount);
_SelectTab(newTabIndex);
}
else
{
// Determine what the next "most recently used" index is.
// In this case, our focused tab index (in the MRU ordering) is
// always 0. So, going next should go to index 1, and going prev
// should wrap to the end.
uint32_t tabCount = _mruTabs.Size();
newTabIndex = ((tabCount + (bMoveRight ? 1 : -1)) % tabCount);
}

const bool useTabSwitcher = tabSwitchMode != TabSwitcherMode::Disabled;

if (useTabSwitcher)
{
CommandPalette().SetTabs(useInOrderTabIndex ? _tabs : _mruTabs, true);
CommandPalette().SetTabs(_tabs, _mruTabs);

// Otherwise, set up the tab switcher in the selected mode, with
// the given ordering, and make it visible.
CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
CommandPalette().EnableTabSwitcherMode(index, tabSwitchMode);
CommandPalette().Visibility(Visibility::Visible);
}
else if (auto index{ _GetFocusedTabIndex() })
{
_SelectTab(newTabIndex);
CommandPalette().SelectNextItem(bMoveRight);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ namespace winrt::TerminalApp::implementation
Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };

Windows::Foundation::Collections::IObservableVector<TerminalApp::TabBase> _tabs;
Windows::Foundation::Collections::IVector<TerminalApp::TabBase> _mruTabs;
Windows::Foundation::Collections::IObservableVector<TerminalApp::TabBase> _mruTabs;
static winrt::com_ptr<TerminalTab> _GetTerminalTabImpl(const TerminalApp::TabBase& tab);

void _UpdateTabIndices();
Expand Down

0 comments on commit ad42fd0

Please sign in to comment.