Address feedback

src/vs/vscode.d.ts

@@ -1391,7 +1391,7 @@ declare module 'vscode' {
 		getLabel?(node: T): string;
 
 		/**
-		 * Determine if `node` has children and is expandable. Default to return `true` if not provided.
+		 * Determine if `node` has children and is expandable. Default to `true` if not provided.
 		 *
 		 * @param node The node to determine if it has children and is expandable.
 		 * @return A boolean that determines if `node` has children and is expandable.

src/vs/workbench/api/node/extHostTreeExplorers.ts

@@ -38,6 +38,7 @@ export class ExtHostTreeExplorers extends ExtHostTreeExplorersShape {
 
 		return new Disposable(() => {
 			delete this._treeExplorerNodeProviders[providerId];
+			delete this._treeExplorerNodeProviders[providerId];
 		});
 	}
 

src/vs/workbench/browser/parts/activitybar/activitybarPart.ts

@@ -97,7 +97,7 @@ export class ActivitybarPart extends Part implements IActivityService {
 		}
 	}
 
-	getIsEnabledForRegisteredViewlets(): { [viewletId: string]: boolean } {
+	public getIsEnabledForRegisteredViewlets(): { [viewletId: string]: boolean } {
 		const result = {};
 		for (let viewletId in this.registeredViewlets) {
 			result[viewletId] = (this.enabledExternalViewlets.indexOf(viewletId) !== -1);
@@ -105,7 +105,7 @@ export class ActivitybarPart extends Part implements IActivityService {
 		return result;
 	}
 
-	toggleViewlet(viewletId: string): void {
+	public toggleViewlet(viewletId: string): void {
 		const index = this.enabledExternalViewlets.indexOf(viewletId);
 		if (index === -1) {
 			this.enabledExternalViewlets.push(viewletId);
@@ -140,7 +140,7 @@ export class ActivitybarPart extends Part implements IActivityService {
 		const $result = $('.content').appendTo($el);
 
 		// Top Actionbar with action items for each viewlet action
-		this.createViewletSwitcher($result.clone().addClass('position-top'));
+		this.createViewletSwitcher($result.clone());
 
 		return $result;
 	}
@@ -235,7 +235,7 @@ class ViewletActivityAction extends ActivityAction {
 	private lastRun: number = 0;
 
 	private _onOpenExternalViewlet = new Emitter<string>();
-	get onOpenExternalViewlet(): Event<string> { return this._onOpenExternalViewlet.event; };
+	public get onOpenExternalViewlet(): Event<string> { return this._onOpenExternalViewlet.event; };
 
 	constructor(
 		id: string,

src/vs/workbench/parts/explorers/browser/treeExplorerActions.contribution.ts

@@ -29,13 +29,13 @@ export class ToggleExternalViewletAction extends Action {
 	}
 
 	run(): TPromise<any> {
-		const viewletsToggleStataus = this.activityService.getIsEnabledForRegisteredViewlets();
+		const isEnabledForRegisteredViewlets = this.activityService.getIsEnabledForRegisteredViewlets();
 
 		const picks: IPickOpenEntry[] = [];
-		for (let viewletId in viewletsToggleStataus) {
+		for (let viewletId in isEnabledForRegisteredViewlets) {
 			picks.push({
 				id: viewletId,
-				label: (viewletsToggleStataus[viewletId] ? 'Disable ' : 'Enable ') + this.getShortViewletId(viewletId)
+				label: (isEnabledForRegisteredViewlets[viewletId] ? 'Disable ' : 'Enable ') + this.getShortViewletId(viewletId)
 			});
 		}
 

src/vs/workbench/parts/explorers/browser/treeExplorerViewlet.ts

@@ -42,30 +42,30 @@ export class TreeExplorerViewlet extends Viewlet {
 		TreeExplorerViewlet._idCounter++;
 	}
 
-	getId(): string {
+	public getId(): string {
 		return this.externalViewletId;
 	}
 
-	create(parent: Builder): TPromise<void> {
+	public create(parent: Builder): TPromise<void> {
 		super.create(parent);
 
-		this.viewletContainer = parent.div().addClass('custom-tree-explorer-viewlet');
+		this.viewletContainer = parent.div();
 		this.addTreeView();
 
 		return TPromise.as(null);
 	}
 
-	layout(dimension: Dimension): void {
+	public layout(dimension: Dimension): void {
 		this.view.layout(dimension.height, Orientation.VERTICAL);
 	}
 
-	setVisible(visible: boolean): TPromise<void> {
+	public setVisible(visible: boolean): TPromise<void> {
 		return super.setVisible(visible).then(() => {
 			this.view.setVisible(visible).done();
 		});
 	}
 
-	getActions(): IAction[] {
+	public getActions(): IAction[] {
 		return this.view.getActions();
 	}
 
@@ -82,7 +82,7 @@ export class TreeExplorerViewlet extends Viewlet {
 		return tokens[tokens.length - 1];
 	}
 
-	dispose(): void {
+	public dispose(): void {
 		this.view = null;
 	}
 }

src/vs/workbench/parts/explorers/browser/treeExplorerViewletService.ts

@@ -29,7 +29,7 @@ export class TreeExplorerViewletService implements ITreeExplorerViewletService {
 	public _serviceBrand: any;
 
 	private _onTreeExplorerNodeProviderRegistered = new Emitter<String>();
-	get onTreeExplorerNodeProviderRegistered(): Event<string> { return this._onTreeExplorerNodeProviderRegistered.event; };
+	public get onTreeExplorerNodeProviderRegistered(): Event<string> { return this._onTreeExplorerNodeProviderRegistered.event; };
 
 	private _treeExplorerNodeProviders: { [providerId: string]: InternalTreeExplorerNodeProvider };
 
@@ -40,26 +40,26 @@ export class TreeExplorerViewletService implements ITreeExplorerViewletService {
 		this._treeExplorerNodeProviders = Object.create(null);
 	}
 
-	registerTreeExplorerNodeProvider(providerId: string, provider: InternalTreeExplorerNodeProvider): void {
+	public registerTreeExplorerNodeProvider(providerId: string, provider: InternalTreeExplorerNodeProvider): void {
 		this._treeExplorerNodeProviders[providerId] = provider;
 		this._onTreeExplorerNodeProviderRegistered.fire(providerId);
 	}
 
-	hasProvider(providerId: string): boolean {
+	public hasProvider(providerId: string): boolean {
 		return !!this._treeExplorerNodeProviders[providerId];
 	}
 
-	provideRootNode(providerId: string): TPromise<InternalTreeExplorerNode> {
+	public provideRootNode(providerId: string): TPromise<InternalTreeExplorerNode> {
 		const provider = this.getProvider(providerId);
 		return TPromise.wrap(provider.provideRootNode());
 	}
 
-	resolveChildren(providerId: string, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode[]> {
+	public resolveChildren(providerId: string, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode[]> {
 		const provider = this.getProvider(providerId);
 		return TPromise.wrap(provider.resolveChildren(node));
 	}
 
-	executeCommand(providerId: string, node: InternalTreeExplorerNode): TPromise<void> {
+	public executeCommand(providerId: string, node: InternalTreeExplorerNode): TPromise<void> {
 		const provider = this.getProvider(providerId);
 		return TPromise.wrap(provider.executeCommand(node));
 	}

src/vs/workbench/parts/explorers/browser/views/treeExplorerView.ts

@@ -93,7 +93,12 @@ export class TreeExplorerView extends CollapsibleViewletView {
 			return this.treeExplorerViewletService.provideRootNode(this.treeNodeProviderId).then(tree => {
 				this.tree.setInput(tree);
 			});
-		} else {
+		}
+		// Provider registration happens independently of the reading of extension's contribution,
+		// which constructs the viewlet, so it's possible the viewlet is constructed before a provider
+		// is registered.
+		// This renders the viewlet first and wait for a corresponding provider is registered.
+		else {
 			this.treeExplorerViewletService.onTreeExplorerNodeProviderRegistered(providerId => {
 				if (this.treeNodeProviderId === providerId) {
 					return this.treeExplorerViewletService.provideRootNode(this.treeNodeProviderId).then(tree => {

src/vs/workbench/parts/explorers/browser/views/treeExplorerViewer.ts

@@ -29,15 +29,15 @@ export class TreeDataSource implements IDataSource {
 
 	}
 
-	getId(tree: ITree, node: InternalTreeExplorerNode): string {
+	public getId(tree: ITree, node: InternalTreeExplorerNode): string {
 		return node.id.toString();
 	}
 
-	hasChildren(tree: ITree, node: InternalTreeExplorerNode): boolean {
+	public hasChildren(tree: ITree, node: InternalTreeExplorerNode): boolean {
 		return node.hasChildren;
 	}
 
-	getChildren(tree: ITree, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode[]> {
+	public getChildren(tree: ITree, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode[]> {
 		const promise = this.treeExplorerViewletService.resolveChildren(this.treeNodeProviderId, node);
 
 		this.progressService.showWhile(promise, 800);
@@ -45,7 +45,7 @@ export class TreeDataSource implements IDataSource {
 		return promise;
 	}
 
-	getParent(tree: ITree, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode> {
+	public getParent(tree: ITree, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode> {
 		return TPromise.as(null);
 	}
 }
@@ -66,11 +66,11 @@ export class TreeRenderer extends ActionsRenderer implements IRenderer {
 		});
 	}
 
-	getContentHeight(tree: ITree, element: any): number {
+	public getContentHeight(tree: ITree, element: any): number {
 		return 22;
 	}
 
-	renderContents(tree: ITree, node: InternalTreeExplorerNode, domElement: HTMLElement, previousCleanupFn: IElementCallback): IElementCallback {
+	public renderContents(tree: ITree, node: InternalTreeExplorerNode, domElement: HTMLElement, previousCleanupFn: IElementCallback): IElementCallback {
 		const el = $(domElement).clearChildren();
 		const item = $('.custom-viewlet-tree-node-item');
 		item.appendTo(el);
@@ -94,7 +94,7 @@ export class TreeController extends DefaultController {
 		super({ clickBehavior: ClickBehavior.ON_MOUSE_UP /* do not change to not break DND */ });
 	}
 
-	onLeftClick(tree: ITree, node: InternalTreeExplorerNode, event: IMouseEvent, origin: string = 'mouse'): boolean {
+	public onLeftClick(tree: ITree, node: InternalTreeExplorerNode, event: IMouseEvent, origin: string = 'mouse'): boolean {
 		super.onLeftClick(tree, node, event, origin);
 
 		if (node.clickCommand) {
@@ -126,5 +126,5 @@ export class TreeExplorerViewletState implements ITreeExplorerViewletState {
 		this._actionProvider = new TreeExplorerActionProvider(this);
 	}
 
-	get actionProvider() { return this._actionProvider; }
+	public get actionProvider() { return this._actionProvider; }
 }

src/vs/workbench/parts/explorers/common/treeExplorerViewModel.ts

@@ -8,26 +8,20 @@ import { TPromise } from 'vs/base/common/winjs.base';
 import { TreeExplorerNodeProvider } from 'vscode';
 
 export class InternalTreeExplorerNode implements TreeExplorerNodeContent {
-	static idCounter = 1;
+	private static idCounter = 1;
 
 	id: number;
 
-	label: string = 'label';
-	hasChildren: boolean = true;
-	clickCommand: string = null;
+	label: string;
+	hasChildren: boolean;
+	clickCommand: string;
 
 	constructor(node: any, provider: TreeExplorerNodeProvider<any>) {
 		this.id = InternalTreeExplorerNode.idCounter++;
 
-		if (provider.getLabel) {
-			this.label = provider.getLabel(node);
-		}
-		if (provider.getHasChildren) {
-			this.hasChildren = provider.getHasChildren(node);
-		}
-		if (provider.getClickCommand) {
-			this.clickCommand = provider.getClickCommand(node);
-		}
+		this.label = provider.getLabel ? provider.getLabel(node) : node.toString();
+		this.hasChildren = provider.getHasChildren ? provider.getHasChildren(node) : true;
+		this.clickCommand = provider.getClickCommand ? provider.getClickCommand(node) : null;
 	}
 }