diff --git a/src/sql/workbench/contrib/notebook/browser/cellToolbarActions.ts b/src/sql/workbench/contrib/notebook/browser/cellToolbarActions.ts index 8cd53a6d70..d022f37f24 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellToolbarActions.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellToolbarActions.ts @@ -118,37 +118,38 @@ export class CellToggleMoreActions { } public onInit(elementRef: HTMLElement, context: CellContext) { - this._moreActionsElement = elementRef; + this._moreActionsElement = elementRef; if (this._moreActionsElement.childNodes.length > 0) { this._moreActionsElement.removeChild(this._moreActionsElement.childNodes[0]); } this._moreActions = new ActionBar(this._moreActionsElement, { orientation: ActionsOrientation.VERTICAL }); this._moreActions.context = { target: this._moreActionsElement }; let validActions = this._actions.filter(a => a instanceof Separator || a instanceof CellActionBase && a.canRun(context)); - this.removeDuplicatedAndStartingSeparators(validActions); + removeDuplicatedAndStartingSeparators(validActions); this._moreActions.push(this.instantiationService.createInstance(ToggleMoreActions, validActions, context), { icon: true, label: false }); } - - private removeDuplicatedAndStartingSeparators(actions: (Action | CellActionBase)[]): void { - let indexesToRemove: number[] = []; - for (let i = 0; i < actions.length; i++) { - // Never should have a separator at the beginning of the list - if (i === 0 && actions[i] instanceof Separator) { - indexesToRemove.push(0); - } - // Handle multiple separators in a row - if (i > 0 && actions[i] instanceof Separator && actions[i - 1] instanceof Separator) { - indexesToRemove.push(i); - } - } - if (indexesToRemove.length > 0) { - for (let i = indexesToRemove.length - 1; i >= 0; i--) { - actions.splice(indexesToRemove[i], 1); - } - } - } } +export function removeDuplicatedAndStartingSeparators(actions: (Action | CellActionBase)[]): void { + let indexesToRemove: number[] = []; + for (let i = 0; i < actions.length; i++) { + // Handle multiple separators in a row + if (i > 0 && actions[i] instanceof Separator && actions[i - 1] instanceof Separator) { + indexesToRemove.push(i); + } + } + if (indexesToRemove.length > 0) { + for (let i = indexesToRemove.length - 1; i >= 0; i--) { + actions.splice(indexesToRemove[i], 1); + } + } + if (actions[0] instanceof Separator) { + actions.splice(0, 1); + } + if (actions[actions.length - 1] instanceof Separator) { + actions.splice(actions.length - 1, 1); + } +} export class AddCellFromContextAction extends CellActionBase { constructor( diff --git a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts new file mode 100644 index 0000000000..f7cf2e9ebe --- /dev/null +++ b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts @@ -0,0 +1,117 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as TypeMoq from 'typemoq'; +import * as assert from 'assert'; +import { CellToggleMoreActions, RunCellsAction, removeDuplicatedAndStartingSeparators, AddCellFromContextAction, CollapseCellAction } from 'sql/workbench/contrib/notebook/browser/cellToolbarActions'; +import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl'; +import { INotificationService } from 'vs/platform/notification/common/notification'; +import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { TestLifecycleService, TestEnvironmentService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { Separator } from 'vs/base/browser/ui/actionbar/actionbar'; +import { CellContext } from 'sql/workbench/contrib/notebook/browser/cellViews/codeActions'; +import { INotebookService } from 'sql/workbench/services/notebook/browser/notebookService'; +import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; +import * as DOM from 'vs/base/browser/dom'; +import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; +import { ContextMenuService } from 'vs/platform/contextview/browser/contextMenuService'; +import { CellModel } from 'sql/workbench/services/notebook/browser/models/cell'; + +suite('CellToolbarActions', function (): void { + suite('removeDuplicatedAndStartingSeparators', function (): void { + test('Empty actions array is unchanged', function (): void { + const actions = []; + removeDuplicatedAndStartingSeparators(actions); + assert(actions.length === 0); + }); + test('Array with only non-separator actions is unchanged', function (): void { + const actions = [ + TypeMoq.Mock.ofType(RunCellsAction).object, + TypeMoq.Mock.ofType(AddCellFromContextAction).object, + TypeMoq.Mock.ofType(CollapseCellAction).object + ]; + removeDuplicatedAndStartingSeparators(actions); + assert(actions.length === 3); + }); + test('Array with only separators is cleared', function (): void { + const actions = [new Separator(), new Separator(), new Separator()]; + removeDuplicatedAndStartingSeparators(actions); + assert(actions.length === 0); + }); + test('Array with separators not on the ends is unchanged', function (): void { + const actions = [ + TypeMoq.Mock.ofType(RunCellsAction).object, + new Separator(), + TypeMoq.Mock.ofType(AddCellFromContextAction).object, + new Separator(), + TypeMoq.Mock.ofType(CollapseCellAction).object + ]; + removeDuplicatedAndStartingSeparators(actions); + assert(actions.length === 5); + }); + test('Duplicate separators are removed', function (): void { + const actions = [ + TypeMoq.Mock.ofType(RunCellsAction).object, + new Separator(), + new Separator(), + new Separator(), + TypeMoq.Mock.ofType(AddCellFromContextAction).object, + new Separator(), + new Separator(), + TypeMoq.Mock.ofType(CollapseCellAction).object + ]; + removeDuplicatedAndStartingSeparators(actions); + assert(actions.length === 5); + }); + test('Starting and ending separators are removed', function (): void { + const actions = [ + new Separator(), + new Separator(), + TypeMoq.Mock.ofType(RunCellsAction).object, + new Separator(), + TypeMoq.Mock.ofType(AddCellFromContextAction).object, + new Separator(), + TypeMoq.Mock.ofType(CollapseCellAction).object, + new Separator(), + new Separator() + ]; + removeDuplicatedAndStartingSeparators(actions); + assert(actions.length === 5); + }); + }); + + suite('CellToggleMoreActions', function (): void { + const instantiationService: TestInstantiationService = new TestInstantiationService(); + const contextMock = TypeMoq.Mock.ofType(CellContext); + const cellModelMock = TypeMoq.Mock.ofType(CellModel); + + suiteSetup(function (): void { + contextMock.setup(x => x.cell).returns(() => cellModelMock.object); + const mockNotebookService = TypeMoq.Mock.ofType(NotebookService, undefined, new TestLifecycleService(), undefined, undefined, undefined, instantiationService, new MockContextKeyService(), + undefined, instantiationService, undefined, undefined, undefined, undefined, TestEnvironmentService); + instantiationService.stub(INotificationService, new TestNotificationService()); + instantiationService.stub(INotebookService, mockNotebookService.object); + instantiationService.stub(IContextMenuService, TypeMoq.Mock.ofType(ContextMenuService).object); + }); + + test('CellToggleMoreActions with Code CellType', function (): void { + const testContainer = DOM.$('div'); + cellModelMock.setup(x => x.cellType).returns(() => 'code'); + const action = new CellToggleMoreActions(instantiationService); + action.onInit(testContainer, contextMock.object); + assert(action['_moreActions']['viewItems'][0]['_action']['_actions'].length === 13, 'Unexpected number of valid elements'); + }); + + test('CellToggleMoreActions with Markdown CellType', function (): void { + const testContainer = DOM.$('div'); + cellModelMock.setup(x => x.cellType).returns(() => 'markdown'); + const action = new CellToggleMoreActions(instantiationService); + action.onInit(testContainer, contextMock.object); + // Markdown elements don't show the code-cell related actions such as Run Cell + assert(action['_moreActions']['viewItems'][0]['_action']['_actions'].length === 5, 'Unexpected number of valid elements'); + }); + }); +});