Fixes terminal not restoring properly

src/vs/workbench/contrib/terminal/browser/media/terminal.css

@@ -39,7 +39,7 @@
 
 .monaco-workbench .editor-instance .terminal-wrapper,
 .monaco-workbench .pane-body.integrated-terminal .terminal-wrapper {
-	display: none;
+	display: block;
 	height: 100%;
 	box-sizing: border-box;
 }
@@ -112,11 +112,6 @@
 .xterm.xterm-cursor-pointer .xterm-screen { cursor: pointer; }
 .xterm.column-select.focus .xterm-screen  { cursor: crosshair; }
 
-.monaco-workbench .editor-instance .terminal-wrapper.active,
-.monaco-workbench .pane-body.integrated-terminal .terminal-wrapper.active {
-	display: block;
-}
-
 .monaco-workbench .editor-instance .xterm {
 	padding-left: 20px !important;
 }

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

@@ -339,6 +339,7 @@ export interface ITerminalGroupService extends ITerminalInstanceHost, ITerminalF
 	hidePanel(): void;
 	focusTabs(): void;
 	showTabs(): void;
+	updateVisibility(): void;
 }
 
 /**
@@ -752,7 +753,7 @@ export interface ITerminalInstance {
 	 *
 	 * @param container The element to attach the terminal instance to.
 	 */
-	attachToElement(container: HTMLElement): Promise<void> | void;
+	attachToElement(container: HTMLElement): void;
 
 	/**
 	 * Detaches the terminal instance from the terminal editor DOM element.

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

@@ -31,6 +31,7 @@ import { INotificationService } from 'vs/platform/notification/common/notificati
 import { openContextMenu } from 'vs/workbench/contrib/terminal/browser/terminalContextMenu';
 import { ICommandService } from 'vs/platform/commands/common/commands';
 import { ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService';
+import { IWorkbenchLayoutService, Parts } from 'vs/workbench/services/layout/browser/layoutService';
 
 const findWidgetSelector = '.simple-find-part-wrapper';
 
@@ -68,7 +69,8 @@ export class TerminalEditor extends EditorPane {
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@IContextMenuService private readonly _contextMenuService: IContextMenuService,
 		@INotificationService private readonly _notificationService: INotificationService,
-		@ITerminalProfileService private readonly _terminalProfileService: ITerminalProfileService
+		@ITerminalProfileService private readonly _terminalProfileService: ITerminalProfileService,
+		@IWorkbenchLayoutService private readonly _workbenchLayoutService: IWorkbenchLayoutService
 	) {
 		super(terminalEditorId, telemetryService, themeService, storageService);
 		this._findState = new FindReplaceState();
@@ -86,7 +88,7 @@ export class TerminalEditor extends EditorPane {
 		if (this._lastDimension) {
 			this.layout(this._lastDimension);
 		}
-		this._editorInput.terminalInstance?.setVisible(this.isVisible());
+		this._editorInput.terminalInstance?.setVisible(this.isVisible() && this._workbenchLayoutService.isVisible(Parts.EDITOR_PART));
 		if (this._editorInput.terminalInstance) {
 			// since the editor does not monitor focus changes, for ex. between the terminal
 			// panel and the editors, this is needed so that the active instance gets set
@@ -208,7 +210,7 @@ export class TerminalEditor extends EditorPane {
 
 	override setVisible(visible: boolean, group?: IEditorGroup): void {
 		super.setVisible(visible, group);
-		return this._editorInput?.terminalInstance?.setVisible(visible);
+		this._editorInput?.terminalInstance?.setVisible(visible && this._workbenchLayoutService.isVisible(Parts.EDITOR_PART));
 	}
 
 	override getActionViewItem(action: IAction): IActionViewItem | undefined {

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

@@ -253,7 +253,6 @@ export class TerminalGroup extends Disposable implements ITerminalGroup {
 	private _instanceDisposables: Map<number, IDisposable[]> = new Map();
 
 	private _activeInstanceIndex: number = -1;
-	private _isVisible: boolean = false;
 
 	get terminalInstances(): ITerminalInstance[] { return this._terminalInstances; }
 
@@ -315,8 +314,6 @@ export class TerminalGroup extends Disposable implements ITerminalGroup {
 			this._splitPaneContainer!.split(instance, parentIndex + 1);
 		}
 
-		instance.setVisible(this._isVisible);
-
 		this._onInstancesChanged.fire();
 	}
 
@@ -481,7 +478,6 @@ export class TerminalGroup extends Disposable implements ITerminalGroup {
 				this._initialRelativeSizes = undefined;
 			}
 		}
-		this.setVisible(this._isVisible);
 	}
 
 	get title(): string {
@@ -514,7 +510,6 @@ export class TerminalGroup extends Disposable implements ITerminalGroup {
 	}
 
 	setVisible(visible: boolean): void {
-		this._isVisible = visible;
 		if (this._groupElement) {
 			this._groupElement.style.display = visible ? '' : 'none';
 		}

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

@@ -5,7 +5,7 @@
 
 import { Orientation } from 'vs/base/browser/ui/sash/sash';
 import { timeout } from 'vs/base/common/async';
-import { Emitter } from 'vs/base/common/event';
+import { Emitter, Event } from 'vs/base/common/event';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { URI } from 'vs/base/common/uri';
 import { FindReplaceState } from 'vs/editor/contrib/find/browser/findState';
@@ -75,6 +75,8 @@ export class TerminalGroupService extends Disposable implements ITerminalGroupSe
 		this.onDidChangeGroups(() => this._terminalGroupCountContextKey.set(this.groups.length));
 
 		this._findState = new FindReplaceState();
+
+		Event.any(this.onDidChangeActiveGroup, this.onDidChangeInstances)(() => this.updateVisibility());
 	}
 
 	hidePanel(): void {
@@ -280,7 +282,6 @@ export class TerminalGroupService extends Disposable implements ITerminalGroupSe
 		const oldActiveGroup = this.activeGroup;
 		this.activeGroupIndex = index;
 		if (force || oldActiveGroup !== this.activeGroup) {
-			this.groups.forEach((g, i) => g.setVisible(i === this.activeGroupIndex));
 			this._onDidChangeActiveGroup.fire(this.activeGroup);
 			this._onDidChangeActiveInstance.fire(this.activeInstance);
 		}
@@ -318,8 +319,6 @@ export class TerminalGroupService extends Disposable implements ITerminalGroupSe
 		this.activeGroupIndex = instanceLocation.groupIndex;
 		this._onDidChangeActiveGroup.fire(this.activeGroup);
 		instanceLocation.group.setActiveInstanceByIndex(activeInstanceIndex, true);
-		this.groups.forEach((g, i) => g.setVisible(i === instanceLocation.groupIndex));
-
 	}
 
 	setActiveGroupToNext() {
@@ -485,6 +484,17 @@ export class TerminalGroupService extends Disposable implements ITerminalGroupSe
 			return `${index + 1}: ${group.title ? group.title : ''}`;
 		});
 	}
+
+	/**
+	 * Visibility should be updated in the following cases:
+	 * 1. Toggle `TERMINAL_VIEW_ID` visibility
+	 * 2. Change active group
+	 * 3. Change instances in active group
+	 */
+	updateVisibility() {
+		const visible = this._viewsService.isViewVisible(TERMINAL_VIEW_ID);
+		this.groups.forEach((g, i) => g.setVisible(visible && i === this.activeGroupIndex));
+	}
 }
 
 interface IInstanceLocation {

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

@@ -561,8 +561,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		// The terminal panel needs to have been created to get the real view dimensions
 		if (!this._container) {
 			// Set the fallback dimensions if not
-			this._cols = 80;
-			this._rows = 30;
+			this._cols = Constants.DefaultCols;
+			this._rows = Constants.DefaultRows;
 			return;
 		}
 
@@ -745,6 +745,11 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		this._pathService.userHome().then(userHome => {
 			this._userHome = userHome.fsPath;
 		});
+
+		if (this._isVisible) {
+			this._open();
+		}
+
 		return xterm;
 	}
 
@@ -963,40 +968,44 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		this._container = undefined;
 	}
 
-	attachToElement(container: HTMLElement): Promise<void> | void {
+	attachToElement(container: HTMLElement): void {
 		// The container did not change, do nothing
 		if (this._container === container) {
 			return;
 		}
 
 		this._attachBarrier.open();
 
-		// Attach has not occurred yet
-		if (!this._wrapperElement) {
-			return this._attachToElement(container);
-		}
-		this.xterm?.attachToElement(this._wrapperElement);
-
 		// The container changed, reattach
 		this._container = container;
-		this._container.appendChild(this._wrapperElement);
+		if (this._wrapperElement) {
+			this._container.appendChild(this._wrapperElement);
+		}
 		setTimeout(() => this._initDragAndDrop(container));
 	}
 
-	private async _attachToElement(container: HTMLElement): Promise<void> {
-		if (this._wrapperElement) {
-			throw new Error('The terminal instance has already been attached to a container');
+	/**
+	 * Opens the the terminal instance inside the parent DOM element previously set with
+	 * `attachToElement`, you must ensure the parent DOM element is explicitly visible before
+	 * invoking this function as it performs some DOM calculations internally
+	 */
+	private _open(): void {
+		if (this._wrapperElement || !this.xterm) {
+			return;
+		}
+
+		if (!this._container || !this._container.isConnected) {
+			throw new Error('A container element needs to be set with `attachToElement` and be part of the DOM before calling `_open`');
 		}
 
-		this._container = container;
 		this._wrapperElement = document.createElement('div');
 		this._wrapperElement.classList.add('terminal-wrapper');
 		const xtermElement = document.createElement('div');
 		this._wrapperElement.appendChild(xtermElement);
 
 		this._container.appendChild(this._wrapperElement);
 
-		const xterm = await this._xtermReadyPromise;
+		const xterm = this.xterm;
 
 		// Attach the xterm object to the DOM, exposing it to the smoke tests
 		this._wrapperElement.xterm = xterm.raw;
@@ -1115,21 +1124,16 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		this._register(dom.addDisposableListener(xterm.raw.textarea, 'blur', () => this._setFocus(false)));
 		this._register(dom.addDisposableListener(xterm.raw.textarea, 'focusout', () => this._setFocus(false)));
 
-		this._initDragAndDrop(container);
+		this._initDragAndDrop(this._container);
 
 		this._widgetManager.attachToElement(screenElement);
 		this._processManager.onProcessReady((e) => {
 			this._linkManager?.setWidgetManager(this._widgetManager);
 		});
 
-		// const computedStyle = window.getComputedStyle(this._container);
-		// const computedStyle = window.getComputedStyle(this._container.parentElement!);
-		// const width = parseInt(computedStyle.getPropertyValue('width').replace('px', ''), 10);
-		// const height = parseInt(computedStyle.getPropertyValue('height').replace('px', ''), 10);
 		if (this._lastLayoutDimensions) {
 			this.layout(this._lastLayoutDimensions);
 		}
-		this.setVisible(this._isVisible);
 		this.updateConfig();
 
 		// If IShellLaunchConfig.waitOnExit was true and the process finished before the terminal
@@ -1390,10 +1394,9 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 
 	setVisible(visible: boolean): void {
 		this._isVisible = visible;
-		if (this._wrapperElement) {
-			this._wrapperElement.classList.toggle('active', visible);
-		}
+		this._wrapperElement?.classList.toggle('active', visible);
 		if (visible && this.xterm) {
+			this._open();
 			// Resize to re-evaluate dimensions, this will ensure when switching to a terminal it is
 			// using the most up to date dimensions (eg. when terminal is created in the background
 			// using cached dimensions of a split terminal).

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

@@ -764,8 +764,6 @@ export class TerminalService implements ITerminalService {
 		group.addInstance(source);
 		this.setActiveInstance(source);
 		await this._terminalGroupService.showPanel(true);
-		// TODO: Shouldn't this happen automatically?
-		source.setVisible(true);
 
 		if (target && side) {
 			const index = group.terminalInstances.indexOf(target) + (side === 'after' ? 1 : 0);
@@ -775,7 +773,6 @@ export class TerminalService implements ITerminalService {
 		// Fire events
 		this._onDidChangeInstances.fire();
 		this._onDidChangeActiveGroup.fire(this._terminalGroupService.activeGroup);
-		this._terminalGroupService.showPanel(true);
 		this._onDidRequestHideFindWidget.fire();
 	}
 
@@ -1033,7 +1030,6 @@ export class TerminalService implements ITerminalService {
 			}
 			shellLaunchConfig.parentTerminalId = parent.instanceId;
 			instance = group.split(shellLaunchConfig);
-			this._terminalGroupService.groups.forEach((g, i) => g.setVisible(i === this._terminalGroupService.activeGroupIndex));
 		}
 		return instance;
 	}

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

@@ -171,15 +171,12 @@ export class TerminalViewPane extends ViewPane {
 				// defer focusing the panel to the focus() call
 				// to prevent overriding preserveFocus for extensions
 				this._terminalGroupService.showPanel(false);
-				if (hadTerminals) {
-					this._terminalGroupService.activeGroup?.setVisible(visible);
-				}
 			} else {
 				for (const instance of this._terminalGroupService.instances) {
 					instance.resetFocusContextKey();
 				}
-				this._terminalGroupService.activeGroup?.setVisible(false);
 			}
+			this._terminalGroupService.updateVisibility();
 		}));
 		this.layoutBody(this._parentDomElement.offsetHeight, this._parentDomElement.offsetWidth);
 	}

src/vs/workbench/test/browser/workbenchTestServices.ts

@@ -1832,6 +1832,7 @@ export class TestTerminalGroupService implements ITerminalGroupService {
 	getFindState(): FindReplaceState { throw new Error('Method not implemented.'); }
 	findNext(): void { throw new Error('Method not implemented.'); }
 	findPrevious(): void { throw new Error('Method not implemented.'); }
+	updateVisibility(): void { throw new Error('Method not implemented.'); }
 }
 
 export class TestTerminalProfileService implements ITerminalProfileService {