Cleanup Notebook actions (#23368)

* Cleanup Notebook actions

* Comments

src/sql/workbench/contrib/notebook/browser/cellViews/cellToolbar.component.ts

@@ -11,7 +11,7 @@ import { Taskbar, ITaskbarContent } from 'sql/base/browser/ui/taskbar/taskbar';
 import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { DeleteCellAction, EditCellAction, CellToggleMoreActions, MoveCellAction, SplitCellAction } from 'sql/workbench/contrib/notebook/browser/cellToolbarActions';
-import { AddCellAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
+import { AddCodeCellAction, AddTextCellAction, ToggleAddCellDropdownAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
 import { CellTypes } from 'sql/workbench/services/notebook/common/contracts';
 import { DropdownMenuActionViewItem } from 'sql/base/browser/ui/buttonMenu/buttonMenu';
 import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
@@ -79,29 +79,29 @@ export class CellToolbarComponent {
 	private setupActions(): void {
 		this._disposableActions.clear();
 
-		let addCellsButton = this.instantiationService.createInstance(AddCellAction, 'notebook.AddCodeCell', localize('codeCellsPreview', "Add cell"), 'masked-pseudo code');
+		let toggleAddCellDropdown = this.instantiationService.createInstance(ToggleAddCellDropdownAction);
-		this._disposableActions.add(addCellsButton);
+		this._disposableActions.add(toggleAddCellDropdown);
 
-		let addCodeCellButton = this.instantiationService.createInstance(AddCellAction, 'notebook.AddCodeCell', localize('codePreview', "Code cell"), 'masked-pseudo code');
+		let addCodeCellAction = this.instantiationService.createInstance(AddCodeCellAction);
-		addCodeCellButton.cellType = CellTypes.Code;
+		addCodeCellAction.cellType = CellTypes.Code;
-		this._disposableActions.add(addCodeCellButton);
+		this._disposableActions.add(addCodeCellAction);
 
-		let addTextCellButton = this.instantiationService.createInstance(AddCellAction, 'notebook.AddTextCell', localize('textPreview', "Text cell"), 'masked-pseudo markdown');
+		let addTextCellAction = this.instantiationService.createInstance(AddTextCellAction);
-		addTextCellButton.cellType = CellTypes.Markdown;
+		addTextCellAction.cellType = CellTypes.Markdown;
-		this._disposableActions.add(addTextCellButton);
+		this._disposableActions.add(addTextCellAction);
 
-		let moveCellDownButton = this.instantiationService.createInstance(MoveCellAction, 'notebook.MoveCellDown', 'masked-icon move-down', this.buttonMoveDown);
+		let moveCellDownAction = this.instantiationService.createInstance(MoveCellAction, 'notebook.MoveCellDown', 'masked-icon move-down', this.buttonMoveDown);
-		let moveCellUpButton = this.instantiationService.createInstance(MoveCellAction, 'notebook.MoveCellUp', 'masked-icon move-up', this.buttonMoveUp);
+		let moveCellUpAction = this.instantiationService.createInstance(MoveCellAction, 'notebook.MoveCellUp', 'masked-icon move-up', this.buttonMoveUp);
-		this._disposableActions.add(moveCellDownButton);
+		this._disposableActions.add(moveCellDownAction);
-		this._disposableActions.add(moveCellUpButton);
+		this._disposableActions.add(moveCellUpAction);
 
-		let splitCellButton = this.instantiationService.createInstance(SplitCellAction, 'notebook.SplitCellAtCursor', this.buttonSplitCell, 'masked-icon icon-split-cell');
+		let splitCellAction = this.instantiationService.createInstance(SplitCellAction, 'notebook.SplitCellAtCursor', this.buttonSplitCell, 'masked-icon icon-split-cell');
-		splitCellButton.setListener(this._cellContext);
+		splitCellAction.setListener(this._cellContext);
-		splitCellButton.enabled = this.cellModel.cellType !== 'markdown';
+		splitCellAction.enabled = this.cellModel.cellType !== 'markdown';
-		this._disposableActions.add(splitCellButton);
+		this._disposableActions.add(splitCellAction);
 
-		let deleteButton = this.instantiationService.createInstance(DeleteCellAction, 'notebook.DeleteCell', 'masked-icon delete', this.buttonDelete);
+		let deleteAction = this.instantiationService.createInstance(DeleteCellAction, 'notebook.DeleteCell', 'masked-icon delete', this.buttonDelete);
-		this._disposableActions.add(deleteButton);
+		this._disposableActions.add(deleteAction);
 
 		let moreActionsContainer = DOM.$('li.action-item');
 		this._cellToggleMoreActions = this.instantiationService.createInstance(CellToggleMoreActions);
@@ -114,8 +114,8 @@ export class CellToolbarComponent {
 		let addCellDropdownContainer = DOM.$('li.action-item');
 		addCellDropdownContainer.setAttribute('role', 'presentation');
 		let dropdownMenuActionViewItem = new DropdownMenuActionViewItem(
-			addCellsButton,
+			toggleAddCellDropdown,
-			[addCodeCellButton, addTextCellButton],
+			[addCodeCellAction, addTextCellAction],
 			this.contextMenuService,
 			undefined,
 			this._actionBar.actionRunner,
@@ -135,10 +135,10 @@ export class CellToolbarComponent {
 		}
 		taskbarContent.push(
 			{ element: addCellDropdownContainer },
-			{ action: moveCellDownButton },
+			{ action: moveCellDownAction },
-			{ action: moveCellUpButton },
+			{ action: moveCellUpAction },
-			{ action: splitCellButton },
+			{ action: splitCellAction },
-			{ action: deleteButton },
+			{ action: deleteAction },
 			{ element: moreActionsContainer });
 
 		this._actionBar.setContent(taskbarContent);

src/sql/workbench/contrib/notebook/browser/notebook.component.ts

@@ -27,7 +27,7 @@ import { INotebookService, INotebookParams, INotebookEditor, INotebookSection, I
 import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel';
 import { Deferred } from 'sql/base/common/promise';
 import { ITaskbarContent, Taskbar } from 'sql/base/browser/ui/taskbar/taskbar';
-import { AddCellAction, KernelsDropdown, AttachToDropdown, TrustedAction, RunAllCellsAction, ClearAllOutputsAction, CollapseCellsAction, RunParametersAction, NotebookViewsActionProvider } from 'sql/workbench/contrib/notebook/browser/notebookActions';
+import { KernelsDropdown, AttachToDropdown, TrustedAction, RunAllCellsAction, ClearAllOutputsAction, CollapseCellsAction, RunParametersAction, NotebookViewsActionProvider, AddCodeCellAction, AddTextCellAction, ToggleAddCellDropdownAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
 import { DropdownMenuActionViewItem } from 'sql/base/browser/ui/buttonMenu/buttonMenu';
 import { INotebookEditOperation } from 'sql/workbench/api/common/sqlExtHostTypes';
 import { IConnectionDialogService } from 'sql/workbench/services/connection/common/connectionDialogService';
@@ -517,19 +517,19 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 		let spacerElement = document.createElement('li');
 		spacerElement.style.marginLeft = 'auto';
 
-		let addCellsButton = this.instantiationService.createInstance(AddCellAction, 'notebook.AddCodeCell', localize('codeCellsPreview', "Add cell"), 'masked-pseudo code');
+		let toggleAddCellDropdownAction = this.instantiationService.createInstance(ToggleAddCellDropdownAction);
 
-		let addCodeCellButton = this.instantiationService.createInstance(AddCellAction, 'notebook.AddCodeCell', localize('codePreview', "Code cell"), 'masked-pseudo code');
+		let addCodeCellAction = this.instantiationService.createInstance(AddCodeCellAction);
-		addCodeCellButton.cellType = CellTypes.Code;
+		addCodeCellAction.cellType = CellTypes.Code;
 
-		let addTextCellButton = this.instantiationService.createInstance(AddCellAction, 'notebook.AddTextCell', localize('textPreview', "Text cell"), 'masked-pseudo markdown');
+		let addTextCellAction = this.instantiationService.createInstance(AddTextCellAction);
-		addTextCellButton.cellType = CellTypes.Markdown;
+		addTextCellAction.cellType = CellTypes.Markdown;
 
 		this._runAllCellsAction = this.instantiationService.createInstance(RunAllCellsAction, 'notebook.runAllCells', localize('runAllPreview', "Run all"), 'masked-pseudo start-outline');
 
 		let collapseCellsAction = this.instantiationService.createInstance(CollapseCellsAction, 'notebook.collapseCells', true);
 
-		let clearResultsButton = this.instantiationService.createInstance(ClearAllOutputsAction, 'notebook.ClearAllOutputs', true);
+		let clearResultsAction = this.instantiationService.createInstance(ClearAllOutputsAction, 'notebook.ClearAllOutputs', true);
 
 		this._trustedAction = this.instantiationService.createInstance(TrustedAction, 'notebook.Trusted', true);
 		this._trustedAction.enabled = false;
@@ -544,8 +544,8 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 		let buttonDropdownContainer = DOM.$('li.action-item');
 		buttonDropdownContainer.setAttribute('role', 'presentation');
 		let dropdownMenuActionViewItem = new DropdownMenuActionViewItem(
-			addCellsButton,
+			toggleAddCellDropdownAction,
-			[addCodeCellButton, addTextCellButton],
+			[addCodeCellAction, addTextCellAction],
 			this.contextMenuService,
 			undefined,
 			this._actionBar.actionRunner,
@@ -589,7 +589,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 				{ element: spacerElement },
 				{ element: viewsDropdownContainer },
 				{ action: collapseCellsAction },
-				{ action: clearResultsButton },
+				{ action: clearResultsAction },
 				{ action: this._trustedAction },
 				{ action: runParametersAction },
 			];

src/sql/workbench/contrib/notebook/browser/notebookActions.ts

@@ -59,8 +59,13 @@ export const noParameterCell: string = localize('noParametersCell', "This notebo
 export const noParametersInCell: string = localize('noParametersInCell', "This notebook cannot run with parameters until there are parameters added to the parameter cell. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization).");
 export const untitledNotSupported: string = localize('untitledNotSupported', "Run with parameters is not supported for Untitled notebooks. Please save the notebook before continuing. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization).");
 
-// Action to add a cell to notebook based on cell type(code/markdown).
+export class ToggleAddCellDropdownAction extends Action {
-export class AddCellAction extends Action {
+	constructor() {
+		super('notebook.toggleAddCell', localize('codeCellsPreview', "Add cell"))
+	}
+}
+
+export abstract class AddCellAction extends Action {
 	public cellType: CellType;
 
 	constructor(
@@ -95,6 +100,28 @@ export class AddCellAction extends Action {
 	}
 }
 
+/**
+ * Action to add a new Text cell to a Notebook
+ */
+export class AddTextCellAction extends AddCellAction {
+	public override cellType: CellType = 'markdown';
+
+	constructor(@INotebookService notebookService: INotebookService) {
+		super('notebook.AddTextCell', localize('textPreview', "Text cell"), 'masked-pseudo markdown', notebookService);
+	}
+}
+
+/**
+ * Action to add a new Code cell to a Notebook
+ */
+export class AddCodeCellAction extends AddCellAction {
+	public override cellType: CellType = 'code';
+
+	constructor(@INotebookService notebookService: INotebookService) {
+		super('notebook.AddCodeCell', localize('codePreview', "Code cell"), 'masked-pseudo code', notebookService);
+	}
+}
+
 export interface ITooltipState {
 	label: string;
 	baseClass: string;

src/sql/workbench/contrib/notebook/browser/notebookViews/notebookViews.component.ts

@@ -30,7 +30,7 @@ import { NotebookViewsGridComponent } from 'sql/workbench/contrib/notebook/brows
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { DeleteViewAction, InsertCellAction, ViewSettingsAction } from 'sql/workbench/contrib/notebook/browser/notebookViews/notebookViewsActions';
 import { DropdownMenuActionViewItem } from 'sql/base/browser/ui/buttonMenu/buttonMenu';
-import { NotebookViewsActionProvider, AddCellAction, RunAllCellsAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
+import { NotebookViewsActionProvider, RunAllCellsAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
 import * as DOM from 'vs/base/browser/dom';
 import { AnchorAlignment } from 'vs/base/browser/ui/contextview/contextview';
 import { LabeledMenuItemActionItem } from 'sql/platform/actions/browser/menuEntryActionViewItem';
@@ -267,11 +267,11 @@ export class NotebookViewComponent extends AngularDisposable implements INoteboo
 
 		let viewsContainer = document.createElement('li');
 		let viewsActionsProvider = new NotebookViewsActionProvider(viewsContainer, this.views, this.modelReady, this._notebookService, this._notificationService, this._instantiationService);
-		let viewsButton = this._instantiationService.createInstance(AddCellAction, 'notebook.OpenViews', undefined, 'notebook-button masked-pseudo code');
+		let viewsAction = new Action('notebook.OpenViews');
 		let viewsDropdownContainer = DOM.$('li.action-item');
 		viewsDropdownContainer.setAttribute('role', 'presentation');
 		let viewsDropdownMenuActionViewItem = new DropdownMenuActionViewItem(
-			viewsButton,
+			viewsAction,
 			viewsActionsProvider,
 			this._contextMenuService,
 			undefined,

src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts

@@ -7,7 +7,7 @@ import * as assert from 'assert';
 import * as azdata from 'azdata';
 import * as sinon from 'sinon';
 import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService';
-import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, CreateNotebookViewAction, DashboardViewAction, kernelNotSupported, KernelsDropdown, msgChanging, noKernelName, noParameterCell, noParametersInCell, NotebookViewAction, NotebookViewsActionProvider, RunAllCellsAction, RunParametersAction, TrustedAction, untitledNotSupported } from 'sql/workbench/contrib/notebook/browser/notebookActions';
+import { AddCodeCellAction, AddTextCellAction, ClearAllOutputsAction, CollapseCellsAction, CreateNotebookViewAction, DashboardViewAction, kernelNotSupported, KernelsDropdown, msgChanging, noKernelName, noParameterCell, noParametersInCell, NotebookViewAction, NotebookViewsActionProvider, RunAllCellsAction, RunParametersAction, TrustedAction, untitledNotSupported } from 'sql/workbench/contrib/notebook/browser/notebookActions';
 import { ClientSessionStub, ContextViewProviderStub, NotebookComponentStub, NotebookModelStub, NotebookServiceStub, NotebookViewsStub, NotebookViewStub } from 'sql/workbench/contrib/notebook/test/stubs';
 import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon';
 import { ICellModel, INotebookModel, ViewMode } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
@@ -126,11 +126,36 @@ suite('Notebook Actions', function (): void {
 		mockNotebookEditor.reset();
 	});
 
-	test('Add Cell Action', async function (): Promise<void> {
+	test('Add Text Cell Action', async function (): Promise<void> {
+		let testCellType: CellType = 'markdown'
+		let actualCellType: CellType;
+
+		let action = new AddTextCellAction(mockNotebookService.object);
+
+		mockNotebookEditor.setup(x => x.model).returns(() => testNotebookModel);
+
+		// Normal use case
+		mockNotebookEditor.setup(x => x.addCell(TypeMoq.It.isAny(), TypeMoq.It.isAnyNumber())).returns((cellType, index) => { actualCellType = cellType; });
+		let mockNotebookComponent = TypeMoq.Mock.ofType<INotebookEditor>(NotebookComponentStub);
+		mockNotebookComponent.setup(c => c.addCell(TypeMoq.It.isAny(), TypeMoq.It.isAnyNumber())).returns(cellType => {
+			actualCellType = cellType;
+		});
+
+		assert.doesNotThrow(() => action.run(testUri));
+		assert.strictEqual(actualCellType, testCellType);
+
+		// Handle error case
+		mockNotebookEditor.reset();
+		mockNotebookEditor.setup(x => x.model).returns(() => testNotebookModel);
+		mockNotebookEditor.setup(x => x.addCell(TypeMoq.It.isAny(), TypeMoq.It.isAnyNumber())).throws(new Error('Test Error'));
+		await assert.rejects(action.run(URI.parse('untitled')));
+	});
+
+	test('Add Code Cell Action', async function (): Promise<void> {
 		let testCellType: CellType = 'code';
 		let actualCellType: CellType;
 
-		let action = new AddCellAction('TestId', 'TestLabel', 'TestClass', mockNotebookService.object);
+		let action = new AddCodeCellAction(mockNotebookService.object);
 		action.cellType = testCellType;
 
 		mockNotebookEditor.setup(x => x.model).returns(() => testNotebookModel);