Display ATS tabs in MRU order #7952

leonMSFT · 2020-10-16T21:04:28Z

This PR changes the ATS display order to always be in most recently
used (MRU) order. I chose not to give ATS the option to be displayed
in-order because that order is better served through the traditional
left-right TabRow switching.

Note: TabSearch will stay in-order.

This means that users can only choose one order or another in their
nextTab/prevTab bindings. Setting useTabSwitcher to true will make
nT/pT open the ATS in MRU order. If it's set to false, the ATS won't
open and nT/pT will simply go left and right on the TabRow.

I'm open to getting rid of the global and making ATS its own keybinding,
but for now I figured I would keep the current behavior and open the PR
to get eyes on the code that doesn't have anything to do with the
settings.

Closes #973

…t fashion

… times

zadjii-msft

What happens if we re-order the tabs? Won't we not update the indicies in _mruTabActions, causing the MRU order to be reflective only of the positions the tabs were in, not the actual tabs themselves?

src/cascadia/TerminalApp/TerminalPage.cpp

zadjii-msft · 2020-10-21T07:31:48Z

src/cascadia/TerminalApp/TerminalPage.cpp

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


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

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.

Okay that's fine.

leonMSFT · 2020-10-21T18:12:04Z

I make a call to update the indices of Tab::SwitchToTabCommand whenever there's a rearrange/close/open, so it actually works perfectly fine 😄

zadjii-msft

Ah okay, I see how it works now. Thanks for clarifying!

zadjii-msft · 2020-10-21T19:28:06Z

src/cascadia/TerminalApp/TerminalPage.cpp

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


Okay that's fine.

DHowett

Elegant!

ghost · 2020-10-21T19:37:13Z

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

This PR changes the ATS display order to _always_ be in most recently used (MRU) order. I chose not to give ATS the option to be displayed in-order because that order is better served through the traditional left-right TabRow switching. _Note_: `TabSearch` will stay in-order. This means that users can only choose one order or another in their `nextTab/prevTab` bindings. Setting `useTabSwitcher` to true will make nT/pT open the ATS in MRU order. If it's set to false, the ATS won't open and nT/pT will simply go left and right on the TabRow. I'm open to getting rid of the global and making ATS its own keybinding, but for now I figured I would keep the current behavior and open the PR to get eyes on the code that doesn't have anything to do with the settings. Closes #973 (cherry picked from commit 00f5fba)

…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.

ghost · 2020-11-11T18:07:47Z

🎉Windows Terminal v1.4.3141.0 has been released which incorporates this pull request.:tada:

Handy links:

ghost · 2020-11-11T18:13:44Z

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

mselmanyildirim · 2021-12-06T23:36:22Z

It seems, to achive the MRU behavior, you need to set the tabSwitcherMode entry in the settings.json file to mru from the default one, which is inOrder. This is also documented here, but this issue thread seems to be shown earlier in search results, so I wanted to make an update here.

Note: Valid as of version 1.11.2921.0

leonMSFT added 6 commits October 15, 2020 14:07

new tab vec who dis

6eb461a

mru, but just for next tab, and it always mrus in classic first commi…

a1463d0

…t fashion

format

6410144

helps to update more frequently so we don't see drastic listview load…

0e6eb0f

… times

vector of commands, not tabs

9df56b5

format

48e3f06

zadjii-msft requested changes Oct 21, 2020

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 21, 2020

zadjii-msft assigned leonMSFT Oct 21, 2020

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 21, 2020

leonMSFT assigned zadjii-msft and unassigned leonMSFT Oct 21, 2020

prc

fe6eb80

zadjii-msft approved these changes Oct 21, 2020

View reviewed changes

zadjii-msft removed their assignment Oct 21, 2020

zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Oct 21, 2020

ghost requested review from miniksa, carlos-zamora, DHowett and PankajBhojwani October 21, 2020 19:29

DHowett approved these changes Oct 21, 2020

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 21, 2020

ghost merged commit 00f5fba into master Oct 21, 2020

ghost deleted the dev/lelian/mru branch October 21, 2020 19:37

DHowett mentioned this pull request Oct 22, 2020

Fix slowdown on open/close tabs when the user has many profiles #7993

Merged

zadjii-msft mentioned this pull request Oct 23, 2020

Add a setting for in-order tab traversal with the tab switcher #8025

Closed

zadjii-msft mentioned this pull request Oct 28, 2020

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

Merged

4 tasks

ghost mentioned this pull request Nov 11, 2020

Ctrl+Tab toggle between last two windows like Alt+Tab #973

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display ATS tabs in MRU order #7952

Display ATS tabs in MRU order #7952

leonMSFT commented Oct 16, 2020 •

edited by DHowett

Loading

zadjii-msft left a comment

zadjii-msft Oct 21, 2020

leonMSFT Oct 21, 2020

zadjii-msft Oct 21, 2020

leonMSFT commented Oct 21, 2020

zadjii-msft left a comment

zadjii-msft Oct 21, 2020

DHowett left a comment

ghost commented Oct 21, 2020

ghost commented Nov 11, 2020

ghost commented Nov 11, 2020

mselmanyildirim commented Dec 6, 2021

Display ATS tabs in MRU order #7952

Display ATS tabs in MRU order #7952

Conversation

leonMSFT commented Oct 16, 2020 • edited by DHowett Loading

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Oct 21, 2020

Choose a reason for hiding this comment

leonMSFT Oct 21, 2020

Choose a reason for hiding this comment

zadjii-msft Oct 21, 2020

Choose a reason for hiding this comment

leonMSFT commented Oct 21, 2020

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Oct 21, 2020

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Oct 21, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Nov 11, 2020

ghost commented Nov 11, 2020

mselmanyildirim commented Dec 6, 2021

leonMSFT commented Oct 16, 2020 •

edited by DHowett

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.