Disable view actions while a viewer is already loading (#8268)

This PR disables view actions in the variables pane when a view request
is already in-progress to avoid unintended duplicate requests.

### Release Notes

#### New Features

- N/A

#### Bug Fixes

- "View" actions are disabled in the variables pane while one is already
in-progress (#4547).

### QA Notes

Run `import time; time.sleep(10)` in a Python console and check:

1. If there is no viewer open for the variable, the following should be
disabled after the first click:
  1. The viewer button (e.g. table icon)
  2. Right click -> "View"
  3. Double-click to view
2. If there is a viewer open for the variable, any of the above view
actions should focus the viewer pane.
3. You should be able to queue view actions for multiple variables.

@:data-explorer @:variables

Author: seeM

src/vs/workbench/contrib/positronVariables/browser/components/variableItem.css

@@ -123,7 +123,12 @@
 	background-color: transparent;
 }
 
-.variable-item .details-column .right-column .viewer-icon:hover {
+.variable-item .details-column .right-column .viewer-icon.enabled:hover {
 	border: 1px solid var(--vscode-button-foreground, #ffffff);
-	background-color: var(--vscode-button-background, #0e639c);
+	background-color: var(--vscode-button-background, #b7c7d1);
+}
+
+.variable-item .details-column .right-column .viewer-icon.disabled {
+	cursor: auto;
+	opacity: 0.6;
 }

src/vs/workbench/contrib/positronVariables/browser/components/variableItem.tsx

@@ -69,6 +69,28 @@ export const formatSize = (size: number) => {
 	return localize('positron.sizeTB', "{0} TB", (size / TB).toFixed(2));
 };
 
+const viewQueuedLabel = (variableItem: IVariableItem) => {
+	switch (variableItem.kind) {
+		case 'table':
+			return localize('positron.variables.viewTableQueued', 'Data Table Queued...');
+		case 'connection':
+			return localize('positron.variables.viewConnectionQueued', 'Connection Queued...');
+		default:
+			return localize('positron.variables.viewQueued', 'View Queued...');
+	}
+};
+
+const viewLabel = (variableItem: IVariableItem) => {
+	switch (variableItem.kind) {
+		case 'table':
+			return localize('positron.variables.viewTable', 'View Data Table');
+		case 'connection':
+			return localize('positron.variables.viewConnection', 'View Connection');
+		default:
+			return localize('positron.variables.view', 'View');
+	}
+};
+
 /**
  * VariableItemProps interface.
  */
@@ -104,11 +126,17 @@ export const VariableItem = (props: VariableItemProps) => {
 	 * State hooks.
 	 */
 	const [isRecent, setIsRecent] = useState(props.variableItem.isRecent.get());
+	const [isViewLoading, setIsViewLoading] = useState(props.variableItem.isViewLoading.get());
 
 	useEffect(() => {
 		const disposableStore = new DisposableStore();
-		const evt = Event.fromObservable(props.variableItem.isRecent, disposableStore);
-		evt(e => setIsRecent(e));
+
+		const onDidChangeIsRecent = Event.fromObservable(props.variableItem.isRecent, disposableStore);
+		disposableStore.add(onDidChangeIsRecent(e => setIsRecent(e)));
+
+		const onDidChangeIsViewLoading = Event.fromObservable(props.variableItem.isViewLoading, disposableStore);
+		disposableStore.add(onDidChangeIsViewLoading(e => setIsViewLoading(e)));
+
 		return () => disposableStore.dispose();
 	}, [props.variableItem]);
 
@@ -274,10 +302,10 @@ export const VariableItem = (props: VariableItemProps) => {
 		if (!props.disabled && props.variableItem.hasViewer) {
 			actions.push({
 				id: POSITRON_VARIABLES_VIEW,
-				label: localize('positron.variables.view', "View"),
+				label: viewLabel(props.variableItem),
 				tooltip: '',
 				class: undefined,
-				enabled: true,
+				enabled: !isViewLoading,
 				run: () => viewVariableItem(props.variableItem)
 			});
 		}
@@ -391,19 +419,24 @@ export const VariableItem = (props: VariableItemProps) => {
 	const RightColumn = () => {
 		if (!props.disabled && props.variableItem.hasViewer) {
 			let icon = 'codicon codicon-open-preview';
-			if (props.variableItem.kind === 'table') {
+			if (isViewLoading) {
+				icon = 'codicon codicon-notebook-state-pending';
+			} else if (props.variableItem.kind === 'table') {
 				icon = 'codicon codicon-table';
 			} else if (props.variableItem.kind === 'connection') {
 				icon = 'codicon codicon-database';
 			}
-			icon = 'viewer-icon ' + icon + ' ' + props.variableItem.kind;
+			const enablement = isViewLoading ? 'disabled' : 'enabled';
+			icon = `viewer-icon ${enablement} ${icon} ${props.variableItem.kind}`;
 
 			return (
 				<div className='right-column'>
 					<div
 						className={icon}
-						title={localize('positron.variables.clickToView', "Click to view")}
-						onMouseDown={viewerMouseDownHandler}
+						title={isViewLoading ?
+							viewQueuedLabel(props.variableItem) :
+							viewLabel(props.variableItem)}
+						onMouseDown={isViewLoading ? undefined : viewerMouseDownHandler}
 					></div>
 				</div>
 			);
@@ -428,7 +461,7 @@ export const VariableItem = (props: VariableItemProps) => {
 
 	// Render.
 	return (
-		<div className={classNames} style={props.style} onDoubleClick={doubleClickHandler} onMouseDown={mouseDownHandler}>
+		<div className={classNames} style={props.style} onDoubleClick={isViewLoading ? undefined : doubleClickHandler} onMouseDown={mouseDownHandler}>
 			<div className='name-column' style={{ width: props.nameColumnWidth, minWidth: props.nameColumnWidth }}>
 				<div className='name-column-indenter' style={{ marginLeft: props.variableItem.indentLevel * 20 }}>
 					<div className='gutter'>

src/vs/workbench/services/languageRuntime/common/languageRuntimeVariablesClient.ts

@@ -5,7 +5,7 @@
 
 import { Emitter } from '../../../../base/common/event.js';
 import { Disposable } from '../../../../base/common/lifecycle.js';
-import { ISettableObservable } from '../../../../base/common/observableInternal/base.js';
+import { ISettableObservable } from '../../../../base/common/observable.js';
 import { IRuntimeClientInstance, RuntimeClientState, RuntimeClientStatus } from './languageRuntimeClientInstance.js';
 import { ClipboardFormatFormat, PositronVariablesComm, RefreshEvent, UpdateEvent, Variable } from './positronVariablesComm.js';
 
@@ -14,6 +14,11 @@ import { ClipboardFormatFormat, PositronVariablesComm, RefreshEvent, UpdateEvent
  * and methods.
  */
 export class PositronVariable {
+	/**
+	 * The current pending view request.
+	 */
+	private _pendingViewPromise?: Promise<string | undefined>;
+
 	/**
 	 * Creates a new PositronVariable instance.
 	 *
@@ -79,8 +84,18 @@ export class PositronVariable {
 	 * @returns The ID of the viewer that was opened, if any.
 	 */
 	async view(): Promise<string | undefined> {
+		if (this._pendingViewPromise) {
+			// If a view is already pending, return the existing promise.
+			return this._pendingViewPromise;
+		}
+
 		const path = this.parentKeys.concat(this.data.access_key);
-		return this._comm.view(path);
+		try {
+			this._pendingViewPromise = this._comm.view(path);
+			return await this._pendingViewPromise;
+		} finally {
+			this._pendingViewPromise = undefined;
+		}
 	}
 }
 
@@ -229,15 +244,6 @@ export class VariablesClientInstance extends Disposable {
 		return formatted.content;
 	}
 
-	/**
-	 * Requests that the variables client open a viewer for the specified variable.
-	 *
-	 * @param path The path to the variable to view
-	 */
-	public async requestView(path: string[]) {
-		await this._comm.view(path);
-	}
-
 	/**
 	 * Gets the underlying comm client.
 	 */

src/vs/workbench/services/positronVariables/common/classes/variableItem.ts

@@ -42,6 +42,11 @@ export class VariableItem implements IVariableItem {
 	 */
 	private readonly _isRecent: ISettableObservable<boolean>;
 
+	/**
+	 * Whether a viewer is currently loading for the variable item.
+	 */
+	private readonly _isViewLoading: ISettableObservable<boolean>;
+
 	//#endregion Private Properties
 
 	//#region Public Properties
@@ -181,6 +186,13 @@ export class VariableItem implements IVariableItem {
 		return this._isRecent;
 	}
 
+	/**
+	 * Whether a viewer is currently loading for the variable item.
+	 */
+	get isViewLoading() {
+		return this._isViewLoading;
+	}
+
 	/**
 	 * Get the raw data of the variable from the language runtime.
 	 */
@@ -200,6 +212,7 @@ export class VariableItem implements IVariableItem {
 	constructor(variable: PositronVariable, isRecent: boolean) {
 		this._variable = variable;
 		this._isRecent = observableValue(variable.data.access_key, isRecent);
+		this._isViewLoading = observableValue(this, false);
 
 		// Clear recent flag after 2 seconds.
 		setTimeout(() => this._isRecent.set(false, undefined), 2000);
@@ -325,7 +338,12 @@ export class VariableItem implements IVariableItem {
 	 * Requests that a viewer be opened for this variable.
 	 */
 	async view(): Promise<string | undefined> {
-		return this._variable.view();
+		this._isViewLoading.set(true, undefined);
+		try {
+			return await this._variable.view();
+		} finally {
+			this._isViewLoading.set(false, undefined);
+		}
 	}
 
 	//#endregion Public Methods

src/vs/workbench/services/positronVariables/common/interfaces/variableItem.ts

@@ -75,6 +75,11 @@ export interface IVariableItem {
 	 */
 	readonly isRecent: IObservable<boolean>;
 
+	/**
+	 * Whether a viewer is currently loading for the variable item.
+	 */
+	readonly isViewLoading: IObservable<boolean>;
+
 	/**
 	 * Formats the value of this variable item in a format suitable for placing on the clipboard.
 	 * @param mime The desired MIME type of the format, such as 'text/plain' or 'text/html'.