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

Plot editor tab sizing policies #6043

Open
wants to merge 10 commits 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 @@ -78,7 +78,7 @@ export const DynamicPlotInstance = (props: DynamicPlotInstanceProps) => {

// Wait for the plot to render.
const result =
await props.plotClient.render(plotSize, ratio);
await props.plotClient.renderWithSizingPolicy(plotSize, ratio);

// Update the URI to the URI of the new plot.
setUri(result.uri);
Expand All @@ -94,15 +94,15 @@ export const DynamicPlotInstance = (props: DynamicPlotInstanceProps) => {
};

// Render using the current sizing policy.
render(plotsContext.positronPlotsService.selectedSizingPolicy);
render(props.plotClient.sizingPolicy);

// When the plot is rendered, update the URI.
disposables.add(props.plotClient.onDidCompleteRender((result) => {
setUri(result.uri);
}));

// Re-render if the sizing policy changes.
disposables.add(plotsContext.positronPlotsService.onDidChangeSizingPolicy((policy) => {
disposables.add(props.plotClient.onDidChangeSizingPolicy((policy) => {
render(policy);
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export const OpenInEditorMenuButton = (props: OpenInEditorMenuButtonProps) => {
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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const SizingPolicyMenuButton = (props: SizingPolicyMenuButtonProps) => {

// State.
const [activePolicyLabel, setActivePolicyLabel] =
React.useState(props.plotsService.selectedSizingPolicy.getName(props.plotClient));
React.useState(props.plotClient.sizingPolicy.getName(props.plotClient));

React.useEffect(() => {
const disposables = new DisposableStore();
Expand Down Expand Up @@ -76,7 +76,7 @@ export const SizingPolicyMenuButton = (props: SizingPolicyMenuButtonProps) => {
let policyDisposables = attachPolicy(props.plotsService.selectedSizingPolicy);

// Update the active policy label when the selected policy changes.
disposables.add(props.plotsService.onDidChangeSizingPolicy(policy => {
disposables.add(props.plotClient.onDidChangeSizingPolicy(policy => {
policyDisposables.dispose();
policyDisposables = attachPolicy(policy);
}));
Expand Down Expand Up @@ -106,7 +106,7 @@ export const SizingPolicyMenuButton = (props: SizingPolicyMenuButtonProps) => {
enabled,
checked: policy.id === selectedPolicy.id,
run: () => {
props.plotsService.selectSizingPolicy(policy.id);
props.plotClient.sizingPolicy = policy;
}
});
}
Expand All @@ -125,7 +125,7 @@ export const SizingPolicyMenuButton = (props: SizingPolicyMenuButtonProps) => {
enabled: true,
checked: customPolicy.id === selectedPolicy.id,
run: () => {
props.plotsService.selectSizingPolicy(customPolicy.id);
props.plotClient.sizingPolicy = customPolicy;
}
});
}
Expand Down Expand Up @@ -162,6 +162,7 @@ export const SizingPolicyMenuButton = (props: SizingPolicyMenuButtonProps) => {
} else {
// The user entered a valid size; set the custom policy.
props.plotsService.setCustomPlotSize(result.size);
props.plotClient.sizingPolicy = new PlotSizingPolicyCustom(result.size);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
}
size = { height: height.value, width: width.value };
}
return props.plotClient.render(size, dpi.value / BASE_DPI, format, true);
return props.plotClient.renderWithSizingPolicy(size, dpi.value / BASE_DPI, format, true);
};

const previewButton = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { IPositronPlotsService, POSITRON_PLOTS_VIEW_ID } from '../../../services
import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions, IWorkbenchContribution } from '../../../common/contributions.js';
import { Extensions as ViewContainerExtensions, IViewsRegistry } from '../../../common/views.js';
import { registerAction2 } from '../../../../platform/actions/common/actions.js';
import { PlotsActiveEditorCopyAction, PlotsActiveEditorSaveAction, PlotsClearAction, PlotsCopyAction, PlotsEditorAction, PlotsNextAction, PlotsPopoutAction, PlotsPreviousAction, PlotsRefreshAction, PlotsSaveAction } from './positronPlotsActions.js';
import { PlotsActiveEditorCopyAction, PlotsActiveEditorSaveAction, PlotsClearAction, PlotsCopyAction, PlotsEditorAction, PlotsNextAction, PlotsPopoutAction, PlotsPreviousAction, PlotsRefreshAction, PlotsSaveAction, PlotsSizingPolicyAction } from './positronPlotsActions.js';
import { POSITRON_SESSION_CONTAINER } from '../../positronSession/browser/positronSessionContainer.js';

// Register the Positron plots service.
Expand Down Expand Up @@ -72,6 +72,7 @@ class PositronPlotsContribution extends Disposable implements IWorkbenchContribu
registerAction2(PlotsEditorAction);
registerAction2(PlotsActiveEditorCopyAction);
registerAction2(PlotsActiveEditorSaveAction);
registerAction2(PlotsSizingPolicyAction);
}
}

Expand Down
163 changes: 148 additions & 15 deletions src/vs/workbench/contrib/positronPlots/browser/positronPlotsActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@ import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextke
import { IsDevelopmentContext } from '../../../../platform/contextkey/common/contextkeys.js';
import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js';
import { INotificationService } from '../../../../platform/notification/common/notification.js';
import { IQuickInputService, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js';
import { IQuickInputService, IQuickPick, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js';
import { PLOT_IS_ACTIVE_EDITOR, POSITRON_EDITOR_PLOTS } from '../../positronPlotsEditor/browser/positronPlotsEditor.contribution.js';
import { PositronPlotsEditorInput } from '../../positronPlotsEditor/browser/positronPlotsEditorInput.js';
import { IEditorService } from '../../../services/editor/common/editorService.js';
import { IPositronPlotsService } from '../../../services/positronPlots/common/positronPlots.js';
import { IPositronPlotClient, IPositronPlotsService } from '../../../services/positronPlots/common/positronPlots.js';
import { PlotClientInstance } from '../../../services/languageRuntime/common/languageRuntimePlotClient.js';
import { ThemeIcon } from '../../../../base/common/themables.js';

export enum PlotActionTarget {
VIEW = 'view',
ACTIVE_EDITOR = 'activeEditor',
}

export const POSITRON_PLOTS_ACTION_CATEGORY = nls.localize('positronPlotsCategory', "Plots");
const category: ILocalizedString = { value: POSITRON_PLOTS_ACTION_CATEGORY, original: 'Plots' };

Expand All @@ -34,6 +35,8 @@ const category: ILocalizedString = { value: POSITRON_PLOTS_ACTION_CATEGORY, orig
* has a plot.
*/
abstract class AbstractPlotsAction extends Action2 {
quickPickService?: IQuickInputService;

constructor(descriptor: IAction2Options) {
super(descriptor);
}
Expand Down Expand Up @@ -64,8 +67,8 @@ abstract class AbstractPlotsAction extends Action2 {
const plotsService = accessor.get(IPositronPlotsService);
const notificationService = accessor.get(INotificationService);
const editorService = accessor.get(IEditorService);
const quickPick = accessor.get(IQuickInputService);
const configurationService = accessor.get(IConfigurationService);
this.quickPickService = accessor.get(IQuickInputService);

const editorPlotsEnabled = Boolean(configurationService.getValue(POSITRON_EDITOR_PLOTS));

Expand All @@ -84,7 +87,7 @@ abstract class AbstractPlotsAction extends Action2 {
if (target) {
this.executeTargetAction(target, plotsService, editorService, notificationService);
} else {
const quickPicker = quickPick.createQuickPick();
const quickPicker = this.quickPickService.createQuickPick();

quickPicker.items = quickPickItems;
quickPicker.ignoreFocusOut = true;
Expand All @@ -110,6 +113,15 @@ abstract class AbstractPlotsAction extends Action2 {
}
}

/**
* A filter to determine if the plot should be included in the action.
*
* @returns true if the plot should be included
*/
protected plotActionFilter(_plotClient: IPositronPlotClient): boolean {
return true;
}

/**
* Gets the active editor plot id.
*
Expand All @@ -134,25 +146,29 @@ abstract class AbstractPlotsAction extends Action2 {
const items: IQuickPickItem[] = [];

if (plotsService.selectedPlotId) {
items.push(
{
const plotClient = plotsService.positronPlotInstances.find(p => p.id === plotsService.selectedPlotId);
if (plotClient && this.plotActionFilter(plotClient)) {
items.push({
id: PlotActionTarget.VIEW,
label: localize('positronPlots.copyPlotsView', 'From Plots View'),
ariaLabel: localize('positronPlots.copyPlotsView', 'From Plots View'),
}
);
});
}
}

editorService.editors.forEach(input => {
if (input.editorId === PositronPlotsEditorInput.EditorID) {
const name = input.getName();
const plotId = input.resource?.path.toString();
if (plotId) {
items.push({
id: plotId,
label: localize('positronPlots.copyEditor', 'Editor: {0}', name),
ariaLabel: localize('positronPlots.copyEditor', 'Editor: {0}', name),
});
const plotClient = plotsService.getEditorInstance(plotId);
if (plotClient && this.plotActionFilter(plotClient)) {
items.push({
id: plotId,
label: localize('positronPlots.copyEditor', 'Editor: {0}', name),
ariaLabel: localize('positronPlots.copyEditor', 'Editor: {0}', name),
});
}
}
}
});
Expand Down Expand Up @@ -418,7 +434,7 @@ export class PlotsEditorAction extends Action2 {
async run(accessor: ServicesAccessor, groupType?: number) {
const plotsService = accessor.get(IPositronPlotsService);
if (plotsService.selectedPlotId) {
plotsService.openEditor(groupType);
plotsService.openEditor(plotsService.selectedPlotId, groupType);
} else {
accessor.get(INotificationService).info(localize('positronPlots.noPlotSelected', 'No plot selected.'));
}
Expand Down Expand Up @@ -498,3 +514,120 @@ export class PlotsActiveEditorSaveAction extends Action2 {
commandService.executeCommand(PlotsSaveAction.ID, PlotActionTarget.ACTIVE_EDITOR);
}
}

/** Action to change the plot's sizing policy */
export class PlotsSizingPolicyAction extends AbstractPlotsAction {
static ID = 'workbench.action.positronPlots.sizingPolicy';
sizingPicker?: IQuickPick<IQuickPickItem, any>;

constructor() {
super({
id: PlotsSizingPolicyAction.ID,
title: localize2('positronPlots.sizingPolicy', 'Change Plot Sizing Policy'),
category,
f1: true,
});
}

override run(accessor: ServicesAccessor, target?: PlotActionTarget): Promise<void> {
return super.run(accessor, target);
}

override executeQuickPick(quickPick: IQuickPickItem, plotsService: IPositronPlotsService, editorService: IEditorService, notificationService: INotificationService): void {
const plotId = quickPick.id;

if (!plotId) {
notificationService.info(localize('positronPlots.noPlotSelected', 'No plot selected.'));
return;
}

const isView = plotId === PlotActionTarget.VIEW;

if (isView) {
this.executeTargetAction(PlotActionTarget.VIEW, plotsService, editorService, notificationService);
} else {
const plotClient = plotsService.getEditorInstance(plotId);
if (plotClient instanceof PlotClientInstance) {
if (!this.quickPickService) {
return;
}

this.getSizingPolicy(plotsService, editorService, () => {
if (!this.sizingPicker) {
return;
}

const selectedItem = this.sizingPicker.selectedItems[0];
if (selectedItem?.id) {
plotsService.setEditorSizingPolicy(plotId, selectedItem.id);
}
});
}
}
}

override executeTargetAction(target: PlotActionTarget, plotsService: IPositronPlotsService, editorService: IEditorService, notificationService: INotificationService): void {
this.getSizingPolicy(plotsService, editorService, () => {
if (!this.sizingPicker) {
return;
}

const selectedItem = this.sizingPicker.selectedItems[0];
if (selectedItem?.id) {
if (target === PlotActionTarget.VIEW) {
plotsService.selectSizingPolicy(selectedItem.id);
} else {
const plotId = this.getActiveEditorPlotId(editorService);
if (plotId) {
plotsService.setEditorSizingPolicy(plotId, selectedItem.id);
}
}
}
});
}

private getSizingPolicy(plotsService: IPositronPlotsService, editorService: IEditorService, onAccept: () => void): void {
if (!this.quickPickService) {
return;
}

const sizingItems = this.createSizingItems(plotsService);
this.sizingPicker = this.quickPickService.createQuickPick();

this.sizingPicker.items = sizingItems;
this.sizingPicker.title = localize('positronPlots.action.selectSizingPolicy', 'Select a sizing policy');

this.sizingPicker.show();

this.sizingPicker.onDidAccept(() => {
onAccept();
this.sizingPicker?.hide();
});
}

private createSizingItems(plotsService: IPositronPlotsService, plotId?: string): IQuickPickItem[] {
const items: IQuickPickItem[] = [];
const plotClient = plotId ? plotsService.getEditorInstance(plotId)
: plotsService.positronPlotInstances.find(p => p.id === plotsService.selectedPlotId) as PlotClientInstance;

if (!plotClient || !(plotClient instanceof PlotClientInstance)) {
throw new Error('Plot not found');
}

plotsService.sizingPolicies.forEach(policy => {
items.push({
id: policy.id,
label: policy.getName(plotClient),
ariaLabel: policy.getName(plotClient),
iconClass: plotClient.sizingPolicy.id === policy.id ? ThemeIcon.asClassName(Codicon.positronCheckMark)
: ThemeIcon.asClassName(Codicon.blank),
});
});

return items;
}

override plotActionFilter(plotClient: IPositronPlotClient): boolean {
return plotClient instanceof PlotClientInstance;
}
}
Loading
Loading