Skip to content

Commit

Permalink
Remove Title Command Arguments from August Iteration (#159076)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@benibenj
benibenj authored Aug 25, 2022
1 parent 947ca71 commit ad7ce39
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 78 deletions.
1 change: 0 additions & 1 deletion src/vs/workbench/api/browser/mainThreadTreeViews.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie
this._register(treeView.onDidExpandItem(item => this._proxy.$setExpanded(treeViewId, item.handle, true)));
this._register(treeView.onDidCollapseItem(item => this._proxy.$setExpanded(treeViewId, item.handle, false)));
this._register(treeView.onDidChangeSelection(items => this._proxy.$setSelection(treeViewId, items.map(({ handle }) => handle))));
this._register(treeView.onDidChangeFocus(item => this._proxy.$setFocus(treeViewId, item.handle)));
this._register(treeView.onDidChangeVisibility(isVisible => this._proxy.$setVisible(treeViewId, isVisible)));
}

Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/api/common/extHost.protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,6 @@ export interface ExtHostTreeViewsShape {
$handleDrag(sourceViewId: string, sourceTreeItemHandles: string[], operationUuid: string, token: CancellationToken): Promise<DataTransferDTO | undefined>;
$setExpanded(treeViewId: string, treeItemHandle: string, expanded: boolean): void;
$setSelection(treeViewId: string, treeItemHandles: string[]): void;
$setFocus(treeViewId: string, treeItemHandle: string): void;
$setVisible(treeViewId: string, visible: boolean): void;
$hasResolve(treeViewId: string): Promise<boolean>;
$resolve(treeViewId: string, treeItemHandle: string, token: CancellationToken): Promise<ITreeItem | undefined>;
Expand Down
35 changes: 7 additions & 28 deletions src/vs/workbench/api/common/extHostTreeViews.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { URI } from 'vs/base/common/uri';
import { Emitter, Event } from 'vs/base/common/event';
import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
import { DataTransferDTO, ExtHostTreeViewsShape, MainThreadTreeViewsShape } from './extHost.protocol';
import { ITreeItem, TreeViewItemHandleArg, ITreeItemLabel, IRevealOptions, TreeCommand, TreeViewPaneHandleArg } from 'vs/workbench/common/views';
import { ITreeItem, TreeViewItemHandleArg, ITreeItemLabel, IRevealOptions, TreeCommand } from 'vs/workbench/common/views';
import { ExtHostCommands, CommandsConverter } from 'vs/workbench/api/common/extHostCommands';
import { asPromise } from 'vs/base/common/async';
import { TreeItemCollapsibleState, ThemeIcon, MarkdownString as MarkdownStringType, TreeItem } from 'vs/workbench/api/common/extHostTypes';
Expand Down Expand Up @@ -58,16 +58,16 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
private logService: ILogService
) {

function isTreeViewConvertableItem(arg: any): boolean {
return arg && arg.$treeViewId && (arg.$treeItemHandle || arg.$selectedTreeItems || arg.$focusedTreeItem);
function isTreeViewItemHandleArg(arg: any): boolean {
return arg && arg.$treeViewId && arg.$treeItemHandle;
}
commands.registerArgumentProcessor({
processArgument: arg => {
if (isTreeViewConvertableItem(arg)) {
if (isTreeViewItemHandleArg(arg)) {
return this.convertArgument(arg);
} else if (Array.isArray(arg) && (arg.length > 0)) {
return arg.map(item => {
if (isTreeViewConvertableItem(item)) {
if (isTreeViewItemHandleArg(item)) {
return this.convertArgument(item);
}
return item;
Expand Down Expand Up @@ -221,14 +221,6 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
treeView.setSelection(treeItemHandles);
}

$setFocus(treeViewId: string, treeItemHandles: string) {
const treeView = this.treeViews.get(treeViewId);
if (!treeView) {
throw new Error(localize('treeView.notRegistered', 'No tree view with id \'{0}\' registered.', treeViewId));
}
treeView.setFocus(treeItemHandles);
}

$setVisible(treeViewId: string, isVisible: boolean): void {
const treeView = this.treeViews.get(treeViewId);
if (!treeView) {
Expand All @@ -243,15 +235,9 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
return treeView;
}

private convertArgument(arg: TreeViewItemHandleArg | TreeViewPaneHandleArg): any {
private convertArgument(arg: TreeViewItemHandleArg): any {
const treeView = this.treeViews.get(arg.$treeViewId);
if (treeView && '$treeItemHandle' in arg) {
return treeView.getExtensionElement(arg.$treeItemHandle);
}
if (treeView && '$focusedTreeItem' in arg && arg.$focusedTreeItem) {
return treeView.focusedElement;
}
return null;
return treeView ? treeView.getExtensionElement(arg.$treeItemHandle) : null;
}
}

Expand Down Expand Up @@ -284,9 +270,6 @@ class ExtHostTreeView<T> extends Disposable {
private _selectedHandles: TreeItemHandle[] = [];
get selectedElements(): T[] { return <T[]>this._selectedHandles.map(handle => this.getExtensionElement(handle)).filter(element => !isUndefinedOrNull(element)); }

private _focusedHandle: TreeItemHandle | undefined = undefined;
get focusedElement(): T | undefined { return <T | undefined>(this._focusedHandle ? this.getExtensionElement(this._focusedHandle) : undefined); }

private _onDidExpandElement: Emitter<vscode.TreeViewExpansionEvent<T>> = this._register(new Emitter<vscode.TreeViewExpansionEvent<T>>());
readonly onDidExpandElement: Event<vscode.TreeViewExpansionEvent<T>> = this._onDidExpandElement.event;

Expand Down Expand Up @@ -466,10 +449,6 @@ class ExtHostTreeView<T> extends Disposable {
}
}

setFocus(treeItemHandle: TreeItemHandle) {
this._focusedHandle = treeItemHandle;
}

setVisible(visible: boolean): void {
if (visible !== this._visible) {
this._visible = visible;
Expand Down
32 changes: 4 additions & 28 deletions src/vs/workbench/browser/parts/views/treeView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import { API_OPEN_DIFF_EDITOR_COMMAND_ID, API_OPEN_EDITOR_COMMAND_ID } from 'vs/
import { IViewPaneOptions, ViewPane } from 'vs/workbench/browser/parts/views/viewPane';
import { IViewletViewOptions } from 'vs/workbench/browser/parts/views/viewsViewlet';
import { PANEL_BACKGROUND, SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme';
import { Extensions, ITreeItem, ITreeItemLabel, ITreeView, ITreeViewDataProvider, ITreeViewDescriptor, ITreeViewDragAndDropController, IViewBadge, IViewDescriptorService, IViewsRegistry, ResolvableTreeItem, TreeCommand, TreeItemCollapsibleState, TreeViewItemHandleArg, TreeViewPaneHandleArg, ViewContainer, ViewContainerLocation } from 'vs/workbench/common/views';
import { Extensions, ITreeItem, ITreeItemLabel, ITreeView, ITreeViewDataProvider, ITreeViewDescriptor, ITreeViewDragAndDropController, IViewBadge, IViewDescriptorService, IViewsRegistry, ResolvableTreeItem, TreeCommand, TreeItemCollapsibleState, TreeViewItemHandleArg, ViewContainer, ViewContainerLocation } from 'vs/workbench/common/views';
import { IActivityService, NumberBadge } from 'vs/workbench/services/activity/common/activity';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IHoverService } from 'vs/workbench/services/hover/browser/hover';
Expand All @@ -71,7 +71,6 @@ export class TreeViewPane extends ViewPane {

protected readonly treeView: ITreeView;
private _container: HTMLElement | undefined;
private _actionRunner: MultipleSelectionActionRunner;

constructor(
options: IViewletViewOptions,
Expand All @@ -84,9 +83,8 @@ 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);
super({ ...(options as IViewPaneOptions), titleMenuId: MenuId.ViewTitle, donotForwardArgs: true }, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, telemetryService);
const { treeView } = (<ITreeViewDescriptor>Registry.as<IViewsRegistry>(Extensions.ViewsRegistry).getView(options.id));
this.treeView = treeView;
this._register(this.treeView.onDidChangeActions(() => this.updateActions(), this));
Expand All @@ -105,7 +103,6 @@ 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();
}
Expand Down Expand Up @@ -145,15 +142,6 @@ export class TreeViewPane extends ViewPane {
private updateTreeVisibility(): void {
this.treeView.setVisibility(this.isBodyVisible());
}

override getActionRunner() {
return this._actionRunner;
}

override getActionsContext(): TreeViewPaneHandleArg {
return { $treeViewId: this.id, $focusedTreeItem: true, $selectedTreeItems: true };
}

}

class Root implements ITreeItem {
Expand Down Expand Up @@ -218,9 +206,6 @@ abstract class AbstractTreeView extends Disposable implements ITreeView {
private _onDidChangeSelection: Emitter<ITreeItem[]> = this._register(new Emitter<ITreeItem[]>());
readonly onDidChangeSelection: Event<ITreeItem[]> = this._onDidChangeSelection.event;

private _onDidChangeFocus: Emitter<ITreeItem> = this._register(new Emitter<ITreeItem>());
readonly onDidChangeFocus: Event<ITreeItem> = this._onDidChangeFocus.event;

private readonly _onDidChangeVisibility: Emitter<boolean> = this._register(new Emitter<boolean>());
readonly onDidChangeVisibility: Event<boolean> = this._onDidChangeVisibility.event;

Expand Down Expand Up @@ -624,11 +609,6 @@ abstract class AbstractTreeView extends Disposable implements ITreeView {
customTreeKey.set(true);
this._register(this.tree.onContextMenu(e => this.onContextMenu(treeMenus, e, actionRunner)));
this._register(this.tree.onDidChangeSelection(e => this._onDidChangeSelection.fire(e.elements)));
this._register(this.tree.onDidChangeFocus(e => {
if (e.elements.length) {
this._onDidChangeFocus.fire(e.elements[0]);
}
}));
this._register(this.tree.onDidChangeCollapseState(e => {
if (!e.node.element) {
return;
Expand Down Expand Up @@ -815,10 +795,6 @@ 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);
Expand Down Expand Up @@ -1250,13 +1226,13 @@ class MultipleSelectionActionRunner extends ActionRunner {
}));
}

override async runAction(action: IAction, context: TreeViewItemHandleArg | TreeViewPaneHandleArg): Promise<void> {
override async runAction(action: IAction, context: TreeViewItemHandleArg): 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 as TreeViewItemHandleArg).$treeItemHandle) || (context as TreeViewPaneHandleArg).$selectedTreeItems) {
if (selected.handle === context.$treeItemHandle) {
actionInSelected = true;
}
return { $treeViewId: context.$treeViewId, $treeItemHandle: selected.handle };
Expand Down
9 changes: 2 additions & 7 deletions src/vs/workbench/browser/parts/views/viewPane.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, IActionRunner } from 'vs/base/common/actions';
import { IAction } 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';
Expand Down Expand Up @@ -298,8 +298,7 @@ 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,
actionRunner: this.getActionRunner()
renderDropdownAsChildElement: true
});

this._register(this.toolbar);
Expand Down Expand Up @@ -520,10 +519,6 @@ export abstract class ViewPane extends Pane implements IView {
return undefined;
}

getActionRunner(): IActionRunner | undefined {
return undefined;
}

getOptimalWidth(): number {
return 0;
}
Expand Down
10 changes: 0 additions & 10 deletions src/vs/workbench/common/views.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,6 @@ export interface ITreeView extends IDisposable {

readonly onDidChangeSelection: Event<ITreeItem[]>;

readonly onDidChangeFocus: Event<ITreeItem>;

readonly onDidChangeVisibility: Event<boolean>;

readonly onDidChangeActions: Event<void>;
Expand Down Expand Up @@ -693,8 +691,6 @@ export interface ITreeView extends IDisposable {

setSelection(items: ITreeItem[]): void;

getSelection(): ITreeItem[];

setFocus(item: ITreeItem): void;

show(container: any): void;
Expand All @@ -714,12 +710,6 @@ export interface ITreeViewDescriptor extends IViewDescriptor {
treeView: ITreeView;
}

export type TreeViewPaneHandleArg = {
$treeViewId: string;
$selectedTreeItems?: boolean;
$focusedTreeItem?: boolean;
};

export type TreeViewItemHandleArg = {
$treeViewId: string;
$treeItemHandle: string;
Expand Down
5 changes: 2 additions & 3 deletions src/vs/workbench/contrib/userDataSync/browser/userDataSyncMergesView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 { INotificationService, Severity } from 'vs/platform/notification/common/notification';
import { Severity } from 'vs/platform/notification/common/notification';
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
import { DEFAULT_EDITOR_ASSOCIATION } from 'vs/workbench/common/editor';

Expand Down Expand Up @@ -66,9 +66,8 @@ 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, notificationService);
super(options, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, telemetryService);
this.userDataSyncPreview = userDataSyncWorkbenchService.userDataSyncPreview;

this._register(this.userDataSyncPreview.onDidChangeResources(() => this.updateSyncButtonEnablement()));
Expand Down

0 comments on commit ad7ce39

Please sign in to comment.