Allow the user to use the tab switcher with in-order tab switching

doc/cascadia/profiles.schema.json

@@ -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": [

src/cascadia/LocalTests_TerminalApp/TabTests.cpp

@@ -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
 {
@@ -66,6 +81,8 @@ namespace TerminalAppLocalTests
         TEST_METHOD(MoveFocusFromZoomedPane);
         TEST_METHOD(CloseZoomedPane);
 
+        TEST_METHOD(NextMRUTab);
+
         TEST_CLASS_SETUP(ClassSetup)
         {
             return true;
@@ -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
@@ -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
                 }
             ]
         })" };
@@ -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");
+        });
+    }
 }

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -508,12 +508,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);

src/cascadia/TerminalApp/CommandPalette.cpp

@@ -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);
 
@@ -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(
@@ -472,8 +472,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:
@@ -684,9 +685,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();
     }
 
@@ -1066,4 +1078,5 @@ namespace winrt::TerminalApp::implementation
 
         _updateFilteredActions();
     }
+
 }

src/cascadia/TerminalApp/CommandPalette.h

@@ -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();
@@ -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);
@@ -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 };
@@ -97,7 +97,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();
 

src/cascadia/TerminalApp/CommandPalette.idl

@@ -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();