Change open in editor tab to menu button (#5733)

src/vs/platform/positronActionBar/browser/components/actionBarMenuButton.tsx

@@ -7,7 +7,7 @@
 import './actionBarMenuButton.css';
 
 // React.
-import React, { useEffect, useRef } from 'react';
+import React, { useEffect, useRef, useState } from 'react';
 
 // Other dependencies.
 import { IAction } from '../../../../base/common/actions.js';
@@ -36,6 +36,11 @@ interface ActionBarMenuButtonProps {
 
 /**
  * ActionBarCommandButton component.
+ *
+ * Actions can be set as checked. If `enabled-split` is set then a default action is allowed to run
+ * when the button is clicked. The default action is the first action that is checked or the first
+ * action if none are checked.
+ *
  * @param props An ActionBarMenuButtonProps that contains the component properties.
  * @returns The rendered component.
  */
@@ -46,6 +51,10 @@ export const ActionBarMenuButton = (props: ActionBarMenuButtonProps) => {
 	// Reference hooks.
 	const buttonRef = useRef<HTMLButtonElement>(undefined!);
 
+	// State hooks.
+	const [actions, setActions] = useState<readonly IAction[]>([]);
+	const [defaultAction, setDefaultAction] = useState<IAction | undefined>(undefined);
+
 	// Manage the aria-haspopup and aria-expanded attributes.
 	useEffect(() => {
 		buttonRef.current.setAttribute('aria-haspopup', 'menu');
@@ -60,6 +69,20 @@ export const ActionBarMenuButton = (props: ActionBarMenuButtonProps) => {
 		}
 	}, [positronActionBarContext.menuShowing]);
 
+	const getMenuActions = React.useCallback(async () => {
+		const actions = await props.actions();
+		const defaultAction = actions.find(action => action.checked);
+
+		setDefaultAction(defaultAction);
+		setActions(actions);
+
+		return actions;
+	}, [props]);
+
+	useEffect(() => {
+		getMenuActions();
+	}, [getMenuActions]);
+
 	// Participate in roving tabindex.
 	useRegisterWithActionBar([buttonRef]);
 
@@ -69,7 +92,6 @@ export const ActionBarMenuButton = (props: ActionBarMenuButtonProps) => {
 	 */
 	const showMenu = async () => {
 		// Get the actions. If there are no actions, return.
-		const actions = await props.actions();
 		if (!actions.length) {
 			return;
 		}
@@ -111,11 +133,8 @@ export const ActionBarMenuButton = (props: ActionBarMenuButtonProps) => {
 				if (props.dropdownIndicator !== 'enabled-split') {
 					await showMenu();
 				} else {
-					// Get the actions and run the first action.
-					const actions = await props.actions();
-					if (actions.length) {
-						actions[0].run();
-					}
+					// Run the preferred action.
+					defaultAction ? defaultAction.run() : actions[0].run();
 				}
 			}}
 			onDropdownPressed={async () => await showMenu()}

src/vs/workbench/contrib/positronPlots/browser/components/actionBars.tsx

@@ -34,6 +34,7 @@ import { IHoverService } from '../../../../../platform/hover/browser/hover.js';
 import { HtmlPlotClient } from '../htmlPlotClient.js';
 import { POSITRON_EDITOR_PLOTS, positronPlotsEditorEnabled } from '../../../positronPlotsEditor/browser/positronPlotsEditor.contribution.js';
 import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js';
+import { OpenInEditorMenuButton } from './openInEditorMenuButton.js';
 
 // Constants.
 const kPaddingLeft = 14;
@@ -179,16 +180,12 @@ export const ActionBars = (props: PropsWithChildren<ActionBarsProps>) => {
 							: null
 						}
 						{enableEditorPlot ?
-							<ActionBarButton
-								iconId='go-to-file'
-								align='right'
-								tooltip={localize('positron-open-plot-editor', "Open plot in editor")}
-								ariaLabel={localize('positron-open-plot-editor', "Open plot in editor")}
-								onPressed={() => {
-									if (hasPlots) {
-										positronPlotsContext.positronPlotsService.openEditor();
-									}
-								}} />
+							<OpenInEditorMenuButton
+								tooltip={localize('positron-editor-plot-popout', "Open in editor tab")}
+								ariaLabel={localize('positron-editor-plot-popout', "Open in editor tab")}
+								defaultGroup={positronPlotsContext.positronPlotsService.getPreferredEditorGroup()}
+								commandService={positronPlotsContext.commandService}
+							/>
 							: null
 						}
 					</ActionBarRegion>

src/vs/workbench/contrib/positronPlots/browser/components/openInEditorMenuButton.tsx

@@ -0,0 +1,87 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (C) 2024 Posit Software, PBC. All rights reserved.
+ *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import React, { useState, useCallback, useEffect } from 'react';
+
+import { IAction } from '../../../../../base/common/actions.js';
+import { localize } from '../../../../../nls.js';
+import { ICommandService } from '../../../../../platform/commands/common/commands.js';
+import { ActionBarMenuButton } from '../../../../../platform/positronActionBar/browser/components/actionBarMenuButton.js';
+import { AUX_WINDOW_GROUP_TYPE, ACTIVE_GROUP_TYPE, SIDE_GROUP_TYPE, AUX_WINDOW_GROUP, ACTIVE_GROUP, SIDE_GROUP } from '../../../../services/editor/common/editorService.js';
+import { PlotsEditorAction } from '../positronPlotsActions.js';
+
+interface OpenInEditorMenuButtonProps {
+	tooltip: string;
+	ariaLabel: string;
+	defaultGroup: number;
+	commandService: ICommandService;
+}
+
+interface OpenInEditorCommand {
+	editorTarget: AUX_WINDOW_GROUP_TYPE | ACTIVE_GROUP_TYPE | SIDE_GROUP_TYPE;
+	label: string;
+}
+
+// create an array of action ids with labels
+const openInEditorCommands: Array<OpenInEditorCommand> = [
+	{
+		'editorTarget': AUX_WINDOW_GROUP,
+		'label': localize('positron-editor-new-window', 'Open in new window')
+	},
+	{
+		'editorTarget': ACTIVE_GROUP,
+		'label': localize('positron-editor-new-tab', 'Open in editor tab')
+	},
+	{
+		'editorTarget': SIDE_GROUP,
+		'label': localize('positron-editor-new-tab-right', 'Open in editor tab to the Side')
+	},
+];
+
+/**
+ * OpenInEditorMenuButton component.
+ *
+ * Creates a menu button that allows the user to open a plot in a new editor tab. Choosing a menu
+ * action will update the default action. The default action is preserved by the plots service when
+ * opening the editor tab succeeds.
+ *
+ * @param props An OpenInEditorMenuButtonProps that contains the component properties.
+ * @returns
+ */
+export const OpenInEditorMenuButton = (props: OpenInEditorMenuButtonProps) => {
+	const [defaultAction, setDefaultEditorAction] = useState<number>(props.defaultGroup);
+	const [actions, setActions] = useState<readonly IAction[]>([]);
+
+	const openEditorPlotHandler = useCallback((groupType: number) => {
+		// props.plotsService.openEditor(groupType);
+		props.commandService.executeCommand(PlotsEditorAction.ID, groupType);
+		setDefaultEditorAction(groupType);
+	}, [props.commandService]);
+
+	useEffect(() => {
+		const actions = openInEditorCommands.map((command) => {
+			return {
+				id: PlotsEditorAction.ID,
+				label: command.label,
+				tooltip: '',
+				class: undefined,
+				checked: defaultAction === command.editorTarget,
+				enabled: true,
+				run: () => openEditorPlotHandler(command.editorTarget)
+			};
+		});
+		setActions(actions);
+	}, [defaultAction]);
+
+
+	return (
+		<ActionBarMenuButton
+			iconId='go-to-file'
+			tooltip={props.tooltip}
+			ariaLabel={props.ariaLabel}
+			actions={() => actions}
+			dropdownIndicator='enabled-split' />
+	);
+};

src/vs/workbench/contrib/positronPlots/browser/positronPlotsActions.ts

@@ -352,10 +352,10 @@ export class PlotsEditorAction extends Action2 {
 	 *
 	 * @param accessor The service accessor.
 	 */
-	async run(accessor: ServicesAccessor) {
+	async run(accessor: ServicesAccessor, groupType?: number) {
 		const plotsService = accessor.get(IPositronPlotsService);
 		if (plotsService.selectedPlotId) {
-			plotsService.openEditor();
+			plotsService.openEditor(groupType);
 		} else {
 			accessor.get(INotificationService).info(localize('positronPlots.noPlotSelected', 'No plot selected.'));
 		}

src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts

@@ -44,12 +44,13 @@ import { PlotSizingPolicyIntrinsic } from '../../../services/positronPlots/commo
 import { ILogService } from '../../../../platform/log/common/log.js';
 import { INotificationService } from '../../../../platform/notification/common/notification.js';
 import { WebviewPlotClient } from './webviewPlotClient.js';
-import { IEditorService } from '../../../services/editor/common/editorService.js';
+import { ACTIVE_GROUP, IEditorService } from '../../../services/editor/common/editorService.js';
 import { URI } from '../../../../base/common/uri.js';
 import { PositronPlotCommProxy } from '../../../services/languageRuntime/common/positronPlotCommProxy.js';
 import { IPositronModalDialogsService } from '../../../services/positronModalDialogs/common/positronModalDialogs.js';
 import { ILabelService } from '../../../../platform/label/common/label.js';
 import { IPathService } from '../../../services/path/common/pathService.js';
+import { DynamicPlotInstance } from './components/dynamicPlotInstance.js';
 
 /** The maximum number of recent executions to store. */
 const MaxRecentExecutions = 10;
@@ -358,6 +359,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe
 		if (selectedPlot instanceof HtmlPlotClient) {
 			this._openerService.open(selectedPlot.uri,
 				{ openExternal: true, fromUserGesture: true });
+		} else if (selectedPlot instanceof DynamicPlotInstance) {
 		} else {
 			throw new Error(`Cannot open plot in new window: plot ${this._selectedPlotId} is not an HTML plot`);
 		}
@@ -1048,7 +1050,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe
 	}
 
 	/**
-	 * Registser a new plot client with the service, select it, and fire the
+	 * Register a new plot client with the service, select it, and fire the
 	 * appropriate events.
 	 *
 	 * @param client The plot client to register
@@ -1061,7 +1063,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe
 		this._showPlotsPane();
 	}
 
-	public async openEditor(): Promise<void> {
+	public async openEditor(groupType?: number): Promise<void> {
 		const plotClient = this._plots.find(plot => plot.id === this.selectedPlotId);
 
 		if (plotClient instanceof WebviewPlotClient) {
@@ -1088,16 +1090,25 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe
 			throw new Error('Cannot open plot in editor: plot not found');
 		}
 
+		const preferredEditorGroup = this._storageService.getNumber('positronPlots.defaultEditorAction', StorageScope.WORKSPACE, ACTIVE_GROUP);
+		const selectedEditorGroup = groupType ?? preferredEditorGroup;
 		const editorPane = await this._editorService.openEditor({
 			resource: URI.from({
 				scheme: Schemas.positronPlotsEditor,
 				path: plotId,
 			}),
-		});
+		}, selectedEditorGroup);
 
 		if (!editorPane) {
 			throw new Error('Failed to open editor');
 		}
+
+		this._storageService.store('positronPlots.defaultEditorAction', selectedEditorGroup, StorageScope.WORKSPACE, StorageTarget.MACHINE);
+	}
+
+	public getPreferredEditorGroup(): number {
+		const preferredEditorGroup = this._storageService.getNumber('positronPlots.defaultEditorAction', StorageScope.WORKSPACE, ACTIVE_GROUP);
+		return preferredEditorGroup;
 	}
 
 	public getEditorInstance(id: string) {

src/vs/workbench/services/positronPlots/common/positronPlots.ts

@@ -178,8 +178,16 @@ export interface IPositronPlotsService {
 
 	/**
 	 * Opens the currently selected plot in an editor.
+	 *
+	 * @param groupType Specify where the editor tab will be opened. Defaults to the preferred
+	 *   editor group.
+	 */
+	openEditor(groupType?: number): Promise<void>;
+
+	/**
+	 * Gets the preferred editor group for opening the plot in an editor tab.
 	 */
-	openEditor(): Promise<void>;
+	getPreferredEditorGroup(): number;
 
 	/**
 	 * Gets the plot client that is connected to an editor for the specified id.