Cell toolbar More Actions menu -- remove duplicate separators (#10827)

* Added code to remove multiple separators from more actions menu.

* Moved check for separators after list is deduped.

* Logic cleanup

* Added unit test for cell toolbar ellipses menu.

* Fix tests

Co-authored-by: chgagnon <chgagnon@microsoft.com>
This commit is contained in:
Hale Rankin
2020-06-26 13:45:53 -07:00
committed by GitHub
parent dbf9d3426c
commit 8d7b3ef6ec
2 changed files with 139 additions and 21 deletions

View File

@@ -118,37 +118,38 @@ export class CellToggleMoreActions {
}
public onInit(elementRef: HTMLElement, context: CellContext) {
this._moreActionsElement = <HTMLElement>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(

View File

@@ -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');
});
});
});