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

focused element and selected elements as arguments for title commands #158916

Merged
merged 3 commits into from Aug 24, 2022

Conversation

benibenj
Copy link
Contributor

@benibenj benibenj commented Aug 23, 2022

Creates the arguments (focusedItem: T | undefined, selectedItems: T[] | undefined) for title commands. As we want to create 2 arguments from the result of getActionContext() we have to create an argument which can be unrolled into multiple arguments later.

@benibenj benibenj self-assigned this Aug 23, 2022
@VSCodeTriageBot VSCodeTriageBot added this to the August 2022 milestone Aug 23, 2022
@benibenj benibenj requested a review from alexr00 August 23, 2022 14:42
@alexr00
Copy link
Member

alexr00 commented Aug 24, 2022

We can use the MultipleSelectionActionRunner to do some additional context processing to avoid adding the unrollable type.

diff --git a/src/vs/workbench/api/common/extHostTreeViews.ts b/src/vs/workbench/api/common/extHostTreeViews.ts
index bfad1a2fc67..b5b3f8350be 100644
--- a/src/vs/workbench/api/common/extHostTreeViews.ts
+++ b/src/vs/workbench/api/common/extHostTreeViews.ts
@@ -248,9 +248,6 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
 		if (treeView && '$treeItemHandle' in arg) {
 			return treeView.getExtensionElement(arg.$treeItemHandle);
 		}
-		if (treeView && '$selectedTreeItems' in arg && arg.$selectedTreeItems) {
-			return treeView.selectedElements.length ? treeView.selectedElements : undefined;
-		}
 		if (treeView && '$focusedTreeItem' in arg && arg.$focusedTreeItem) {
 			return treeView.focusedElement;
 		}
diff --git a/src/vs/workbench/browser/parts/views/treeView.ts b/src/vs/workbench/browser/parts/views/treeView.ts
index de0437c5feb..94eaa436215 100644
--- a/src/vs/workbench/browser/parts/views/treeView.ts
+++ b/src/vs/workbench/browser/parts/views/treeView.ts
@@ -73,6 +73,7 @@ export class TreeViewPane extends ViewPane {
 
 	protected readonly treeView: ITreeView;
 	private _container: HTMLElement | undefined;
+	private _actionRunner: MultipleSelectionActionRunner;
 
 	constructor(
 		options: IViewletViewOptions,
@@ -85,6 +86,7 @@ export class TreeViewPane extends ViewPane {
 		@IOpenerService openerService: IOpenerService,
 		@IThemeService themeService: IThemeService,
 		@ITelemetryService telemetryService: ITelemetryService,
+		@INotificationService notificationService: INotificationService
 	) {
 		super({ ...(options as IViewPaneOptions), titleMenuId: MenuId.ViewTitle, donotForwardArgs: false }, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, telemetryService);
 		const { treeView } = (<ITreeViewDescriptor>Registry.as<IViewsRegistry>(Extensions.ViewsRegistry).getView(options.id));
@@ -105,6 +107,7 @@ export class TreeViewPane extends ViewPane {
 		if (options.titleDescription !== this.treeView.description) {
 			this.updateTitleDescription(this.treeView.description);
 		}
+		this._actionRunner = new MultipleSelectionActionRunner(notificationService, () => this.treeView.getSelection());
 
 		this.updateTreeVisibility();
 	}
@@ -145,13 +148,12 @@ export class TreeViewPane extends ViewPane {
 		this.treeView.setVisibility(this.isBodyVisible());
 	}
 
-	override getActionsContext(): UnrollableArgument {
-		return {
-			unrollArguments: [
-				<TreeViewPaneHandleArg>{ $treeViewId: this.id, $focusedTreeItem: true },
-				<TreeViewPaneHandleArg>{ $treeViewId: this.id, $selectedTreeItems: true }
-			]
-		};
+	override getActionRunner() {
+		return this._actionRunner;
+	}
+
+	override getActionsContext(): TreeViewPaneHandleArg {
+		return { $treeViewId: this.id, $focusedTreeItem: true, $selectedTreeItems: true };
 	}
 
 }
@@ -815,6 +817,10 @@ abstract class AbstractTreeView extends Disposable implements ITreeView {
 		this.tree?.setSelection(items);
 	}
 
+	getSelection(): ITreeItem[] {
+		return this.tree?.getSelection() ?? [];
+	}
+
 	setFocus(item: ITreeItem): void {
 		if (this.tree) {
 			this.focus(true, item);
@@ -1236,7 +1242,7 @@ class Aligner extends Disposable {
 
 class MultipleSelectionActionRunner extends ActionRunner {
 
-	constructor(notificationService: INotificationService, private getSelectedResources: (() => ITreeItem[])) {
+	constructor(notificationService: INotificationService, private getSelectedResources: (() => ITreeItem[]),) {
 		super();
 		this._register(this.onDidRun(e => {
 			if (e.error && !isCancellationError(e.error)) {
@@ -1245,13 +1251,13 @@ class MultipleSelectionActionRunner extends ActionRunner {
 		}));
 	}
 
-	override async runAction(action: IAction, context: TreeViewItemHandleArg): Promise<void> {
+	override async runAction(action: IAction, context: TreeViewItemHandleArg | TreeViewPaneHandleArg): Promise<void> {
 		const selection = this.getSelectedResources();
 		let selectionHandleArgs: TreeViewItemHandleArg[] | undefined = undefined;
 		let actionInSelected: boolean = false;
 		if (selection.length > 1) {
 			selectionHandleArgs = selection.map(selected => {
-				if (selected.handle === context.$treeItemHandle) {
+				if ((selected.handle === (context as TreeViewItemHandleArg).$treeItemHandle) || (context as TreeViewPaneHandleArg).$selectedTreeItems) {
 					actionInSelected = true;
 				}
 				return { $treeViewId: context.$treeViewId, $treeItemHandle: selected.handle };
diff --git a/src/vs/workbench/browser/parts/views/viewPane.ts b/src/vs/workbench/browser/parts/views/viewPane.ts
index 22a0701865b..84ee0c009f3 100644
--- a/src/vs/workbench/browser/parts/views/viewPane.ts
+++ b/src/vs/workbench/browser/parts/views/viewPane.ts
@@ -11,7 +11,7 @@ import { attachButtonStyler, attachProgressBarStyler } from 'vs/platform/theme/c
 import { PANEL_BACKGROUND, SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme';
 import { after, append, $, trackFocus, EventType, addDisposableListener, createCSSRule, asCSSUrl } from 'vs/base/browser/dom';
 import { IDisposable, Disposable, DisposableStore } from 'vs/base/common/lifecycle';
-import { IAction } from 'vs/base/common/actions';
+import { IAction, IActionRunner } from 'vs/base/common/actions';
 import { ActionsOrientation, IActionViewItem, prepareActions } from 'vs/base/browser/ui/actionbar/actionbar';
 import { Registry } from 'vs/platform/registry/common/platform';
 import { ToolBar } from 'vs/base/browser/ui/toolbar/toolbar';
@@ -298,7 +298,8 @@ export abstract class ViewPane extends Pane implements IView {
 			actionViewItemProvider: action => this.getActionViewItem(action),
 			ariaLabel: nls.localize('viewToolbarAriaLabel', "{0} actions", this.title),
 			getKeyBinding: action => this.keybindingService.lookupKeybinding(action.id),
-			renderDropdownAsChildElement: true
+			renderDropdownAsChildElement: true,
+			actionRunner: this.getActionRunner()
 		});
 
 		this._register(this.toolbar);
@@ -519,6 +520,10 @@ export abstract class ViewPane extends Pane implements IView {
 		return undefined;
 	}
 
+	getActionRunner(): IActionRunner | undefined {
+		return undefined;
+	}
+
 	getOptimalWidth(): number {
 		return 0;
 	}
diff --git a/src/vs/workbench/common/views.ts b/src/vs/workbench/common/views.ts
index eddb7ed51e0..b84662e5a6d 100644
--- a/src/vs/workbench/common/views.ts
+++ b/src/vs/workbench/common/views.ts
@@ -693,6 +693,8 @@ export interface ITreeView extends IDisposable {
 
 	setSelection(items: ITreeItem[]): void;
 
+	getSelection(): ITreeItem[];
+
 	setFocus(item: ITreeItem): void;
 
 	show(container: any): void;
diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSyncMergesView.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSyncMergesView.ts
index 01b5a3216a9..b5dcbed8bc7 100644
--- a/src/vs/workbench/contrib/userDataSync/browser/userDataSyncMergesView.ts
+++ b/src/vs/workbench/contrib/userDataSync/browser/userDataSyncMergesView.ts
@@ -36,7 +36,7 @@ import { IEditorContribution } from 'vs/editor/common/editorCommon';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { FloatingClickWidget } from 'vs/workbench/browser/codeeditor';
 import { registerEditorContribution } from 'vs/editor/browser/editorExtensions';
-import { Severity } from 'vs/platform/notification/common/notification';
+import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
 import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
 import { EditorResolution } from 'vs/platform/editor/common/editor';
 
@@ -66,8 +66,9 @@ export class UserDataSyncMergesViewPane extends TreeViewPane {
 		@IOpenerService openerService: IOpenerService,
 		@IThemeService themeService: IThemeService,
 		@ITelemetryService telemetryService: ITelemetryService,
+		@INotificationService notificationService: INotificationService
 	) {
-		super(options, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, telemetryService);
+		super(options, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, telemetryService, notificationService);
 		this.userDataSyncPreview = userDataSyncWorkbenchService.userDataSyncPreview;
 
 		this._register(this.userDataSyncPreview.onDidChangeResources(() => this.updateSyncButtonEnablement()));

Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

🚀

@benibenj benibenj merged commit 21ae452 into main Aug 24, 2022
@benibenj benibenj deleted the benibenj/treeViewTitleCommandContext branch August 24, 2022 14:42
@benibenj benibenj linked an issue Aug 24, 2022 that may be closed by this pull request
benibenj added a commit that referenced this pull request Aug 24, 2022
benibenj added a commit that referenced this pull request Aug 25, 2022
* Revert "focused element and selected elements as arguments for title commands (#158916)"

This reverts commit 21ae452.

* Revert "Merge pull request #157803 from microsoft/benibenj/treeItemContextFromCommand"

This reverts commit c3e6bd2, reversing
changes made to ec5da8a.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pass selected tree item context to view/title commands
3 participants