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

Display ATS tabs in MRU order #7952

Merged
7 commits merged into from
Oct 21, 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
8 changes: 8 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,14 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleOpenTabSearch(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
// Tab search is always in-order.
auto tabCommands = winrt::single_threaded_vector<Command>();
for (const auto& tab : _tabs)
{
tabCommands.Append(tab.SwitchToTabCommand());
}
CommandPalette().SetTabActions(tabCommands);

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

Expand Down
45 changes: 6 additions & 39 deletions src/cascadia/TerminalApp/CommandPalette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,12 @@ namespace winrt::TerminalApp::implementation
_updateFilteredActions();
}

void CommandPalette::SetTabActions(Collections::IVector<Command> const& tabs)
{
_allTabActions = tabs;
_updateFilteredActions();
}

void CommandPalette::EnableCommandPaletteMode()
{
_switchToMode(CommandPaletteMode::ActionMode);
Expand Down Expand Up @@ -975,45 +981,6 @@ namespace winrt::TerminalApp::implementation
_currentNestedCommands.Clear();
}

// Method Description:
// - Listens for changes to TerminalPage's _tabs vector. Updates our vector of
// tab switching commands accordingly.
// Arguments:
// - s: The vector being listened to.
// - e: The vector changed args that tells us whether a change, insert, or removal was performed
// on the listened-to vector.
// Return Value:
// - <none>
void CommandPalette::OnTabsChanged(const IInspectable& s, const IVectorChangedEventArgs& e)
{
if (auto tabList = s.try_as<IObservableVector<TerminalApp::Tab>>())
{
auto idx = e.Index();
auto changedEvent = e.CollectionChange();

switch (changedEvent)
{
case CollectionChange::ItemChanged:
{
break;
}
case CollectionChange::ItemInserted:
{
auto tab = tabList.GetAt(idx);
_allTabActions.InsertAt(idx, tab.SwitchToTabCommand());
break;
}
case CollectionChange::ItemRemoved:
{
_allTabActions.RemoveAt(idx);
break;
}
}

_updateFilteredActions();
}
}

void CommandPalette::EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx)
{
_switcherStartIdx = startIdx;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CommandPalette.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace winrt::TerminalApp::implementation
Windows::Foundation::Collections::IObservableVector<Microsoft::Terminal::Settings::Model::Command> FilteredActions();

void SetCommands(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& actions);
void SetTabActions(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& tabs);
void SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings);

void EnableCommandPaletteMode();
Expand All @@ -35,7 +36,6 @@ namespace winrt::TerminalApp::implementation

// Tab Switcher
void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx);
void OnTabsChanged(const Windows::Foundation::IInspectable& s, const Windows::Foundation::Collections::IVectorChangedEventArgs& e);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
OBSERVABLE_GETSET_PROPERTY(winrt::hstring, NoMatchesText, _PropertyChangedHandlers);
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CommandPalette.idl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace TerminalApp
Windows.Foundation.Collections.IObservableVector<Microsoft.Terminal.Settings.Model.Command> FilteredActions { get; };

void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
void SetTabActions(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> tabs);
void SetKeyBindings(Microsoft.Terminal.TerminalControl.IKeyBindings bindings);
void EnableCommandPaletteMode();

Expand All @@ -26,6 +27,5 @@ namespace TerminalApp
void SetDispatch(ShortcutActionDispatch dispatch);

void EnableTabSwitcherMode(Boolean searchMode, UInt32 startIdx);
void OnTabsChanged(IInspectable s, Windows.Foundation.Collections.IVectorChangedEventArgs e);
}
}
74 changes: 52 additions & 22 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ namespace winrt::TerminalApp::implementation
{
TerminalPage::TerminalPage() :
_tabs{ winrt::single_threaded_observable_vector<TerminalApp::Tab>() },
_mruTabActions{ winrt::single_threaded_vector<Command>() },
_startupActions{ winrt::single_threaded_vector<ActionAndArgs>() }
{
InitializeComponent();
Expand Down Expand Up @@ -224,13 +225,6 @@ namespace winrt::TerminalApp::implementation
}
});

_tabs.VectorChanged([weakThis{ get_weak() }](auto&& s, auto&& e) {
if (auto page{ weakThis.get() })
{
page->CommandPalette().OnTabsChanged(s, e);
}
});

// Settings AllowDependentAnimations will affect whether animations are
// enabled application-wide, so we don't need to check it each time we
// want to create an animation.
Expand Down Expand Up @@ -672,6 +666,7 @@ namespace winrt::TerminalApp::implementation
// Add the new tab to the list of our tabs.
auto newTabImpl = winrt::make_self<Tab>(profileGuid, term);
_tabs.Append(*newTabImpl);
_mruTabActions.Append(newTabImpl->SwitchToTabCommand());
Copy link
Member

Choose a reason for hiding this comment

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

is this line necessary? Won't the new tab be appended automatically in _UpdatedSelectedTab?

I suppose it doesn't really matter. The redundancy is nice, especially if we ever have a way to open new tabs without automatically switching to them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kinda thought of _UpdatedSelectedTab as the function that does things when a Tab is focused, so it assumes that the Tab already exists in the list when it's called. That's why for MRU, _UpdatedSelectedTab doesn't append a new tab, it drags and drops the focused tab to the top of MRU. Then the adding of a new Tab/SwitchToTabCommand should be done by CreateNewTabFromSettings.

Copy link
Member

Choose a reason for hiding this comment

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

Okay that's fine.


// Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command.
newTabImpl->UpdateTabViewIndex(_tabs.Size() - 1);
Expand Down Expand Up @@ -1047,6 +1042,13 @@ namespace winrt::TerminalApp::implementation
auto tab{ _GetStrongTabImpl(tabIndex) };
tab->Shutdown();

uint32_t mruIndex;
if (_mruTabActions.IndexOf(_tabs.GetAt(tabIndex).SwitchToTabCommand(), mruIndex))
{
_mruTabActions.RemoveAt(mruIndex);
CommandPalette().SetTabActions(_mruTabActions);
}

_tabs.RemoveAt(tabIndex);
_tabView.TabItems().RemoveAt(tabIndex);
_UpdateTabIndices();
Expand Down Expand Up @@ -1177,29 +1179,34 @@ namespace winrt::TerminalApp::implementation
// - Sets focus to the tab to the right or left the currently selected tab.
void TerminalPage::_SelectNextTab(const bool bMoveRight)
{
if (auto index{ _GetFocusedTabIndex() })
if (_settings.GlobalSettings().UseTabSwitcher())
{
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);
CommandPalette().SetTabActions(_mruTabActions);

if (_settings.GlobalSettings().UseTabSwitcher())
{
if (CommandPalette().Visibility() == Visibility::Visible)
{
CommandPalette().SelectNextItem(bMoveRight);
}
// Since ATS is always MRU, our focused tab index is always 0.
// So, going next should go to index 1, and going prev should wrap to the end.
uint32_t tabCount = _mruTabActions.Size();
auto newTabIndex = ((tabCount + (bMoveRight ? 1 : -1)) % tabCount);

CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
CommandPalette().Visibility(Visibility::Visible);
if (CommandPalette().Visibility() == Visibility::Visible)
{
CommandPalette().SelectNextItem(bMoveRight);
}
else
{
_SelectTab(newTabIndex);
CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
CommandPalette().Visibility(Visibility::Visible);
}
}
else 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.
auto newTabIndex = ((tabCount + *index + (bMoveRight ? 1 : -1)) % tabCount);
_SelectTab(newTabIndex);
}
}

// Method Description:
Expand Down Expand Up @@ -1973,6 +1980,7 @@ namespace winrt::TerminalApp::implementation
if (CommandPalette().Visibility() != Visibility::Visible)
{
tab->SetFocused(true);
_UpdateMRUTab(index);
}

// Raise an event that our title changed
Expand Down Expand Up @@ -2508,6 +2516,7 @@ namespace winrt::TerminalApp::implementation
if (auto index{ _GetFocusedTabIndex() })
{
_GetStrongTabImpl(index.value())->SetFocused(true);
_UpdateMRUTab(index.value());
}
}

Expand Down Expand Up @@ -2548,6 +2557,27 @@ namespace winrt::TerminalApp::implementation
}
}

// Method Description:
// - Bumps the tab in its in-order index up to the top of the mru list.
// Arguments:
// - index: the in-order index of the tab to bump.
// Return Value:
// - <none>
void TerminalPage::_UpdateMRUTab(const uint32_t index)
{
uint32_t mruIndex;
auto command = _tabs.GetAt(index).SwitchToTabCommand();
if (_mruTabActions.IndexOf(command, mruIndex))
{
if (mruIndex > 0)
{
_mruTabActions.RemoveAt(mruIndex);
_mruTabActions.InsertAt(0, command);
CommandPalette().SetTabActions(_mruTabActions);
}
}
}

// -------------------------------- WinRT Events ---------------------------------
// Winrt events need a method for adding a callback to the event and removing the callback.
// These macros will define them both for you.
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ namespace winrt::TerminalApp::implementation
Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };

Windows::Foundation::Collections::IObservableVector<TerminalApp::Tab> _tabs;
Windows::Foundation::Collections::IVector<winrt::Microsoft::Terminal::Settings::Model::Command> _mruTabActions;
winrt::com_ptr<Tab> _GetStrongTabImpl(const uint32_t index) const;
winrt::com_ptr<Tab> _GetStrongTabImpl(const ::winrt::TerminalApp::Tab& tab) const;
void _UpdateTabIndices();
Expand Down Expand Up @@ -206,6 +207,9 @@ namespace winrt::TerminalApp::implementation

void _UnZoomIfNeeded();

void _UpdateTabSwitcherCommands(const bool mru);
void _UpdateMRUTab(const uint32_t index);

#pragma region ActionHandlers
// These are all defined in AppActionHandlers.cpp
void _HandleOpenNewTabDropdown(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
Expand Down