Merge pull request microsoft#183090 from jeanp413/fix-183087

Fix cannot hide terminal find widget when pressing `esc`

Author: Tyriar

src/vs/workbench/contrib/terminal/browser/terminal.ts

@@ -2,7 +2,7 @@
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
-import { Dimension } from 'vs/base/browser/dom';
+import { IDimension } from 'vs/base/browser/dom';
 import { Orientation } from 'vs/base/browser/ui/splitview/splitview';
 import { Color } from 'vs/base/common/color';
 import { Event } from 'vs/base/common/event';
@@ -37,7 +37,7 @@ export const ITerminalInstanceService = createDecorator<ITerminalInstanceService
  * been initialized.
  */
 export interface ITerminalContribution extends IDisposable {
-	layout?(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void;
+	layout?(xterm: IXtermTerminal & { raw: RawXtermTerminal }, dimension: IDimension): void;
 	xtermReady?(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void;
 }
 
@@ -434,7 +434,7 @@ export interface ITerminalInstance {
 	readonly maxRows: number;
 	readonly fixedCols?: number;
 	readonly fixedRows?: number;
-	readonly container?: HTMLElement;
+	readonly domElement: HTMLElement;
 	readonly icon?: TerminalIcon;
 	readonly color?: string;
 	readonly reconnectionProperties?: IReconnectionProperties;
@@ -936,25 +936,12 @@ export interface ITerminalInstance {
 	 */
 	resetScrollbarVisibility(): void;
 
-	/**
-	 * Register a child element to the terminal instance's container.
-	 */
-	registerChildElement(element: ITerminalChildElement): IDisposable;
-
 	/**
 	 * Gets a terminal contribution by its ID.
 	 */
 	getContribution<T extends ITerminalContribution>(id: string): T | null;
 }
 
-// NOTE: This interface is very similar to the widget manager internal to TerminalInstance, in the
-// future these should probably be consolidated.
-export interface ITerminalChildElement {
-	element: HTMLElement;
-	layout?(dimension: Dimension): void;
-	xtermReady?(xterm: IXtermTerminal): void;
-}
-
 export const enum XtermTerminalConstants {
 	SearchHighlightLimit = 1000
 }

src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

@@ -17,7 +17,7 @@ import { ErrorNoTelemetry, onUnexpectedError } from 'vs/base/common/errors';
 import { Emitter, Event } from 'vs/base/common/event';
 import { KeyCode } from 'vs/base/common/keyCodes';
 import { ISeparator, template } from 'vs/base/common/labels';
-import { Disposable, DisposableStore, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
+import { Disposable, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 import { Schemas } from 'vs/base/common/network';
 import * as path from 'vs/base/common/path';
 import { isMacintosh, isWindows, OperatingSystem, OS } from 'vs/base/common/platform';
@@ -53,7 +53,7 @@ import { IWorkspaceContextService, IWorkspaceFolder } from 'vs/platform/workspac
 import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust';
 import { IViewDescriptorService, IViewsService, ViewContainerLocation } from 'vs/workbench/common/views';
 import { TaskSettingId } from 'vs/workbench/contrib/tasks/common/tasks';
-import { IRequestAddInstanceToGroupEvent, ITerminalChildElement, ITerminalContribution, ITerminalInstance, TerminalDataTransfers } from 'vs/workbench/contrib/terminal/browser/terminal';
+import { IRequestAddInstanceToGroupEvent, ITerminalContribution, ITerminalInstance, TerminalDataTransfers } from 'vs/workbench/contrib/terminal/browser/terminal';
 import { TerminalLaunchHelpAction } from 'vs/workbench/contrib/terminal/browser/terminalActions';
 import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper';
 import { TerminalEditorInput } from 'vs/workbench/contrib/terminal/browser/terminalEditorInput';
@@ -163,8 +163,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 	private _title: string = '';
 	private _titleSource: TitleEventSource = TitleEventSource.Process;
 	private _container: HTMLElement | undefined;
-	get container(): HTMLElement | undefined { return this._container; }
 	private _wrapperElement: (HTMLElement & { xterm?: XTermTerminal });
+	get domElement(): HTMLElement { return this._wrapperElement; }
 	private _horizontalScrollbar: DomScrollableElement | undefined;
 	private _terminalFocusContextKey: IContextKey<boolean>;
 	private _terminalHasFixedWidth: IContextKey<boolean>;
@@ -335,12 +335,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 	private readonly _onDidChangeHasChildProcesses = this._register(new Emitter<boolean>());
 	readonly onDidChangeHasChildProcesses = this._onDidChangeHasChildProcesses.event;
 
-	// Internal events
-	private readonly _onDidAttachToElement = new Emitter<HTMLElement>();
-	private readonly _onDidAttachToElementEvent = this._onDidAttachToElement.event;
-	private readonly _onDidLayout = new Emitter<dom.Dimension>();
-	private readonly _onDidLayoutEvent = this._onDidLayout.event;
-
 	constructor(
 		private readonly _terminalShellTypeContextKey: IContextKey<string>,
 		private readonly _terminalInRunCommandPicker: IContextKey<boolean>,
@@ -876,7 +870,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		// The container changed, reattach
 		this._container = container;
 		this._container.appendChild(this._wrapperElement);
-		this._onDidAttachToElement.fire(this._container);
 		this.xterm?.refresh();
 
 		setTimeout(() => this._initDragAndDrop(container));
@@ -1837,17 +1830,16 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		}
 
 		this._resize();
-		this._onDidLayout.fire(dimension);
 
 		// Signal the container is ready
 		this._containerReadyBarrier.open();
 
 		// Layout all contributions
 		for (const contribution of this._contributions.values()) {
 			if (!this.xterm) {
-				this._xtermReadyPromise.then(xterm => contribution.layout?.(xterm));
+				this._xtermReadyPromise.then(xterm => contribution.layout?.(xterm, dimension));
 			} else {
-				contribution.layout?.(this.xterm);
+				contribution.layout?.(this.xterm, dimension);
 			}
 		}
 	}
@@ -2284,28 +2276,11 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 	}
 
 	forceScrollbarVisibility(): void {
-		this._container?.classList.add('force-scrollbar');
+		this._wrapperElement.classList.add('force-scrollbar');
 	}
 
 	resetScrollbarVisibility(): void {
-		this._container?.classList.remove('force-scrollbar');
-	}
-
-	registerChildElement(child: ITerminalChildElement): IDisposable {
-		const store = new DisposableStore();
-		if (this._container) {
-			this._container.appendChild(child.element);
-		} else {
-			store.add(this._onDidAttachToElementEvent(container => container.appendChild(child.element)));
-		}
-		if (this._lastLayoutDimensions) {
-			child.layout?.(this._lastLayoutDimensions);
-		}
-		store.add(this._onDidLayoutEvent(e => child.layout?.(e)));
-		if (child.xtermReady) {
-			this._xtermReadyPromise.then(xterm => child.xtermReady!(xterm));
-		}
-		return store;
+		this._wrapperElement.classList.remove('force-scrollbar');
 	}
 }
 

src/vs/workbench/contrib/terminalContrib/find/browser/terminal.find.contribution.ts

@@ -3,65 +3,82 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { IDimension } from 'vs/base/browser/dom';
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
+import { Lazy } from 'vs/base/common/lazy';
+import { Disposable } from 'vs/base/common/lifecycle';
 import { localize } from 'vs/nls';
 import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
-import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
+import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
 import { findInFilesCommand } from 'vs/workbench/contrib/search/browser/searchActionsFind';
-import { ITerminalInstance, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
+import { ITerminalContribution, ITerminalInstance, ITerminalService, IXtermTerminal } from 'vs/workbench/contrib/terminal/browser/terminal';
 import { registerActiveInstanceAction } from 'vs/workbench/contrib/terminal/browser/terminalActions';
-import { TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal';
+import { registerTerminalContribution } from 'vs/workbench/contrib/terminal/browser/terminalExtensions';
+import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager';
+import { ITerminalProcessManager, TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal';
 import { TerminalContextKeys } from 'vs/workbench/contrib/terminal/common/terminalContextKey';
 import { TerminalFindWidget } from 'vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget';
+import { Terminal as RawXtermTerminal } from 'xterm';
 
-const findWidgets: Map<ITerminalInstance, TerminalFindWidget> = new Map();
+class TerminalFindContribution extends Disposable implements ITerminalContribution {
+	static readonly ID = 'terminal.find';
 
-function getFindWidget(instance: ITerminalInstance | undefined, accessor: ServicesAccessor): TerminalFindWidget | undefined {
-	if (instance === undefined) {
-		return undefined;
+	static get(instance: ITerminalInstance): TerminalFindContribution | null {
+		return instance.getContribution<TerminalFindContribution>(TerminalFindContribution.ID);
 	}
-	let result = findWidgets.get(instance);
-	if (!result) {
-		const terminalService = accessor.get(ITerminalService);
-		const widget = accessor.get(IInstantiationService).createInstance(TerminalFindWidget, instance);
-
-		// Track focus and set state so we can force the scroll bar to be visible
-		let focusState = false;
-		widget.focusTracker.onDidFocus(() => {
-			focusState = true;
-			instance.forceScrollbarVisibility();
-			terminalService.setActiveInstance(instance);
-		});
-		widget.focusTracker.onDidBlur(() => {
-			focusState = false;
-			instance.resetScrollbarVisibility();
-		});
 
-		// Attach the find widget and listen for layout
-		instance.registerChildElement({
-			element: widget.getDomNode(),
-			layout: dimension => widget.layout(dimension.width),
-			xtermReady: xterm => {
-				xterm.onDidChangeFindResults(() => widget.updateResultCount());
-			}
-		});
+	private _findWidget: Lazy<TerminalFindWidget>;
+	private _lastLayoutDimensions: IDimension | undefined;
+
+	get findWidget(): TerminalFindWidget { return this._findWidget.value; }
 
-		// Cache the widget while the instance exists, dispose it when the terminal is disposed
-		instance.onDisposed(e => {
-			const focusTerminal = focusState;
-			widget?.dispose();
-			findWidgets.delete(e);
-			if (focusTerminal) {
-				instance.focus();
+	constructor(
+		private readonly _instance: ITerminalInstance,
+		processManager: ITerminalProcessManager,
+		widgetManager: TerminalWidgetManager,
+		@IInstantiationService instantiationService: IInstantiationService,
+		@ITerminalService terminalService: ITerminalService
+	) {
+		super();
+
+		this._findWidget = new Lazy(() => {
+			const findWidget = instantiationService.createInstance(TerminalFindWidget, this._instance);
+
+			// Track focus and set state so we can force the scroll bar to be visible
+			findWidget.focusTracker.onDidFocus(() => {
+				this._instance.forceScrollbarVisibility();
+				terminalService.setActiveInstance(this._instance);
+			});
+			findWidget.focusTracker.onDidBlur(() => {
+				this._instance.resetScrollbarVisibility();
+			});
+
+			this._instance.domElement.appendChild(findWidget.getDomNode());
+			if (this._lastLayoutDimensions) {
+				findWidget.layout(this._lastLayoutDimensions.width);
 			}
+
+			return findWidget;
 		});
+	}
+
+	layout(xterm: IXtermTerminal & { raw: RawXtermTerminal }, dimension: IDimension): void {
+		this._lastLayoutDimensions = dimension;
+		this._findWidget.rawValue?.layout(dimension.width);
+	}
+
+	xtermReady(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void {
+		this._register(xterm.onDidChangeFindResults(() => this._findWidget.rawValue?.updateResultCount()));
+	}
 
-		findWidgets.set(instance, widget);
-		result = widget;
+	override dispose() {
+		super.dispose();
+		this._findWidget.rawValue?.dispose();
 	}
-	return result;
+
 }
+registerTerminalContribution(TerminalFindContribution.ID, TerminalFindContribution);
 
 registerActiveInstanceAction({
 	id: TerminalCommandId.FindFocus,
@@ -72,8 +89,8 @@ registerActiveInstanceAction({
 		weight: KeybindingWeight.WorkbenchContrib
 	},
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		getFindWidget(activeInstance, accessor)?.reveal();
+	run: (activeInstance) => {
+		TerminalFindContribution.get(activeInstance)?.findWidget.reveal();
 	}
 });
 
@@ -87,8 +104,8 @@ registerActiveInstanceAction({
 		weight: KeybindingWeight.WorkbenchContrib
 	},
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		getFindWidget(activeInstance, accessor)?.hide();
+	run: (activeInstance) => {
+		TerminalFindContribution.get(activeInstance)?.findWidget.hide();
 	}
 });
 
@@ -102,11 +119,9 @@ registerActiveInstanceAction({
 		weight: KeybindingWeight.WorkbenchContrib
 	},
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		const state = getFindWidget(activeInstance, accessor)?.state;
-		if (state) {
-			state.change({ matchCase: !state.isRegex }, false);
-		}
+	run: (activeInstance) => {
+		const state = TerminalFindContribution.get(activeInstance)?.findWidget.state;
+		state?.change({ matchCase: !state.isRegex }, false);
 	}
 });
 
@@ -120,11 +135,9 @@ registerActiveInstanceAction({
 		weight: KeybindingWeight.WorkbenchContrib
 	},
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		const state = getFindWidget(activeInstance, accessor)?.state;
-		if (state) {
-			state.change({ matchCase: !state.wholeWord }, false);
-		}
+	run: (activeInstance) => {
+		const state = TerminalFindContribution.get(activeInstance)?.findWidget.state;
+		state?.change({ matchCase: !state.wholeWord }, false);
 	}
 });
 
@@ -138,11 +151,9 @@ registerActiveInstanceAction({
 		weight: KeybindingWeight.WorkbenchContrib
 	},
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		const state = getFindWidget(activeInstance, accessor)?.state;
-		if (state) {
-			state.change({ matchCase: !state.matchCase }, false);
-		}
+	run: (activeInstance) => {
+		const state = TerminalFindContribution.get(activeInstance)?.findWidget.state;
+		state?.change({ matchCase: !state.matchCase }, false);
 	}
 });
 
@@ -163,8 +174,8 @@ registerActiveInstanceAction({
 		}
 	],
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		const widget = getFindWidget(activeInstance, accessor);
+	run: (activeInstance) => {
+		const widget = TerminalFindContribution.get(activeInstance)?.findWidget;
 		if (widget) {
 			widget.show();
 			widget.find(false);
@@ -189,8 +200,8 @@ registerActiveInstanceAction({
 		}
 	],
 	precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
-	run: (activeInstance, c, accessor) => {
-		const widget = getFindWidget(activeInstance, accessor);
+	run: (activeInstance) => {
+		const widget = TerminalFindContribution.get(activeInstance)?.findWidget;
 		if (widget) {
 			widget.show();
 			widget.find(true);