From 40e0d5cfbfeec1934b5d4a7ed3de47b26aa7779a Mon Sep 17 00:00:00 2001 From: Kevin Cunnane Date: Thu, 7 Feb 2019 09:35:58 -0800 Subject: [PATCH] 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. --- .../parts/notebook/cellToggleMoreActions.ts | 42 ++++++++++++------- .../notebook/cellViews/code.component.ts | 13 +++--- .../cellViews/textCell.component.html | 2 +- .../notebook/cellViews/textCell.component.ts | 5 ++- src/sql/parts/notebook/notebook.css | 3 ++ 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/sql/parts/notebook/cellToggleMoreActions.ts b/src/sql/parts/notebook/cellToggleMoreActions.ts index ef0f8020ee..82c7496354 100644 --- a/src/sql/parts/notebook/cellToggleMoreActions.ts +++ b/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 = elementRef.nativeElement; - if (showIcon) { - if (moreActionsElement.childNodes.length > 0) { - moreActionsElement.removeChild(moreActionsElement.childNodes[0]); - } - this._moreActions = new ActionBar(moreActionsElement, { orientation: ActionsOrientation.VERTICAL }); - this._moreActions.context = { target: moreActionsElement }; - let validActions = this._actions.filter(a => a.canRun(context)); - this._moreActions.push(this.instantiationService.createInstance(ToggleMoreWidgetAction, validActions, context), { icon: showIcon, label: false }); + this._moreActionsElement = elementRef.nativeElement; + if (this._moreActionsElement.childNodes.length > 0) { + this._moreActionsElement.removeChild(this._moreActionsElement.childNodes[0]); } - else if (moreActionsElement.childNodes.length > 0) { - moreActionsElement.removeChild(moreActionsElement.childNodes[0]); + this._moreActions = new ActionBar(this._moreActionsElement, { orientation: ActionsOrientation.VERTICAL, isMenu: true }); + 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: true, label: false }); + } + + 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 { 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); diff --git a/src/sql/parts/notebook/cellViews/code.component.ts b/src/sql/parts/notebook/cellViews/code.component.ts index d5dd019dc2..b31a215064 100644 --- a/src/sql/parts/notebook/cellViews/code.component.ts +++ b/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(); @@ -92,9 +91,7 @@ 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(); - } + this.initActionBar(); } ngOnChanges(changes: { [propKey: string]: SimpleChange }) { @@ -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) { - this._cellToggleMoreActions.toggle(isActive, this.moreActionsElementRef, this.model, this.cellModel); + protected toggleMoreActionsButton(isActiveOrHovered: boolean) { + this._cellToggleMoreActions.toggleVisible(!isActiveOrHovered); } } diff --git a/src/sql/parts/notebook/cellViews/textCell.component.html b/src/sql/parts/notebook/cellViews/textCell.component.html index 1f8501d9f6..c6a18bb4db 100644 --- a/src/sql/parts/notebook/cellViews/textCell.component.html +++ b/src/sql/parts/notebook/cellViews/textCell.component.html @@ -7,7 +7,7 @@
- +
diff --git a/src/sql/parts/notebook/cellViews/textCell.component.ts b/src/sql/parts/notebook/cellViews/textCell.component.ts index ed28494ed3..10997d92c3 100644 --- a/src/sql/parts/notebook/cellViews/textCell.component.ts +++ b/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) { - this._cellToggleMoreActions.toggle(isActive, this.moreActionsElementRef, this.model, this.cellModel); + protected toggleMoreActionsButton(isActiveOrHovered: boolean) { + this._cellToggleMoreActions.toggleVisible(!isActiveOrHovered); } } diff --git a/src/sql/parts/notebook/notebook.css b/src/sql/parts/notebook/notebook.css index 2cc8d13314..e2ce0ad489 100644 --- a/src/sql/parts/notebook/notebook.css +++ b/src/sql/parts/notebook/notebook.css @@ -80,4 +80,7 @@ .moreActions .action-label.icon.toggle-more { height: 20px; width: 20px; +} +.moreActions.actionhidden { + visibility: hidden } \ No newline at end of file