Restore focus to the last focused side in a diff editor

[RMacfarlane]

Issue Type: Bug

Similar to #22403

1. Open a diff view
2. Put cursor in left side editor
3. Open the command palette, and either run a command or close it

Instead of focus returning to the left side, it is shifted to the right. When the quick open hides, it's trying to focus back on the old editor
https://github.com/Microsoft/vscode/blob/d315d18deec4b9ece7f3cbb9d74a6916bb9718e8/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts#L281

but the diff editor always focuses the modified side:
https://github.com/Microsoft/vscode/blob/d315d18deec4b9ece7f3cbb9d74a6916bb9718e8/src/vs/editor/browser/widget/diffEditorWidget.ts#L758

I'm trying to write a command for starting a new comment thread, so I need to be able to get the focused editor. But the focus is always changed before the command runs, so there's no way to tell which editor to use

VS Code version: Code - Insiders 1.32.0-insider (507312a, 2019-03-06T11:10:01.691Z)
OS version: Darwin x64 18.2.0

[alexdima]

The diff editor is considered to be a single editor by our workbench and when .focus() is called, it focuses the right side. The same bug exists, for example, when invoking the quick open from the find widget... focus is not returned to the find widget... Another instance of the same issue is invoking it from an embedded editor like the one in peek definition.

I suggest the quick open checks where focus is before it is invoked and then restores focus when it is dismissed.

[bpasero]

I acknowledge the problem but would not want to change the default here simply because the left hand editor very often is not writable and this could cause frustration.

Having an option for the focus() method to consider diff editors as opt-in could work though. Feel free to spin up a PR for that. I would however not blindly use that for each command from the command palette for these reasons.

[alexdima]

@bpasero Please check my suggestion:

I suggest the quick open checks where focus is before it is invoked and then restores focus when it is dismissed

i.e. record document.activeElement when quick open is shown and return focus there when it finishes, like the custom menus.

[bpasero]

@alexandrudima quick open was never tracking document.activeElement so far but would always try to restore focus to the active editor by calling editor.focus(). I got a lot of complaints in the past that after using quick open, focus was not in the editor (e.g. because focus was somewhere else) so we changed this a long time ago and I think we should keep that behaviour.

Thus, to make this work properly, you would need to change diffEditorWidget.ts#focus() to restore focus to the left or right hand side based on what was the last active side:

[chrmarti]

This is a more general problem. We could track the focus for the entire workbench and when QuickOpen closes, we would just return to the previous active element.

The current solution appears to be to always return focus to the editor, which is not always correct, but has worked surprisingly well so far.

[alexdima]

@bpasero My recommendation is to read document.activeElement when quick open is shown and focus back to that element when quick open is hidden. This is completely unrelated to the diff editor. This would cover the case when focus is in the find widget and quick open is invoked, focus would return to the find widget. Similar to how the custom menus work.

[bpasero]

@alexandrudima we can do that, just worried that this is changing a pattern people might have gotten used to. Today we give the editor some extra treatment by always moving focus into it when any quick picker closes. We also have to make 100% sure that the command that executes from quick open does not require the editor to be focused. I remember in the past we had some issues with that where focus was not back in the editor but the command assumed that.

@chrmarti btw you are doing the same for quickInput:

https://github.com/Microsoft/vscode/blob/3e2e048b9a3d65c8c47b309e01186bbf788ee6d8/src/vs/workbench/browser/parts/quickinput/quickInput.ts#L1386

I do not see myself doing a lot of changes for old quick open given I want quick open to move over to your widget (via #69955)

[chrmarti]

/duplicate #44722

[vscodebot]

Thanks for creating this issue! We figured it's covering the same as another one we already have. Thus, we closed this one as a duplicate. You can search for existing issues here. See also our issue reporting guidelines.

Happy Coding!