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

Feature Request: Add nextPane and prevPane keybindings #2871

Closed
moswald opened this issue Sep 24, 2019 · 8 comments · Fixed by #8183
Closed

Feature Request: Add nextPane and prevPane keybindings #2871

moswald opened this issue Sep 24, 2019 · 8 comments · Fixed by #8183
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@moswald
Copy link
Member

moswald commented Sep 24, 2019

There's nextTab and prevTab, but since I work in a single-tab, multi-pane environment, those keybindings do nothing for me. How about being able to bind ctrl+tab to nextPanel?

(Would fit in nicely with the rest of #1000 .)

Edit: I can't believe I typoed that last nextPanel as nextTab, making the sentence and request completely nonsensical. :D

@moswald moswald added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Sep 24, 2019
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 24, 2019
@zadjii-msft
Copy link
Member

As of #1910, you can move the focus between panes "directionally", with moveFocus{Up|Down|Left|Right}.

For example:

            { "keys": ["alt+left"], "command": "moveFocusLeft" },
            { "keys": ["alt+right"], "command": "moveFocusRight" },
            { "keys": ["alt+up"], "command": "moveFocusUp" },
            { "keys": ["alt+down"], "command": "moveFocusDown" },

I suppose though there's also room for moving focus "in order" or "in MRU order" through panes as well. MRU order makes sense, but I'm not really sure how "in order" would work

I'm going to leave this open because this makes sense as a potential feature, though I believe that #1910 will probably satisfy your needs.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Sep 24, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 24, 2019
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Sep 24, 2019
@moswald
Copy link
Member Author

moswald commented Sep 24, 2019

Yeah, I'm using the current moveFocus bindings, but they're just not as natural as the ctrl+tab muscle memory I've built up using other apps. I usually switch between just two tabs so MRU would work great, but "in order" could be implemented by remembering the order in which tabs were created.

@DHowett-MSFT DHowett-MSFT removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 7, 2019
@DHowett-MSFT
Copy link
Contributor

Yanking the triage tag on this one, since it's a worthwhile thing we should probably have. "Backlog" status stays!

@zadjii-msft
Copy link
Member

From @mfeiertag in #7194

New panes get assigned increasing IDs.
{ "command": { "action": "moveFocus", "direction": "next" }, "keys": "ctrl+alt+tab" }
focuses the pane with the next ID
{ "command": { "action": "moveFocus", "direction": "prev" }, "keys": "ctrl+alt+shift+tab" }
focuses the pane with the previous ID

IDs could be in order of creation or in order of last used.
Property Necessity Type Default Description
paneCycleMode Optional String created Defines the order of panes when cycling. Possible values: created or lastUsed

Related to #5803 and #5464
Similar to #4692

Example (pane in focus bold):

Single Pane:
| 1 |
Duplicate pane:
| 1 | | 2 |
Pressing ctrl+alt+tab:
| 1 | | 2 |
Duplicate Pane 1, than pane 2:
| 1 | | 2 |
| 3 | | 4 |
Pressing ctrl+alt+tab would go to pane 1:
| 1 | | 2 |
| 3 | | 4 |
Pressing ctrl+alt+shift+tab would go back to pane 4:
| 1 | | 2 |
| 3 | | 4 |
Pressing ctrl+alt+shift+tab again would go to pane 3:
| 1 | | 2 |
| 3 | | 4 |

@zadjii-msft zadjii-msft added this to Spec Needed ❓ in Specification Tracker via automation Dec 1, 2020
@zadjii-msft zadjii-msft moved this from Spec Needed ❓ to Spec In Review ⏰ in Specification Tracker Dec 1, 2020
@ghost ghost added the In-PR This issue has a related PR label Dec 1, 2020
@ghost ghost closed this as completed in #8183 Dec 11, 2020
@ghost ghost removed the In-PR This issue has a related PR label Dec 11, 2020
ghost pushed a commit that referenced this issue Dec 11, 2020
Adds a "move to previous pane" and "move to next pane" keybinding, which
navigates to the last/first focused pane

We assign pane IDs on creation and maintain a vector of active pane IDs
in MRU order. Navigating panes by MRU then requires specifying which
pane ID we want to focus. 

From our offline discussion (thanks @zadjii-msft for the concise
description):

> For the record, the full spec I'm imagining is:
> 
> { command": { "action": "focus(Next|Prev)Pane", "order": "inOrder"|"mru", "useSwitcher": true|false } },
> 
> and order defaults to mru, and useSwitcher will default to true, when
> there is a switcher. So 
> 
> { command": { "action": "focusNextPane" } },
> { command": { "action": "focusNextPane", "order": "mru" } },
> 
> these are the same action. (but right now we don't support the order
> param)
>  
> Then there'll be another PR for "focusPane(target=id)"
> 
> Then a third PR for "focus(Next|Prev)Pane(order=inOrder)"

> for the record, I prefer this approach over the "one action to rule
> them all" version with both target and order/direction as params,
> because I don't like the confusion of what happens if there's both
> target and order/direction provided. 

References #1000 
Closes #2871
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Dec 11, 2020
@DHowett
Copy link
Member

DHowett commented Dec 11, 2020

moveFocus with direction: previous will be how you do this.

zadjii-msft added a commit that referenced this issue Jan 11, 2021
## Summary of the Pull Request

This is a spec for "pane navigation", as we've already got a bit of an implementation in #8183. We've also had a heated discussion in Teams, and I wanted to capture a bit of that in a more formal doc. I suppose that "informal Teams chat" didn't work out in the end 😆.

Also, this is @PankajBhojwani's feature so I'm gonna let him drive. I mostly wrote this to test out a new spec template.

After discussion, we landed on proposal D, with a minor change of `last` to `prev`. This is how it was in #8183 before I started meddling 😝 

## PR Checklist
* [x] spec for #2871
* [x] I work here

## Detailed Description of the Pull Request / Additional comments

This is not my best spec ever - again, mostly just trying to spawn discussion, and prototype the new spec template.
mpela81 pushed a commit to mpela81/terminal that referenced this issue Jan 28, 2021
Adds a "move to previous pane" and "move to next pane" keybinding, which
navigates to the last/first focused pane

We assign pane IDs on creation and maintain a vector of active pane IDs
in MRU order. Navigating panes by MRU then requires specifying which
pane ID we want to focus. 

From our offline discussion (thanks @zadjii-msft for the concise
description):

> For the record, the full spec I'm imagining is:
> 
> { command": { "action": "focus(Next|Prev)Pane", "order": "inOrder"|"mru", "useSwitcher": true|false } },
> 
> and order defaults to mru, and useSwitcher will default to true, when
> there is a switcher. So 
> 
> { command": { "action": "focusNextPane" } },
> { command": { "action": "focusNextPane", "order": "mru" } },
> 
> these are the same action. (but right now we don't support the order
> param)
>  
> Then there'll be another PR for "focusPane(target=id)"
> 
> Then a third PR for "focus(Next|Prev)Pane(order=inOrder)"

> for the record, I prefer this approach over the "one action to rule
> them all" version with both target and order/direction as params,
> because I don't like the confusion of what happens if there's both
> target and order/direction provided. 

References microsoft#1000 
Closes microsoft#2871
mpela81 pushed a commit to mpela81/terminal that referenced this issue Jan 28, 2021
## Summary of the Pull Request

This is a spec for "pane navigation", as we've already got a bit of an implementation in microsoft#8183. We've also had a heated discussion in Teams, and I wanted to capture a bit of that in a more formal doc. I suppose that "informal Teams chat" didn't work out in the end 😆.

Also, this is @PankajBhojwani's feature so I'm gonna let him drive. I mostly wrote this to test out a new spec template.

After discussion, we landed on proposal D, with a minor change of `last` to `prev`. This is how it was in microsoft#8183 before I started meddling 😝 

## PR Checklist
* [x] spec for microsoft#2871
* [x] I work here

## Detailed Description of the Pull Request / Additional comments

This is not my best spec ever - again, mostly just trying to spawn discussion, and prototype the new spec template.
@Rosefield
Copy link
Contributor

Rosefield commented Jul 29, 2021

This item seems incomplete, specifically the next portion is missing from the current implementation and the linked spec. To me the idea of having the "in order" style cycling is important (equivalent to just cyclically alt-tabbing between panes) that lets me get to any pane through one keybinding. The "in order" order can be fairly arbitrary (so long as it is consistent), but just iterating through the tree leaves from first-leaf to last-leaf seems reasonable.

Also, the current implementation of the previous movement seems tangential to the "in order backwards" version, in that it only cycles between two panes. e.g. if you have 3 panes, 1| 2 | 3, and you go from 2 to 3, pressing previous will go to 2, then pressing previous will go back to 3 again, with no way to go back to 1. This can be separately useful, but seems to be solving a different feature request.

I would be happy to implement the "in order" next/previous version, but I'm not sure if that goes against the grand plan.

@zadjii-msft
Copy link
Member

You know, that's a fair point. We never did come up with an in-order traversal method - that was seemingly lost part of the discussion. I think we got so focused on the MRU previous element, the in-order bit was lost.

I'll bring this up in team sync. See what other folks think. Thanks for bringing this up!

@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Aug 3, 2021
@zadjii-msft
Copy link
Member

After discussion, yea, we totally forgot about the inOrder version of this. "moveFocus": "nextInOrder" and "moveFocus": "previousInOrder" seemed like they were likely candidates for the param to mean "take me to ++paneId" / "take me to --paneId".

I'll move this to a new issue. Thanks for keeping us honest!

@zadjii-msft zadjii-msft removed the Needs-Discussion Something that requires a team discussion before we can proceed label Aug 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
Specification Tracker
  
Spec In Review ⏰
Development

Successfully merging a pull request may close this issue.

5 participants