Fix New Notebook issues (#5958)

* Fix New Notebook issues
- Fix #5338 New Notebook menu item should be next to New Query
- Fix #4936 Add a shortcut to create a notebook in the document well

Created a built-in New Notebook command
that routes to the existing extension-based command.
This avoided a rearchitecture that was more complex that seemed worth it.
Per VSCode patterns, used a _ modifier for the existing command so it's "private"

extensions/notebook/package.json

@@ -45,7 +45,7 @@
 				"title": "%notebook.analyzeJupyterNotebook%"
 			},
 			{
-				"command": "notebook.command.new",
+				"command": "_notebook.command.new",
 				"title": "%notebook.command.new%",
 				"icon": {
 					"dark": "resources/dark/new_notebook_inverse.svg",
@@ -145,7 +145,8 @@
 					"when": "false"
 				},
 				{
-					"command": "notebook.command.new"
+					"command": "_notebook.command.new",
+					"when": "false"
 				},
 				{
 					"command": "notebook.command.open"
@@ -204,11 +205,6 @@
 				}
 			],
 			"objectExplorer/item/context": [
-				{
-					"command": "notebook.command.new",
-					"when": "isQueryProvider && nodeType == Server",
-					"group": "1root@1"
-				},
 				{
 					"command": "notebook.command.analyzeNotebook",
 					"when": "nodeType=~/^mssqlCluster/ && nodeLabel=~/[^\\s]+(\\.(csv|tsv|txt))$/ && nodeType == mssqlCluster:file",
@@ -233,10 +229,6 @@
 			]
 		},
 		"keybindings": [
-			{
-				"command": "notebook.command.new",
-				"key": "Ctrl+Shift+N"
-			},
 			{
 				"command": "notebook.command.runactivecell",
 				"key": "F5",

extensions/notebook/src/extension.ts

@@ -25,7 +25,7 @@ let controller: JupyterController;
 type ChooseCellType = { label: string, id: CellType };
 
 export async function activate(extensionContext: vscode.ExtensionContext): Promise<IExtensionApi> {
-	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.new', (context?: azdata.ConnectedContext) => {
+	extensionContext.subscriptions.push(vscode.commands.registerCommand('_notebook.command.new', (context?: azdata.ConnectedContext) => {
 		let connectionProfile: azdata.IConnectionProfile = undefined;
 		if (context && context.connectionProfile) {
 			connectionProfile = context.connectionProfile;

src/sql/workbench/parts/notebook/media/dark/new_notebook_inverse.svg

@@ -0,0 +1 @@
+<svg id="Layer_1" data-name="Layer 1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><defs><style>.cls-1{fill:#fff;}.cls-2{fill:#388a34;}</style></defs><title>new_notebook_inverse</title><path class="cls-1" d="M11.87,1.24V.33H9.13A3.78,3.78,0,0,0,7.92.52a3.48,3.48,0,0,0-1.07.58A3.6,3.6,0,0,0,5.78.52,3.78,3.78,0,0,0,4.57.33H1.83v.91H0V13.1H9.67v-.91H7a4,4,0,0,1,.47-.39A2.39,2.39,0,0,1,8,11.52a2.2,2.2,0,0,1,.53-.18,2.93,2.93,0,0,1,.61-.06h2.74V2.15h.91V9h.91V1.24Zm-9.13,0H4.57a3,3,0,0,1,1,.17,2.58,2.58,0,0,1,.85.49v8.93a3.94,3.94,0,0,0-.88-.35,3.73,3.73,0,0,0-.94-.12H2.74Zm-1.82,11v-10h.91v9.13H4.57a2.93,2.93,0,0,1,.61.06,2.55,2.55,0,0,1,.53.18,2.68,2.68,0,0,1,.49.28,3.29,3.29,0,0,1,.46.39Zm8.21-1.83a3.73,3.73,0,0,0-.94.12,4.22,4.22,0,0,0-.89.35V1.9a2.74,2.74,0,0,1,.86-.49,2.91,2.91,0,0,1,1-.17H11v9.12ZM12.87,10v2.2h-2.2v.91h3V10Z"/><polygon class="cls-2" points="16 12.19 16 13.13 13.8 13.13 13.8 15.33 12.87 15.33 12.87 13.13 10.67 13.13 10.67 12.19 12.87 12.19 12.87 9.99 13.8 9.99 13.8 12.19 16 12.19"/><path class="cls-2" d="M13.8,12.19V10h-.93v2.2h-2.2v.94h2.2v2.2h.93v-2.2H16v-.94Z"/></svg>

src/sql/workbench/parts/notebook/media/light/new_notebook.svg

@@ -0,0 +1 @@
+<svg id="Layer_1" data-name="Layer 1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><defs><style>.cls-1{fill:#388a34;}</style></defs><title>new_notebook</title><path d="M11.86,1.24V.33H9.13A3.78,3.78,0,0,0,7.91.52a3.48,3.48,0,0,0-1.07.58A3.6,3.6,0,0,0,5.78.52,3.78,3.78,0,0,0,4.57.33H1.83v.91H0V13.1H9.66v-.91H7a4,4,0,0,1,.47-.39A2.39,2.39,0,0,1,8,11.52a2.2,2.2,0,0,1,.53-.18,2.93,2.93,0,0,1,.61-.06h2.74V2.15h.91V9h.91V1.24Zm-9.13,0H4.57a3,3,0,0,1,1,.17,2.58,2.58,0,0,1,.85.49v8.93a3.94,3.94,0,0,0-.88-.35,3.73,3.73,0,0,0-.94-.12H2.73Zm-1.82,11v-10h.91v9.13H4.57a2.93,2.93,0,0,1,.61.06,2.55,2.55,0,0,1,.53.18,2.68,2.68,0,0,1,.49.28,3.29,3.29,0,0,1,.46.39Zm8.21-1.83a3.73,3.73,0,0,0-.94.12,4.22,4.22,0,0,0-.89.35V1.9a2.74,2.74,0,0,1,.86-.49,2.91,2.91,0,0,1,1-.17h1.82v9.12ZM12.86,10v2.2h-2.2v.91h3V10Z"/><polygon class="cls-1" points="15.99 12.19 15.99 13.13 13.79 13.13 13.79 15.33 12.87 15.33 12.87 13.13 10.66 13.13 10.66 12.19 12.87 12.19 12.87 9.99 13.79 9.99 13.79 12.19 15.99 12.19"/><path class="cls-1" d="M13.79,12.19V10h-.93v2.2h-2.2v.94h2.2v2.2h.93v-2.2H16v-.94Z"/></svg>

src/sql/workbench/parts/notebook/notebook.contribution.ts

@@ -5,9 +5,14 @@
 import { Registry } from 'vs/platform/registry/common/platform';
 import { EditorDescriptor, IEditorRegistry, Extensions as EditorExtensions } from 'vs/workbench/browser/editor';
 import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
+import { IWorkbenchActionRegistry, Extensions } from 'vs/workbench/common/actions';
+import { SyncActionDescriptor } from 'vs/platform/actions/common/actions';
 
 import { NotebookInput } from 'sql/workbench/parts/notebook/notebookInput';
 import { NotebookEditor } from 'sql/workbench/parts/notebook/notebookEditor';
+import { NewNotebookAction } from 'sql/workbench/parts/notebook/notebookActions';
+import { KeyMod } from 'vs/editor/common/standalone/standaloneBase';
+import { KeyCode } from 'vs/base/common/keyCodes';
 
 // Model View editor registration
 const viewModelEditorDescriptor = new EditorDescriptor(
@@ -18,3 +23,17 @@ const viewModelEditorDescriptor = new EditorDescriptor(
 
 Registry.as<IEditorRegistry>(EditorExtensions.Editors)
 	.registerEditor(viewModelEditorDescriptor, [new SyncDescriptor(NotebookInput)]);
+
+// Global Actions
+let actionRegistry = <IWorkbenchActionRegistry>Registry.as(Extensions.WorkbenchActions);
+
+actionRegistry.registerWorkbenchAction(
+	new SyncActionDescriptor(
+		NewNotebookAction,
+		NewNotebookAction.ID,
+		NewNotebookAction.LABEL,
+		{ primary: KeyMod.WinCtrl | KeyMod.Shift | KeyCode.KEY_N },
+
+	),
+	NewNotebookAction.LABEL
+);

src/sql/workbench/parts/notebook/notebook.css

@@ -115,4 +115,13 @@
 
 .notebookEditor .notebook-cellTable .slick-viewport {
 	min-height: 39px;
-}
+}
+
+.monaco-workbench .notebook-action.new-notebook {
+	background: url('./media/light/new_notebook.svg') center center no-repeat;
+}
+
+.vs-dark .monaco-workbench .notebook-action.new-notebook,
+.hc-black .monaco-workbench .notebook-action.new-notebook {
+	background: url('./media/dark/new_notebook_inverse.svg') center center no-repeat;
+}

src/sql/workbench/parts/notebook/notebookActions.ts

@@ -24,6 +24,7 @@ import { NotebookModel } from 'sql/workbench/parts/notebook/models/notebookModel
 import { generateUri } from 'sql/platform/connection/common/utils';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 import { ILogService } from 'vs/platform/log/common/log';
+import { ICommandService } from 'vs/platform/commands/common/commands';
 
 const msgLoading = localize('loading', "Loading kernels...");
 const msgChanging = localize('changing', "Changing kernel...");
@@ -539,3 +540,25 @@ export class AttachToDropdown extends SelectBox {
 		}
 	}
 }
+
+export class NewNotebookAction extends Action {
+
+	public static readonly ID = 'notebook.command.new';
+	public static readonly LABEL = localize('newNotebookAction', "New Notebook");
+
+	private static readonly INTERNAL_NEW_NOTEBOOK_CMD_ID = '_notebook.command.new';
+	constructor(
+		id: string,
+		label: string,
+		@ICommandService private commandService: ICommandService
+	) {
+		super(id, label);
+		this.class = 'notebook-action new-notebook';
+	}
+
+	run(context?: azdata.ConnectedContext): Promise<void> {
+		return this.commandService.executeCommand(NewNotebookAction.INTERNAL_NEW_NOTEBOOK_CMD_ID, context);
+
+	}
+
+}

src/sql/workbench/parts/objectExplorer/browser/serverTreeActionProvider.ts

@@ -31,6 +31,7 @@ import { IQueryManagementService } from 'sql/platform/query/common/queryManageme
 import { IScriptingService } from 'sql/platform/scripting/common/scriptingService';
 import { ServerInfoContextKey } from 'sql/workbench/parts/connection/common/serverInfoContextKey';
 import { fillInActions } from 'vs/platform/actions/browser/menuEntryActionViewItem';
+import { NewNotebookAction } from 'sql/workbench/parts/notebook/notebookActions';
 
 /**
  *  Provides actions for the server tree elements
@@ -111,7 +112,7 @@ export class ServerTreeActionProvider extends ContributableActionProvider {
 	private getBuiltinConnectionActions(context: ObjectExplorerContext): IAction[] {
 		let actions: IAction[] = [];
 		actions.push(this._instantiationService.createInstance(ManageConnectionAction, ManageConnectionAction.ID, ManageConnectionAction.LABEL, context.tree));
-		this.addNewQueryAction(context, actions);
+		this.addNewQueryNotebookActions(context, actions);
 
 		if (this._connectionManagementService.isProfileConnected(context.profile)) {
 			actions.push(this._instantiationService.createInstance(DisconnectConnectionAction, DisconnectConnectionAction.ID, DisconnectConnectionAction.LABEL, context.profile));
@@ -165,7 +166,7 @@ export class ServerTreeActionProvider extends ContributableActionProvider {
 			if (TreeUpdateUtils.isAvailableDatabaseNode(treeNode)) {
 				isAvailableDatabaseNode = true;
 				actions.push(this._instantiationService.createInstance(ManageConnectionAction, ManageConnectionAction.ID, ManageConnectionAction.LABEL, context.tree));
-				this.addNewQueryAction(context, actions);
+				this.addNewQueryNotebookActions(context, actions);
 			} else {
 				return actions;
 			}
@@ -186,9 +187,10 @@ export class ServerTreeActionProvider extends ContributableActionProvider {
 		return actions;
 	}
 
-	private addNewQueryAction(context: ObjectExplorerContext, actions: IAction[]): void {
+	private addNewQueryNotebookActions(context: ObjectExplorerContext, actions: IAction[]): void {
 		if (this._queryManagementService.isProviderRegistered(context.profile.providerName)) {
 			actions.push(this._instantiationService.createInstance(OEAction, NewQueryAction.ID, NewQueryAction.LABEL));
+			actions.push(this._instantiationService.createInstance(OEAction, NewNotebookAction.ID, NewNotebookAction.LABEL));
 		}
 	}
 

src/vs/workbench/contrib/watermark/browser/watermark.ts

@@ -31,6 +31,7 @@ import { IDimension } from 'vs/platform/layout/browser/layoutService';
 
 // {{SQL CARBON EDIT}}
 import { OpenDataExplorerViewletAction } from 'sql/workbench/parts/dataExplorer/browser/dataExplorer.contribution';
+import { NewNotebookAction } from 'sql/workbench/parts/notebook/notebookActions';
 
 const $ = dom.$;
 
@@ -42,14 +43,18 @@ interface WatermarkEntry {
 
 // {{SQL CARBON EDIT}}
 const showServers: WatermarkEntry = {
-	text: nls.localize('watermark.showServers', 'Show Servers'),
+	text: nls.localize('watermark.showServers', "Show Servers"),
 	id: OpenDataExplorerViewletAction.ID
 };
 
 const newSqlFile: WatermarkEntry = {
-	text: nls.localize('watermark.newSqlFile', 'New SQL File'),
+	text: nls.localize('watermark.newSqlFile', "New SQL File"),
 	id: GlobalNewUntitledFileAction.ID
 };
+const newNotebook: WatermarkEntry = {
+	text: nls.localize('watermark.newNotebook', "New Notebook"),
+	id: NewNotebookAction.ID
+};
 
 const showCommands: WatermarkEntry = {
 	text: nls.localize('watermark.showCommands', "Show All Commands"),
@@ -101,12 +106,14 @@ const startDebugging: WatermarkEntry = {
 const noFolderEntries = [
 	showServers,
 	newSqlFile,
+	newNotebook,
 	findInFiles
 ];
 
 const folderEntries = [
 	showServers,
 	newSqlFile,
+	newNotebook,
 	findInFiles
 ];
 // {{SQL CARBON EDIT}} - End