Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change open in editor tab to menu button #5733

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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');
Expand All @@ -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]);

Expand All @@ -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;
}
Expand Down Expand Up @@ -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()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -179,16 +180,11 @@ 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")}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may have lost the ariaLabels on the Open plot in [location] elements

onPressed={() => {
if (hasPlots) {
positronPlotsContext.positronPlotsService.openEditor();
}
}} />
<OpenInEditorMenuButton
tooltip={localize('positron-editor-plot-popout', "Open in editor tab")}
defaultGroup={positronPlotsContext.positronPlotsService.getPreferredEditorGroup()}
commandService={positronPlotsContext.commandService}
/>
: null
}
</ActionBarRegion>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*---------------------------------------------------------------------------------------------
* 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;
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 right')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could use similar verbiage to the File Explorer and hook into this preference: workbench.editor.openSideBySideDirection? Then the user can define whether the plot will open to the right or down based on the same preference as the File Explorer. Maybe something like "Open in editor tab to the side"?

Or maybe the user will want "to the Side" for plots to be different than the File Explorer?

Here's the context menu when right-clicking on a file in the File Explorer:
image

},
];

/**
* 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={localize('positron-editor-plot-popout', "Open in editor tab")}
actions={() => actions}
dropdownIndicator='enabled-split' />
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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.'));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`);
}
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion src/vs/workbench/services/positronPlots/common/positronPlots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading