aux window - improve window focus management (microsoft#196576)

bpasero · web-flow · commit 17dd2608a86b · 2023-10-25T13:46:08.000+02:00
* aux window - improve window focus management

* .
diff --git a/src/vs/workbench/browser/composite.ts b/src/vs/workbench/browser/composite.ts
@@ -168,13 +168,6 @@ export abstract class Composite extends Component implements IComposite {
 	 */
 	abstract setBoundarySashes(sashes: IBoundarySashes): void;
 
-	/**
-	 * Update the styles of the contents of this composite.
-	 */
-	override updateStyles(): void {
-		super.updateStyles();
-	}
-
 	/**
 	 *
 	 * @returns the action runner for this composite
diff --git a/src/vs/workbench/browser/layout.ts b/src/vs/workbench/browser/layout.ts
@@ -5,7 +5,7 @@
 
 import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
 import { Event, Emitter } from 'vs/base/common/event';
-import { EventType, addDisposableListener, getClientArea, Dimension, position, size, IDimension, isAncestorUsingFlowTo, computeScreenAwareSize, getActiveDocument, getWindows, getActiveWindow } from 'vs/base/browser/dom';
+import { EventType, addDisposableListener, getClientArea, Dimension, position, size, IDimension, isAncestorUsingFlowTo, computeScreenAwareSize, getActiveDocument, getWindows, getActiveWindow, focusWindow } from 'vs/base/browser/dom';
 import { onDidChangeFullscreen, isFullscreen, isWCOEnabled } from 'vs/base/browser/browser';
 import { IWorkingCopyBackupService } from 'vs/workbench/services/workingCopy/common/workingCopyBackup';
 import { isWindows, isLinux, isMacintosh, isWeb, isNative, isIOS } from 'vs/base/common/platform';
@@ -1031,6 +1031,11 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
 	}
 
 	focusPart(part: Parts): void {
+		const container = this.getContainer(part);
+		if (container) {
+			focusWindow(container);
+		}
+
 		switch (part) {
 			case Parts.EDITOR_PART:
 				this.editorGroupService.activeGroup.focus();
@@ -1051,8 +1056,6 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
 			case Parts.STATUSBAR_PART:
 				this.statusBarService.focus();
 			default: {
-				// Title Bar & Banner simply pass focus to container
-				const container = this.getContainer(part);
 				container?.focus();
 			}
 		}
diff --git a/src/vs/workbench/browser/panecomposite.ts b/src/vs/workbench/browser/panecomposite.ts
@@ -41,6 +41,7 @@ export abstract class PaneComposite extends Composite implements IPaneComposite
 	}
 
 	override create(parent: HTMLElement): void {
+		super.create(parent);
 		this.viewPaneContainer = this._register(this.createViewPaneContainer(parent));
 		this._register(this.viewPaneContainer.onTitleAreaUpdate(() => this.updateTitleArea()));
 		this.viewPaneContainer.create(parent);
@@ -148,9 +149,8 @@ export abstract class PaneComposite extends Composite implements IPaneComposite
 	}
 
 	override focus(): void {
-		this.viewPaneContainer?.focus();
-
 		super.focus();
+		this.viewPaneContainer?.focus();
 	}
 
 	protected abstract createViewPaneContainer(parent: HTMLElement): ViewPaneContainer;
diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts
@@ -11,7 +11,7 @@ import { EditorInput } from 'vs/workbench/common/editor/editorInput';
 import { SideBySideEditorInput } from 'vs/workbench/common/editor/sideBySideEditorInput';
 import { Emitter, Relay } from 'vs/base/common/event';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
-import { Dimension, trackFocus, addDisposableListener, EventType, EventHelper, findParentWithClass, isAncestor, IDomNodePagePosition, isMouseEvent, isActiveElement } from 'vs/base/browser/dom';
+import { Dimension, trackFocus, addDisposableListener, EventType, EventHelper, findParentWithClass, isAncestor, IDomNodePagePosition, isMouseEvent, isActiveElement, focusWindow } from 'vs/base/browser/dom';
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
 import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
 import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar';
@@ -899,6 +899,9 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 
 	focus(): void {
 
+		// Ensure window focus
+		focusWindow(this.element);
+
 		// Pass focus to editor panes
 		if (this.activeEditorPane) {
 			this.activeEditorPane.focus();
diff --git a/src/vs/workbench/browser/parts/editor/editorPlaceholder.ts b/src/vs/workbench/browser/parts/editor/editorPlaceholder.ts
@@ -166,9 +166,9 @@ export abstract class EditorPlaceholder extends EditorPane {
 	}
 
 	override focus(): void {
-		this.container?.focus();
-
 		super.focus();
+
+		this.container?.focus();
 	}
 
 	override dispose(): void {
diff --git a/src/vs/workbench/browser/parts/editor/sideBySideEditor.ts b/src/vs/workbench/browser/parts/editor/sideBySideEditor.ts
@@ -418,9 +418,9 @@ export class SideBySideEditor extends AbstractEditorWithViewState<ISideBySideEdi
 	}
 
 	override focus(): void {
-		this.getLastFocusedEditorPane()?.focus();
-
 		super.focus();
+
+		this.getLastFocusedEditorPane()?.focus();
 	}
 
 	private getLastFocusedEditorPane(): EditorPane | undefined {
diff --git a/src/vs/workbench/browser/parts/editor/textCodeEditor.ts b/src/vs/workbench/browser/parts/editor/textCodeEditor.ts
@@ -89,9 +89,9 @@ export abstract class AbstractTextCodeEditor<T extends IEditorViewState> extends
 	}
 
 	override focus(): void {
-		this.editorControl?.focus();
-
 		super.focus();
+
+		this.editorControl?.focus();
 	}
 
 	override hasFocus(): boolean {
diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts
@@ -338,9 +338,9 @@ export class TextDiffEditor extends AbstractTextEditor<IDiffEditorViewState> imp
 	}
 
 	override focus(): void {
-		this.diffEditorControl?.focus();
-
 		super.focus();
+
+		this.diffEditorControl?.focus();
 	}
 
 	override hasFocus(): boolean {
diff --git a/src/vs/workbench/browser/parts/views/viewPane.ts b/src/vs/workbench/browser/parts/views/viewPane.ts
@@ -8,7 +8,7 @@ import * as nls from 'vs/nls';
 import { Event, Emitter } from 'vs/base/common/event';
 import { asCssVariable, foreground } from 'vs/platform/theme/common/colorRegistry';
 import { PANEL_BACKGROUND, SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme';
-import { after, append, $, trackFocus, EventType, addDisposableListener, createCSSRule, asCSSUrl, Dimension, reset, asCssValueWithDefault } from 'vs/base/browser/dom';
+import { after, append, $, trackFocus, EventType, addDisposableListener, createCSSRule, asCSSUrl, Dimension, reset, asCssValueWithDefault, focusWindow } from 'vs/base/browser/dom';
 import { DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
 import { Action, IAction, IActionRunner } from 'vs/base/common/actions';
 import { ActionsOrientation, IActionViewItem, prepareActions } from 'vs/base/browser/ui/actionbar/actionbar';
@@ -622,6 +622,8 @@ export abstract class ViewPane extends Pane implements IView {
 	}
 
 	focus(): void {
+		focusWindow(this.element);
+
 		if (this.viewWelcomeController.enabled) {
 			this.viewWelcomeController.focus();
 		} else if (this.element) {
diff --git a/src/vs/workbench/browser/parts/views/viewPaneContainer.ts b/src/vs/workbench/browser/parts/views/viewPaneContainer.ts
@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { addDisposableListener, Dimension, DragAndDropObserver, EventType, isAncestor } from 'vs/base/browser/dom';
+import { addDisposableListener, Dimension, DragAndDropObserver, EventType, focusWindow, isAncestor } from 'vs/base/browser/dom';
 import { StandardMouseEvent } from 'vs/base/browser/mouseEvent';
 import { EventType as TouchEventType, Gesture } from 'vs/base/browser/touch';
 import { IActionViewItem } from 'vs/base/browser/ui/actionbar/actionbar';
@@ -600,16 +600,21 @@ export class ViewPaneContainer extends Component implements IViewPaneContainer {
 	}
 
 	focus(): void {
+		let paneToFocus: ViewPane | undefined = undefined;
 		if (this.lastFocusedPane) {
-			this.lastFocusedPane.focus();
+			paneToFocus = this.lastFocusedPane;
 		} else if (this.paneItems.length > 0) {
-			for (const { pane: pane } of this.paneItems) {
+			for (const { pane } of this.paneItems) {
 				if (pane.isExpanded()) {
-					pane.focus();
-					return;
+					paneToFocus = pane;
+					break;
 				}
 			}
 		}
+		if (paneToFocus) {
+			focusWindow(paneToFocus.element);
+			paneToFocus.focus();
+		}
 	}
 
 	private get orientation(): Orientation {
diff --git a/src/vs/workbench/contrib/chat/browser/chatEditor.ts b/src/vs/workbench/contrib/chat/browser/chatEditor.ts
@@ -72,11 +72,9 @@ export class ChatEditor extends EditorPane {
 	}
 
 	public override focus(): void {
-		if (this.widget) {
-			this.widget.focusInput();
-		}
-
 		super.focus();
+
+		this.widget?.focusInput();
 	}
 
 	override clearInput(): void {
diff --git a/src/vs/workbench/contrib/comments/browser/commentsView.ts b/src/vs/workbench/contrib/comments/browser/commentsView.ts
@@ -225,6 +225,8 @@ export class CommentsPanel extends FilterViewPane implements ICommentsView {
 	}
 
 	public override focus(): void {
+		super.focus();
+
 		const element = this.tree?.getHTMLElement();
 		if (element && dom.isActiveElement(element)) {
 			return;
diff --git a/src/vs/workbench/contrib/debug/browser/callStackView.ts b/src/vs/workbench/contrib/debug/browser/callStackView.ts
@@ -389,6 +389,7 @@ export class CallStackView extends ViewPane {
 	}
 
 	override focus(): void {
+		super.focus();
 		this.tree.domFocus();
 	}
 
diff --git a/src/vs/workbench/contrib/debug/browser/repl.ts b/src/vs/workbench/contrib/debug/browser/repl.ts
@@ -506,6 +506,7 @@ export class Repl extends FilterViewPane implements IHistoryNavigationWidget {
 	}
 
 	override focus(): void {
+		super.focus();
 		setTimeout(() => this.replInput.focus(), 0);
 	}
 
diff --git a/src/vs/workbench/contrib/debug/browser/variablesView.ts b/src/vs/workbench/contrib/debug/browser/variablesView.ts
@@ -191,6 +191,7 @@ export class VariablesView extends ViewPane {
 	}
 
 	override focus(): void {
+		super.focus();
 		this.tree.domFocus();
 	}
 
diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts
@@ -181,6 +181,7 @@ export class WatchExpressionsView extends ViewPane {
 	}
 
 	override focus(): void {
+		super.focus();
 		this.tree.domFocus();
 	}
 
diff --git a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts
@@ -623,9 +623,8 @@ export class ExtensionEditor extends EditorPane {
 	}
 
 	override focus(): void {
-		this.activeElement?.focus();
-
 		super.focus();
+		this.activeElement?.focus();
 	}
 
 	showFind(): void {
diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsViewlet.ts b/src/vs/workbench/contrib/extensions/browser/extensionsViewlet.ts
@@ -635,6 +635,7 @@ export class ExtensionsViewPaneContainer extends ViewPaneContainer implements IE
 	}
 
 	override focus(): void {
+		super.focus();
 		this.searchBox?.focus();
 	}
 
diff --git a/src/vs/workbench/contrib/files/browser/explorerViewlet.ts b/src/vs/workbench/contrib/files/browser/explorerViewlet.ts
@@ -37,7 +37,7 @@ import { OpenRecentAction } from 'vs/workbench/browser/actions/windowActions';
 import { isMacintosh, isWeb } from 'vs/base/common/platform';
 import { Codicon } from 'vs/base/common/codicons';
 import { registerIcon } from 'vs/platform/theme/common/iconRegistry';
-import { isMouseEvent } from 'vs/base/browser/dom';
+import { focusWindow, isMouseEvent } from 'vs/base/browser/dom';
 
 const explorerViewIcon = registerIcon('explorer-view-icon', Codicon.files, localize('explorerViewIcon', 'View icon of the explorer view.'));
 const openEditorsViewIcon = registerIcon('open-editors-view-icon', Codicon.book, localize('openEditorsIcon', 'View icon of the open editors view.'));
@@ -239,6 +239,7 @@ export class ExplorerViewPaneContainer extends ViewPaneContainer {
 			explorerView.setExpanded(true);
 		}
 		if (explorerView?.isExpanded()) {
+			focusWindow(explorerView.element);
 			explorerView.focus();
 		} else {
 			super.focus();
diff --git a/src/vs/workbench/contrib/files/browser/views/explorerView.ts b/src/vs/workbench/contrib/files/browser/views/explorerView.ts
@@ -316,6 +316,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
 	}
 
 	override focus(): void {
+		super.focus();
 		this.tree.domFocus();
 
 		const focused = this.tree.getFocus();
diff --git a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts
@@ -673,10 +673,10 @@ export class InteractiveEditor extends EditorPane {
 	}
 
 	override focus() {
+		super.focus();
+
 		this._notebookWidget.value?.onShow();
 		this._codeEditorWidget.focus();
-
-		super.focus();
 	}
 
 	focusHistory() {
diff --git a/src/vs/workbench/contrib/markers/browser/markersView.ts b/src/vs/workbench/contrib/markers/browser/markersView.ts
@@ -236,6 +236,7 @@ export class MarkersView extends FilterViewPane implements IMarkersView {
 	}
 
 	public override focus(): void {
+		super.focus();
 		if (dom.isActiveElement(this.widget.getHTMLElement())) {
 			return;
 		}
diff --git a/src/vs/workbench/contrib/mergeEditor/browser/view/mergeEditor.ts b/src/vs/workbench/contrib/mergeEditor/browser/view/mergeEditor.ts
@@ -453,9 +453,9 @@ export class MergeEditor extends AbstractTextEditor<IMergeEditorViewState> {
 	}
 
 	override focus(): void {
-		(this.getControl() ?? this.inputResultView.editor).focus();
-
 		super.focus();
+
+		(this.getControl() ?? this.inputResultView.editor).focus();
 	}
 
 	override hasFocus(): boolean {
diff --git a/src/vs/workbench/contrib/outline/browser/outlinePane.ts b/src/vs/workbench/contrib/outline/browser/outlinePane.ts
@@ -124,6 +124,7 @@ export class OutlinePane extends ViewPane implements IOutlinePane {
 	}
 
 	override focus(): void {
+		super.focus();
 		this._tree?.domFocus();
 	}
 
diff --git a/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts b/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts
@@ -186,14 +186,14 @@ export class KeybindingsEditor extends EditorPane implements IKeybindingsEditorP
 	}
 
 	override focus(): void {
+		super.focus();
+
 		const activeKeybindingEntry = this.activeKeybindingEntry;
 		if (activeKeybindingEntry) {
 			this.selectEntry(activeKeybindingEntry);
 		} else if (!isIOS) {
 			this.searchWidget.focus();
 		}
-
-		super.focus();
 	}
 
 	get activeKeybindingEntry(): IKeybindingItemEntry | null {
diff --git a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts
@@ -474,6 +474,8 @@ export class SettingsEditor2 extends EditorPane {
 	}
 
 	override focus(): void {
+		super.focus();
+
 		if (this._currentFocusContext === SettingsFocusContext.Search) {
 			if (!platform.isIOS) {
 				// #122044
@@ -493,8 +495,6 @@ export class SettingsEditor2 extends EditorPane {
 		} else if (this._currentFocusContext === SettingsFocusContext.TableOfContents) {
 			this.tocTree.domFocus();
 		}
-
-		super.focus();
 	}
 
 	protected override setEditorVisible(visible: boolean, group: IEditorGroup | undefined): void {
diff --git a/src/vs/workbench/contrib/scm/browser/scmRepositoriesViewPane.ts b/src/vs/workbench/contrib/scm/browser/scmRepositoriesViewPane.ts
@@ -117,6 +117,7 @@ export class SCMRepositoriesViewPane extends ViewPane {
 	}
 
 	override focus(): void {
+		super.focus();
 		this.list.domFocus();
 	}
 
diff --git a/src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts b/src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts
@@ -272,14 +272,14 @@ export class SearchEditor extends AbstractTextCodeEditor<SearchEditorViewState>
 	}
 
 	override focus() {
+		super.focus();
+
 		const viewState = this.loadEditorViewState(this.getInput());
 		if (viewState && viewState.focused === 'editor') {
 			this.searchResultEditor.focus();
 		} else {
 			this.queryEditorWidget.focus();
 		}
-
-		super.focus();
 	}
 
 	focusSearchInput() {
diff --git a/src/vs/workbench/contrib/terminal/browser/terminalEditor.ts b/src/vs/workbench/contrib/terminal/browser/terminalEditor.ts
@@ -99,9 +99,9 @@ export class TerminalEditor extends EditorPane {
 	}
 
 	override focus() {
-		this._editorInput?.terminalInstance?.focus();
-
 		super.focus();
+
+		this._editorInput?.terminalInstance?.focus();
 	}
 
 	// eslint-disable-next-line @typescript-eslint/naming-convention
diff --git a/src/vs/workbench/contrib/terminal/browser/terminalView.ts b/src/vs/workbench/contrib/terminal/browser/terminalView.ts
diff --git a/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts b/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts
diff --git a/src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts b/src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts
diff --git a/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts b/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts
diff --git a/src/vs/workbench/electron-sandbox/window.ts b/src/vs/workbench/electron-sandbox/window.ts
diff --git a/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts b/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts

Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,7 @@ export abstract class PaneComposite extends Composite implements IPaneComposite`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`override create(parent: HTMLElement): void {`
	`44`	`+ super.create(parent);`
`44`	`45`	`this.viewPaneContainer = this._register(this.createViewPaneContainer(parent));`
`45`	`46`	`this._register(this.viewPaneContainer.onTitleAreaUpdate(() => this.updateTitleArea()));`
`46`	`47`	`this.viewPaneContainer.create(parent);`
`@@ -148,9 +149,8 @@ export abstract class PaneComposite extends Composite implements IPaneComposite`
`148`	`149`	`}`
`149`	`150`
`150`	`151`	`override focus(): void {`
`151`		`- this.viewPaneContainer?.focus();`
`152`		`-`
`153`	`152`	`super.focus();`
	`153`	`+ this.viewPaneContainer?.focus();`
`154`	`154`	`}`
`155`	`155`
`156`	`156`	`protected abstract createViewPaneContainer(parent: HTMLElement): ViewPaneContainer;`
Original file line number	Diff line number	Diff line change
`@@ -166,9 +166,9 @@ export abstract class EditorPlaceholder extends EditorPane {`
`166`	`166`	`}`
`167`	`167`
`168`	`168`	`override focus(): void {`
`169`		`- this.container?.focus();`
`170`		`-`
`171`	`169`	`super.focus();`
	`170`	`+`
	`171`	`+ this.container?.focus();`
`172`	`172`	`}`
`173`	`173`
`174`	`174`	`override dispose(): void {`
Original file line number	Diff line number	Diff line change
`@@ -418,9 +418,9 @@ export class SideBySideEditor extends AbstractEditorWithViewState<ISideBySideEdi`
`418`	`418`	`}`
`419`	`419`
`420`	`420`	`override focus(): void {`
`421`		`- this.getLastFocusedEditorPane()?.focus();`
`422`		`-`
`423`	`421`	`super.focus();`
	`422`	`+`
	`423`	`+ this.getLastFocusedEditorPane()?.focus();`
`424`	`424`	`}`
`425`	`425`
`426`	`426`	`private getLastFocusedEditorPane(): EditorPane \| undefined {`
Original file line number	Diff line number	Diff line change
`@@ -89,9 +89,9 @@ export abstract class AbstractTextCodeEditor<T extends IEditorViewState> extends`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`override focus(): void {`
`92`		`- this.editorControl?.focus();`
`93`		`-`
`94`	`92`	`super.focus();`
	`93`	`+`
	`94`	`+ this.editorControl?.focus();`
`95`	`95`	`}`
`96`	`96`
`97`	`97`	`override hasFocus(): boolean {`
Original file line number	Diff line number	Diff line change
`@@ -338,9 +338,9 @@ export class TextDiffEditor extends AbstractTextEditor<IDiffEditorViewState> imp`
`338`	`338`	`}`
`339`	`339`
`340`	`340`	`override focus(): void {`
`341`		`- this.diffEditorControl?.focus();`
`342`		`-`
`343`	`341`	`super.focus();`
	`342`	`+`
	`343`	`+ this.diffEditorControl?.focus();`
`344`	`344`	`}`
`345`	`345`
`346`	`346`	`override hasFocus(): boolean {`