Fix toggle more actions staying visible, and clickable issues (#3949)

- Fixed so it's now invisible instead of empty when not selected.
 - This fixes clickability and issue where it stayed visible in 1 fix
- Also fixed cell output action which used active cell instead of context cell.

src/sql/parts/notebook/cellToggleMoreActions.ts

@@ -5,14 +5,12 @@
 
 import { ElementRef } from '@angular/core';
 
-import { nb } from 'sqlops';
-
 import { localize } from 'vs/nls';
-import { Action } from 'vs/base/common/actions';
 import { ActionBar, ActionsOrientation } from 'vs/base/browser/ui/actionbar/actionbar';
 import { getErrorMessage } from 'vs/base/common/errors';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
+import * as DOM from 'vs/base/browser/dom';
 
 import { ICellModel } from 'sql/parts/notebook/models/modelInterfaces';
 import { CellContext, CellActionBase } from 'sql/parts/notebook/cellViews/codeActions';
@@ -21,9 +19,12 @@ import { ToggleMoreWidgetAction } from 'sql/parts/dashboard/common/actions';
 import { CellTypes, CellType } from 'sql/parts/notebook/models/contracts';
 import { CellModel } from 'sql/parts/notebook/models/cell';
 
+export const HIDDEN_CLASS ='actionhidden';
+
 export class CellToggleMoreActions {
 	private _actions: CellActionBase[] = [];
 	private _moreActions: ActionBar;
+	private _moreActionsElement: HTMLElement;
 	constructor(
 		@IInstantiationService private instantiationService: IInstantiationService) {
 		this._actions.push(
@@ -36,20 +37,26 @@ export class CellToggleMoreActions {
 		);
 	}
 
-	public toggle(showIcon: boolean, elementRef: ElementRef, model: NotebookModel, cellModel: ICellModel) {
+	public onInit(elementRef: ElementRef, model: NotebookModel, cellModel: ICellModel) {
 		let context = new CellContext(model,cellModel);
-		let moreActionsElement = <HTMLElement>elementRef.nativeElement;
+		this._moreActionsElement = <HTMLElement>elementRef.nativeElement;
-		if (showIcon) {
+		if (this._moreActionsElement.childNodes.length > 0) {
-			if (moreActionsElement.childNodes.length > 0) {
+			this._moreActionsElement.removeChild(this._moreActionsElement.childNodes[0]);
-				moreActionsElement.removeChild(moreActionsElement.childNodes[0]);
 		}
-			this._moreActions = new ActionBar(moreActionsElement, { orientation: ActionsOrientation.VERTICAL });
+		this._moreActions = new ActionBar(this._moreActionsElement, { orientation: ActionsOrientation.VERTICAL, isMenu: true });
-			this._moreActions.context = { target: moreActionsElement };
+		this._moreActions.context = { target: this._moreActionsElement };
 		let validActions = this._actions.filter(a => a.canRun(context));
-			this._moreActions.push(this.instantiationService.createInstance(ToggleMoreWidgetAction, validActions, context), { icon: showIcon, label: false });
+		this._moreActions.push(this.instantiationService.createInstance(ToggleMoreWidgetAction, validActions, context), { icon: true, label: false });
 	}
-		else if (moreActionsElement.childNodes.length > 0) {
+
-			moreActionsElement.removeChild(moreActionsElement.childNodes[0]);
+	public toggleVisible(visible: boolean): void {
+		if (!this._moreActionsElement) {
+			return;
+		}
+		if (visible) {
+			DOM.addClass(this._moreActionsElement, HIDDEN_CLASS);
+		} else {
+			DOM.removeClass(this._moreActionsElement, HIDDEN_CLASS);
 		}
 	}
 }
@@ -118,7 +125,10 @@ export class ClearCellOutputAction extends CellActionBase {
 
 	doRun(context: CellContext): Promise<void> {
 		try {
-			(context.model.activeCell as CellModel).clearOutputs();
+			let cell = context.cell || context.model.activeCell;
+			if (cell) {
+				(cell as CellModel).clearOutputs();
+			}
 		} catch (error) {
 			let message = getErrorMessage(error);
 

src/sql/parts/notebook/cellViews/code.component.ts

@@ -42,7 +42,6 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 	@ViewChild('moreactions', { read: ElementRef }) private moreActionsElementRef: ElementRef;
 	@ViewChild('editor', { read: ElementRef }) private codeElement: ElementRef;
 	@Input() cellModel: ICellModel;
-	@Input() hideVerticalToolbar: boolean = false;
 
 	@Output() public onContentChanged = new EventEmitter<void>();
 
@@ -92,10 +91,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 	ngOnInit() {
 		this._register(this.themeService.onDidColorThemeChange(this.updateTheme, this));
 		this.updateTheme(this.themeService.getColorTheme());
-		if (!this.hideVerticalToolbar) {
 		this.initActionBar();
 	}
-	}
 
 	ngOnChanges(changes: { [propKey: string]: SimpleChange }) {
 		this.updateLanguageMode();
@@ -104,7 +101,7 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 			if (propName === 'activeCellId') {
 				let changedProp = changes[propName];
 				let isActive = this.cellModel.id === changedProp.currentValue;
-				this._cellToggleMoreActions.toggle(isActive, this.moreActionsElementRef, this.model, this.cellModel);
+				this.toggleMoreActionsButton(isActive);
 				if (this._editor) {
 					this._editor.toggleEditorSelected(isActive);
 				}
@@ -173,6 +170,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 		this._actionBar.setContent([
 			{ action: runCellAction }
 		]);
+
+		this._cellToggleMoreActions.onInit(this.moreActionsElementRef, this.model, this.cellModel);
 	}
 
 
@@ -217,7 +216,7 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 		return this.cellModel && this.cellModel.id === this.activeCellId;
 	}
 
-	protected toggleMoreActionsButton(isActive: boolean) {
+	protected toggleMoreActionsButton(isActiveOrHovered: boolean) {
-		this._cellToggleMoreActions.toggle(isActive, this.moreActionsElementRef, this.model, this.cellModel);
+		this._cellToggleMoreActions.toggleVisible(!isActiveOrHovered);
 	}
 }

src/sql/parts/notebook/cellViews/textCell.component.html

@@ -7,7 +7,7 @@
 <div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: column" (mouseover)="hover=true" (mouseleave)="hover=false">
 	<loading-spinner [loading]="isLoading"></loading-spinner>
 	<div class="notebook-text" style="flex: 0 0 auto;">
-		<code-component *ngIf="isEditMode" [cellModel]="cellModel" (onContentChanged)="handleContentChanged()" [model]="model" [activeCellId]="activeCellId" [hideVerticalToolbar]=true>
+		<code-component *ngIf="isEditMode" [cellModel]="cellModel" (onContentChanged)="handleContentChanged()" [model]="model" [activeCellId]="activeCellId">
 		</code-component>
 	</div>
 	<div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: row">

src/sql/parts/notebook/cellViews/textCell.component.ts

@@ -99,6 +99,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 		this.setLoading(false);
 		this._register(this.themeService.onDidColorThemeChange(this.updateTheme, this));
 		this.updateTheme(this.themeService.getColorTheme());
+		this._cellToggleMoreActions.onInit(this.moreActionsElementRef, this.model, this.cellModel);
 		this.setFocusAndScroll();
 		this._register(this.cellModel.onOutputsChanged(e => {
 			this.updatePreview();
@@ -193,7 +194,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 		return this.cellModel && this.cellModel.id === this.activeCellId;
 	}
 
-	protected toggleMoreActionsButton(isActive: boolean) {
+	protected toggleMoreActionsButton(isActiveOrHovered: boolean) {
-		this._cellToggleMoreActions.toggle(isActive, this.moreActionsElementRef, this.model, this.cellModel);
+		this._cellToggleMoreActions.toggleVisible(!isActiveOrHovered);
 	}
 }

src/sql/parts/notebook/notebook.css

@@ -81,3 +81,6 @@
 	height: 20px;
 	width: 20px;
 }
+.moreActions.actionhidden {
+	visibility: hidden
+}