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

Select To Next/Previous Command loses terminal focus when invoked from the command palette. #46961

Closed
sbatten opened this issue Mar 29, 2018 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues verified Verification succeeded
Milestone

Comments

@sbatten
Copy link
Member

sbatten commented Mar 29, 2018

Issue Type: Bug

When invoking Select to next/previous command from the command palette, the terminal loses focus and causes confusion.

VS Code version: Code - Insiders 1.22.0-insider (952b2a6, 2018-03-29T05:17:25.524Z)
OS version: Windows_NT x64 10.0.16299

@sbatten sbatten added the bug Issue identified by VS Code Team member as probable bug label Mar 29, 2018
@vscodebot vscodebot bot added the insiders label Mar 29, 2018
@Tyriar Tyriar added this to the April 2018 milestone Mar 29, 2018
@Tyriar Tyriar added the terminal Integrated terminal issues label Mar 29, 2018
@knyhle
Copy link
Contributor

knyhle commented Mar 30, 2018

I don't think it's just the Select to Next / Previous Command. Anytime the command palette is invoked, the focus goes to the workbench(? main window?) and anything else will lose focus.

@Tyriar
Copy link
Member

Tyriar commented Mar 30, 2018

@kennyle1412 when the command deals with focus or selection we should definitely make sure focus works. Maybe there's a way we can do this generally @bpasero?

@bpasero
Copy link
Member

bpasero commented Mar 30, 2018

@Tyriar currently we just always put focus back to the editor which is probably not clever and can be improved. The tricky part is though that some commands actually might operate on the editor and it would make sense to have focus there. So maybe we should find those commands that want to have focus elsewhere and fix those instead of changing it for all. I think it is fair to say that in most cases you want focus back in the editor after quick commands close.

@knyhle
Copy link
Contributor

knyhle commented Mar 30, 2018

@Tyriar Then I guess every terminalAction that deals with those things should do a terminal.focus() before it returns. That should work for when the action actually goes though how efficient that is I can't say.

However, if the quick command is invoked then cancelled, we'd be out of luck since nothing is executed haha. We can try to save the previousFocus when invoking quick command and refocus previousFocus if the command palette is cancelled (/ nothing selected) instead of forcing it to the editor or only focus the editor if a command is chosen. Though I haven't looked at the quick command code yet so I'm not sure if those are feasible.

@bpasero that way we can still focus to the editor in the general case, but don't change anything when the action is cancelled.

@Tyriar
Copy link
Member

Tyriar commented Mar 30, 2018

@bpasero how about adding something like this to IAction?

export interface IAction {
  restoreFocus(): TPromise<any>
}

Which would be run to restore focus when it's implemented (or noop if not) when escape is pretty or the command is triggered.

@bpasero
Copy link
Member

bpasero commented Mar 31, 2018

@Tyriar I would stay away from adding anything to IAction, that interface is only meant to give a runnable an identifier and a label.

Can you not just change focus from your action when it runs? Maybe we need to pass some extra context into the actions that get executed from the command palette.

@Tyriar
Copy link
Member

Tyriar commented Apr 2, 2018

@bpasero but there's the case @kennyle1412 brought up where cancelling the command palette will then focus the editor as run is not triggered.

@Tyriar Tyriar closed this as completed in 2f25cb8 Apr 5, 2018
@rebornix rebornix added the verified Verification succeeded label Apr 25, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators May 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants