Skip to content

Commit

Permalink
Allow the user to use the tab switcher with in-order tab switching (#…
Browse files Browse the repository at this point in the history 
…8076)

## Summary of the Pull Request

Changes the way the `useTabSwitcher` setting works. It now accepts either a boolean or a string:
* `true`, `"mru"`: Use the tab switcher with MRU tab switching
* `"inOrder"`: Use the tab switcher, with in-order tab switching
* `false`, `"disabled"`: Don't use the tab switcher. Tabs will switch in-order.

This is following the discussion chronicled in #8025, as well as the follow-up investigation in that thread.

## References

* #7952 introduced MRU tab switching

## PR Checklist
* [x] Closes #8025 - there's also discussion of using a parameter in an action to override this setting, but that should get punted to a follow-up task
* [x] I work here
* [x] Tests added/passed - YOU BET THEY WERE
* [ ] Requires documentation to be updated

## Validation Steps Performed

I've been switching tabs all day and all night, with different settings values, and hot-reloading the setting.

I also _ran the test_ I added.
  • Loading branch information
@zadjii-msft
zadjii-msft authored Nov 5, 2020
1 parent 930e24c commit 6639df9
Show file tree
Hide file tree
Showing 13 changed files with 339 additions and 39 deletions.
34 changes: 32 additions & 2 deletions doc/cascadia/profiles.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,38 @@
},
"useTabSwitcher": {
"default": true,
"description": "When set to \"true\", the \"nextTab\" and \"prevTab\" commands will use the tab switcher UI.",
"type": "boolean"
"description": "Deprecated. Please use \"tabSwitcherMode\" instead.",
"oneOf": [
{
"type": "boolean"
},
{
"enum": [
"mru",
"inOrder",
"disabled",
],
"type": "string"
}
],
"deprecated": true
},
"tabSwitcherMode": {
"default": true,
"description": "When set to \"true\" or \"mru\", the \"nextTab\" and \"prevTab\" commands will use the tab switcher UI, with most-recently-used ordering. When set to \"inOrder\", these actions will switch tabs in their current ordering. Set to \"false\" to disable the tab switcher.",
"oneOf": [
{
"type": "boolean"
},
{
"enum": [
"mru",
"inOrder",
"disabled",
],
"type": "string"
}
]
}
},
"required": [
Expand Down
159 changes: 159 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,25 @@ using namespace Microsoft::Console;
using namespace TerminalApp;
using namespace winrt::TerminalApp;
using namespace winrt::Microsoft::Terminal::Settings::Model;

using namespace WEX::Logging;
using namespace WEX::TestExecution;
using namespace WEX::Common;

using namespace winrt::Windows::ApplicationModel::DataTransfer;
using namespace winrt::Windows::Foundation::Collections;
using namespace winrt::Windows::System;
using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Xaml::Controls;
using namespace winrt::Windows::UI::Core;
using namespace winrt::Windows::UI::Text;

namespace winrt
{
namespace MUX = Microsoft::UI::Xaml;
namespace WUX = Windows::UI::Xaml;
using IInspectable = Windows::Foundation::IInspectable;
}

namespace TerminalAppLocalTests
{
Expand Down Expand Up @@ -66,6 +81,8 @@ namespace TerminalAppLocalTests
TEST_METHOD(MoveFocusFromZoomedPane);
TEST_METHOD(CloseZoomedPane);

TEST_METHOD(NextMRUTab);

TEST_CLASS_SETUP(ClassSetup)
{
return true;
Expand All @@ -82,6 +99,13 @@ namespace TerminalAppLocalTests
winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> _commonSetup();
};

template<typename TFunction>
void TestOnUIThread(const TFunction& function)
{
const auto result = RunOnUIThread(function);
VERIFY_SUCCEEDED(result);
}

void TabTests::EnsureTestsActivate()
{
// This test was originally used to ensure that XAML Islands was
Expand Down Expand Up @@ -513,16 +537,31 @@ namespace TerminalAppLocalTests
const std::string settingsJson0{ R"(
{
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"showTabsInTitlebar": false,
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"tabTitle" : "Profile 0",
"historySize": 1
},
{
"name" : "profile1",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
"tabTitle" : "Profile 1",
"historySize": 2
},
{
"name" : "profile2",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}",
"tabTitle" : "Profile 2",
"historySize": 3
},
{
"name" : "profile3",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}",
"tabTitle" : "Profile 3",
"historySize": 4
}
]
})" };
Expand Down Expand Up @@ -689,4 +728,124 @@ namespace TerminalAppLocalTests
});
VERIFY_SUCCEEDED(result);
}

void TabTests::NextMRUTab()
{
// This is a test for GH#8025 - we want to make sure that we can do both
// in-order and MRU tab traversal, using the tab switcher and with the
// tab switcher disabled.

auto page = _commonSetup();

Log::Comment(L"Create a second tab");
TestOnUIThread([&page]() {
NewTerminalArgs newTerminalArgs{ 1 };
page->_OpenNewTab(newTerminalArgs);
});
VERIFY_ARE_EQUAL(2u, page->_tabs.Size());

Log::Comment(L"Create a third tab");
TestOnUIThread([&page]() {
NewTerminalArgs newTerminalArgs{ 2 };
page->_OpenNewTab(newTerminalArgs);
});
VERIFY_ARE_EQUAL(3u, page->_tabs.Size());

Log::Comment(L"Create a fourth tab");
TestOnUIThread([&page]() {
NewTerminalArgs newTerminalArgs{ 3 };
page->_OpenNewTab(newTerminalArgs);
});
VERIFY_ARE_EQUAL(4u, page->_tabs.Size());

TestOnUIThread([&page]() {
uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1);
VERIFY_ARE_EQUAL(3u, focusedIndex, L"Verify the fourth tab is the focused one");
});

Log::Comment(L"Select the second tab");
TestOnUIThread([&page]() {
page->_SelectTab(1);
});

TestOnUIThread([&page]() {
uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1);
VERIFY_ARE_EQUAL(1u, focusedIndex, L"Verify the second tab is the focused one");
});

Log::Comment(L"Change the tab switch order to MRU switching");
TestOnUIThread([&page]() {
page->_settings.GlobalSettings().TabSwitcherMode(TabSwitcherMode::MostRecentlyUsed);
});

Log::Comment(L"Switch to the next MRU tab, which is the fourth tab");
TestOnUIThread([&page]() {
ActionEventArgs eventArgs{};
page->_HandleNextTab(nullptr, eventArgs);
});

Log::Comment(L"Sleep to let events propagate");
Sleep(250);

TestOnUIThread([&page]() {
Log::Comment(L"Hide the command palette, to confirm the selection");
// If you don't do this, the palette will just stay open, and the
// next time we call _HandleNextTab, we'll continue traversing the
// MRU list, instead of just hoping one entry.
page->CommandPalette().Visibility(Visibility::Collapsed);
});

TestOnUIThread([&page]() {
uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1);
VERIFY_ARE_EQUAL(3u, focusedIndex, L"Verify the fourth tab is the focused one");
});

Log::Comment(L"Switch to the next MRU tab, which is the second tab");
TestOnUIThread([&page]() {
ActionEventArgs eventArgs{};
page->_HandleNextTab(nullptr, eventArgs);
});

Log::Comment(L"Sleep to let events propagate");
Sleep(250);

TestOnUIThread([&page]() {
Log::Comment(L"Hide the command palette, to confirm the selection");
// If you don't do this, the palette will just stay open, and the
// next time we call _HandleNextTab, we'll continue traversing the
// MRU list, instead of just hoping one entry.
page->CommandPalette().Visibility(Visibility::Collapsed);
});

TestOnUIThread([&page]() {
uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1);
VERIFY_ARE_EQUAL(1u, focusedIndex, L"Verify the second tab is the focused one");
});

Log::Comment(L"Change the tab switch order to in-order switching");
page->_settings.GlobalSettings().TabSwitcherMode(TabSwitcherMode::InOrder);

Log::Comment(L"Switch to the next in-order tab, which is the third tab");
TestOnUIThread([&page]() {
ActionEventArgs eventArgs{};
page->_HandleNextTab(nullptr, eventArgs);
});
TestOnUIThread([&page]() {
uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1);
VERIFY_ARE_EQUAL(2u, focusedIndex, L"Verify the third tab is the focused one");
});

Log::Comment(L"Change the tab switch order to not use the tab switcher (which is in-order always)");
page->_settings.GlobalSettings().TabSwitcherMode(TabSwitcherMode::Disabled);

Log::Comment(L"Switch to the next in-order tab, which is the fourth tab");
TestOnUIThread([&page]() {
ActionEventArgs eventArgs{};
page->_HandleNextTab(nullptr, eventArgs);
});
TestOnUIThread([&page]() {
uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1);
VERIFY_ARE_EQUAL(3u, focusedIndex, L"Verify the fourth tab is the focused one");
});
}
}
7 changes: 1 addition & 6 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,12 +524,7 @@ namespace winrt::TerminalApp::implementation
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);
_UpdatePaletteWithInOrderTabs();

auto opt = _GetFocusedTabIndex();
uint32_t startIdx = opt.value_or(0);
Expand Down
25 changes: 19 additions & 6 deletions src/cascadia/TerminalApp/CommandPalette.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace winrt::TerminalApp::implementation
_nestedActionStack = winrt::single_threaded_vector<Command>();
_currentNestedCommands = winrt::single_threaded_vector<Command>();
_allCommands = winrt::single_threaded_vector<Command>();
_allTabActions = winrt::single_threaded_vector<Command>();
_tabActions = winrt::single_threaded_vector<Command>();

_switchToMode(CommandPaletteMode::ActionMode);

Expand All @@ -51,19 +51,19 @@ namespace winrt::TerminalApp::implementation
if (_currentMode == CommandPaletteMode::TabSwitchMode)
{
_searchBox().Visibility(Visibility::Collapsed);
_filteredActionsView().Focus(FocusState::Keyboard);
_filteredActionsView().SelectedIndex(_switcherStartIdx);
_filteredActionsView().ScrollIntoView(_filteredActionsView().SelectedItem());
_filteredActionsView().Focus(FocusState::Keyboard);

// Do this right after becoming visible so we can quickly catch scenarios where
// modifiers aren't held down (e.g. command palette invocation).
_anchorKeyUpHandler();
}
else
{
_filteredActionsView().SelectedIndex(0);
_searchBox().Focus(FocusState::Programmatic);
_updateFilteredActions();
_filteredActionsView().SelectedIndex(0);
}

TraceLoggingWrite(
Expand Down Expand Up @@ -491,8 +491,9 @@ namespace winrt::TerminalApp::implementation

return _allCommands;
case CommandPaletteMode::TabSearchMode:
return _tabActions;
case CommandPaletteMode::TabSwitchMode:
return _allTabActions;
return _tabActions;
case CommandPaletteMode::CommandlineMode:
return winrt::single_threaded_vector<Command>();
default:
Expand Down Expand Up @@ -703,9 +704,20 @@ namespace winrt::TerminalApp::implementation
_updateFilteredActions();
}

void CommandPalette::SetTabActions(Collections::IVector<Command> const& tabs)
void CommandPalette::SetTabActions(Collections::IVector<Command> const& tabs, const bool clearList)
{
_allTabActions = tabs;
_tabActions = tabs;
// 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();
}

Expand Down Expand Up @@ -1085,4 +1097,5 @@ namespace winrt::TerminalApp::implementation

_updateFilteredActions();
}

}
6 changes: 3 additions & 3 deletions src/cascadia/TerminalApp/CommandPalette.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +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 SetTabActions(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& tabs, const bool clearList);
void SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings);

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

// Tab Switcher
void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx);
void SetTabSwitchOrder(const Microsoft::Terminal::Settings::Model::TabSwitcherMode order);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
OBSERVABLE_GETSET_PROPERTY(winrt::hstring, NoMatchesText, _PropertyChangedHandlers);
Expand All @@ -57,7 +58,6 @@ namespace winrt::TerminalApp::implementation
Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> _nestedActionStack{ nullptr };

winrt::TerminalApp::ShortcutActionDispatch _dispatch;

Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> _commandsToFilter();

bool _lastFilterTextWasEmpty{ true };
Expand Down Expand Up @@ -99,7 +99,7 @@ namespace winrt::TerminalApp::implementation
Microsoft::Terminal::TerminalControl::IKeyBindings _bindings;

// Tab Switcher
Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> _allTabActions{ nullptr };
Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> _tabActions{ nullptr };
uint32_t _switcherStartIdx;
void _anchorKeyUpHandler();

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CommandPalette.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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 SetTabActions(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> tabs, Boolean clearList);
void SetKeyBindings(Microsoft.Terminal.TerminalControl.IKeyBindings bindings);
void EnableCommandPaletteMode();

Expand Down
Loading

0 comments on commit 6639df9

Please sign in to comment.