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

Should panel toggle keybindings focus instead of toggle when the panel is not active? #19400

Closed
Tyriar opened this issue Jan 25, 2017 · 32 comments
Assignees
Labels
feature-request Request for new features or functionality layout General VS Code workbench layout issues ux User experience issues verification-needed Verification of issue is requested verified Verification succeeded

Comments

@Tyriar
Copy link
Member

Tyriar commented Jan 25, 2017

Currently all the panel toggle commands hide the respective panel when it's visible and show it when it's hidden. I find this annoying though and instead use the following custom keybinding:

{
  "key": "ctrl+`",
  "command": "workbench.action.terminal.focus",
  "when": "!terminalFocus"
}

This overrides the setting when the terminal does not have focus to simply focus it, but retain the hide behavior when it is focused. This means you don't have to hit ctrl+` twice in order to get focus to the terminal when it's already displayed but not focused. This does change the hiding the terminal case when it's not focused to need 2 ctrl+`'s but personally I hardly ever do this compared to switching focus from the editor to the terminal.

I frequently tell users about my config and they all react positively so I'm wondering whether we should make this the default behavior for all panels (to keep them consistent).

/discuss

@Tyriar Tyriar added *question Issue represents a question, should be posted to StackOverflow (VS Code) ux User experience issues workbench labels Jan 25, 2017
@daviwil
Copy link
Contributor

daviwil commented Jan 25, 2017

I believe this should be the default behavior because it aligns with user intent: no matter where I'm at in the editor, if I hit the hotkey it should bring me to the respective panel. If I'm already there I probably want to toggle it.

@bpasero
Copy link
Member

bpasero commented Jan 26, 2017

I am ok changing this, the sidebar behaves the same (focus in when not focussed and invoking the keybinding). As long as we provide a way to configure the old behaviour.

@bpasero bpasero removed their assignment Jan 26, 2017
@kaiwood
Copy link
Contributor

kaiwood commented Jan 26, 2017

Should be changed, yes. I'm using almost exclusively using workbench.action.focusPanel as soon as the terminal is active because I can't stand the do-2-things-at-once-behaviour of workbench.action.terminal.focus.

Consistency with the sidebar would be a big plus 👍

@isidorn
Copy link
Contributor

isidorn commented Jan 26, 2017

I am fine with changing this to behave like the sidebar. Especially if the users seem to prefer this. They can always use cmd + J to toggle

@isidorn isidorn removed their assignment Jan 26, 2017
@sandy081
Copy link
Member

This was brought up already before - #7540

@dbaeumer
Copy link
Member

+1 for the change.

@gandhis1
Copy link

I cannot emphasize how many times I have had to invoke the shortcut twice solely to regain focus on the terminal. Definitely a usability flaw

@Tyriar
Copy link
Member Author

Tyriar commented Jan 31, 2017

@isidorn to be clear I'm proposing a conditional focus though so it would not be consistent with the sidebar, just a little more like the sidebar's behavior.

@sqlaide
Copy link

sqlaide commented Mar 31, 2017

Personally, i would just like to switch cursor between terminal and editor without hiding terminal. This setting works for me.
'{
"key": "ctrl+`",
"command": "workbench.action.terminal.focus",
"when": "!terminalFocus"
},
{
"key": "ctrl+`",
"command": "workbench.action.focusActiveEditorGroup",
"when": "terminalFocus"
}'

@Tyriar Tyriar added feature-request Request for new features or functionality and removed *question Issue represents a question, should be posted to StackOverflow (VS Code) labels Apr 21, 2017
@ericis
Copy link

ericis commented Aug 18, 2017

This keybinding works in v1.15.0 ctrl+Alt+'

However, it's one of the most awkward key combinations ever!

@Tyriar
Copy link
Member Author

Tyriar commented Sep 6, 2017

@dbaeumer @isidorn @sandy081 this is currently blocked on when context clauses for output, problems and tasks which I don't think exist currently https://code.visualstudio.com/docs/getstarted/keybindings#_when-clause-contexts

I'd be happy to make the keybinding changes after the contexts are implemented.

@isidorn isidorn removed their assignment Oct 27, 2017
@bpasero bpasero added layout General VS Code workbench layout issues and removed workbench labels Nov 16, 2017
@Tyriar
Copy link
Member Author

Tyriar commented Sep 14, 2018

@bpasero I'm not suggesting adding another command, rather adding new default keybindings:

{
  "key": "ctrl+`",
  "command": "workbench.action.terminal.focus",
  "when": "!terminalFocus"
},
{
  "key": "shift+ctrl+m",
  "command": "workbench.action.problems.focus",
  "when": "!problemsFocus"
},
...

When the panel is open and focused: It should focus the editor (so you can jump in and out)

This doesn't really solve the problem though imo, I use ctrl/cmd+1 when I want to switch back to the editor personally. The above keybindings just makes it nicer to work with the terminal such that you don't need to trigger toggle terminal twice if it's already visible. I often recommend people use this setup for toggle terminal and I've only heard positive feedback.

@bpasero
Copy link
Member

bpasero commented Sep 14, 2018

@Tyriar I still dislike the fact that viewlets behave differently from panels

bpasero added a commit that referenced this issue Sep 17, 2018
@bpasero
Copy link
Member

bpasero commented Sep 17, 2018

@Tyriar I have a change ready in c7b740f that would change every panel toggle action to behave as outlined in #19400 (comment). I do not think we can just fix this via changing our default keybindings, because what about people that have changed the keybindings manually?

@Tyriar
Copy link
Member Author

Tyriar commented Sep 17, 2018

@bpasero looks ok, one thing to keep in mind is that doing it this way makes you unable to hide the panels via the command palette. You can still use "toggle panel", but "toggle terminal" will actually focus the terminal since the command palette was focused.

@bpasero
Copy link
Member

bpasero commented Sep 18, 2018

Good point, I suggest we talk about this in the UX call.

@bpasero bpasero self-assigned this Sep 20, 2018
@bpasero bpasero added this to the September 2018 milestone Sep 20, 2018
@bpasero bpasero closed this as completed Sep 20, 2018
@bpasero
Copy link
Member

bpasero commented Sep 20, 2018

The behaviour of the commands for toggling panels has changed as outlined in #19400 (comment). To get back the previous behaviour, simply configure a keybinding to close the panel when the related panel is active, e.g. for output:

{
	"key": "cmd+shift+u",
	"command": "workbench.action.closePanel",
	"when": "activePanel==workbench.panel.output"
}

The list of panel identifiers is:

  • workbench.panel.terminal
  • workbench.panel.markers
  • workbench.panel.output
  • workbench.panel.repl

@bpasero bpasero added the verification-needed Verification of issue is requested label Sep 24, 2018
@bpasero
Copy link
Member

bpasero commented Sep 24, 2018

Verification: see #19400 (comment)

@isidorn isidorn added the verified Verification succeeded label Sep 26, 2018
@isidorn
Copy link
Contributor

isidorn commented Sep 26, 2018

@bpasero I think you took @Tyriar comment a bit too literarly. I verified it works great, however there is one corner case which is a regression, thus reopening

  1. View > terminal -> terminal gets nicely opened
  2. View > output -> panel gets closed. Instead the output should be shown

@isidorn isidorn reopened this Sep 26, 2018
@bpasero
Copy link
Member

bpasero commented Sep 26, 2018

Wow, not sure how I did not test that. Fixed, thanks!

@bpasero bpasero closed this as completed Sep 26, 2018
@bpasero bpasero added verification-found Issue verification failed and removed verified Verification succeeded labels Sep 26, 2018
bpasero added a commit that referenced this issue Sep 26, 2018
@bpasero bpasero removed the verification-found Issue verification failed label Sep 27, 2018
@roblourens roblourens added the verified Verification succeeded label Sep 27, 2018
@plagov
Copy link

plagov commented Oct 21, 2018

I found the link to this issue in release notes for the 1.28.2 release and I like the way how ctrl + ` behaves now. I also was annoyed by the fact that I have to type terminal shortcut twice if I want to move back to the terminal.

But what shortcut can I use to move back from the terminal to an editor? I mean, I don't want to close a terminal panel. I want to leave it open and switch focus to an editor. A use case for this would be the following: I will write in the editor a code that depends on the result of a teminal command that I just run. So I want to move focus from terminal to an editor and leave terminal open.

@isidorn
Copy link
Contributor

isidorn commented Oct 22, 2018

@plagov just add a custom shortcut to this command
screen shot 2018-10-22 at 10 17 27

@alekseyt
Copy link

@plagov try Ctrl+P Esc

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality layout General VS Code workbench layout issues ux User experience issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests