diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts index 0cee9939a0..35298da239 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts @@ -427,11 +427,8 @@ export class CodeComponent extends CellView implements OnInit, OnChanges { private onCellModeChanged(isEditMode: boolean): void { if (this.cellModel.id === this._activeCellId || this._activeCellId === '') { - if (isEditMode) { - this._editor.getContainer().contentEditable = 'true'; - this._editor.getControl().focus(); - } else { - this._editor.getContainer().contentEditable = 'false'; + this._editor.getControl().focus(); + if (!isEditMode) { (document.activeElement as HTMLElement).blur(); } } diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/codeCell.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/codeCell.component.ts index 6ae60823b6..a543f99e35 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/codeCell.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/codeCell.component.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { nb } from 'azdata'; -import { OnInit, Component, Input, Inject, forwardRef, ChangeDetectorRef, SimpleChange, OnChanges, HostListener, ViewChildren, QueryList, ViewChild } from '@angular/core'; +import { OnInit, Component, Input, Inject, forwardRef, ChangeDetectorRef, SimpleChange, OnChanges, ViewChildren, QueryList, ViewChild } from '@angular/core'; import { CellView } from 'sql/workbench/contrib/notebook/browser/cellViews/interfaces'; import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; @@ -31,11 +31,6 @@ export class CodeCellComponent extends CellView implements OnInit, OnChanges { this._activeCellId = value; } - // On click to edit code cell in notebook - @HostListener('click', ['$event']) onClick() { - this.toggleEditMode(); - } - private _activeCellId: string; public inputDeferred: Deferred; diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts index 0e03ed2be4..7dfb7ce7b9 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts @@ -548,7 +548,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges { if (!this.isEditMode && this.doubleClickEditEnabled) { this.toggleEditMode(true); } - this._model.updateActiveCell(this.cellModel); + this._model.updateActiveCell(this.cellModel, true); } } diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.component.html b/src/sql/workbench/contrib/notebook/browser/notebook.component.html index 8ec254e04b..6133619bc7 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.component.html +++ b/src/sql/workbench/contrib/notebook/browser/notebook.component.html @@ -7,10 +7,10 @@
-
+
-
+
diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts index 567a80df73..c84f0200d6 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts @@ -140,8 +140,11 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe // on its elements (we have a "virtual" focus that is updated as users click or navigate through cells). So some of the keyboard // events we care about are fired when the document focus is on something else - typically the root window. this._register(DOM.addDisposableListener(window, DOM.EventType.KEY_DOWN, (e: KeyboardEvent) => { - // Make sure that the current active element is an ancestor - this is to prevent us from handling events when the focus is - // on some other dialog or part of the app. + // For DownArrow, UpArrow and Enter - Make sure that the current active element is an ancestor - this is to prevent us from handling events when the focus is + // on some other dialog or part of the app. + // For Escape - the focused element is the div.notebook-preview or textarea.inputarea of the cell, so we need to make sure that it is a descendant of the current active cell + // on the current active editor. + const activeCellElement = this.container.nativeElement.querySelector(`.editor-group-container.active .notebook-cell.active`); if (DOM.isAncestor(this.container.nativeElement, document.activeElement) && this.isActive() && this.model.activeCell) { const event = new StandardKeyboardEvent(e); if (!this.model.activeCell?.isEditMode) { @@ -156,22 +159,23 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe } this.selectCell(this.cells[--index]); this.scrollToActiveCell(); - } else if (event.keyCode === KeyCode.Escape) { - // unselects active cell and removes the focus from code cells - this.unselectActiveCell(); - (document.activeElement as HTMLElement).blur(); } else if (event.keyCode === KeyCode.Enter) { // prevents adding a newline to the cell source e.preventDefault(); - // show edit toolbar - this.setActiveCellEditActionMode(true); this.toggleEditMode(); } - } else if (event.keyCode === KeyCode.Escape) { + else if (event.keyCode === KeyCode.Escape) { + // unselects active cell and removes the focus from code cells + this.unselectActiveCell(); + (document.activeElement as HTMLElement).blur(); + } + } + } else if (DOM.isAncestor(document.activeElement, activeCellElement) && this.isActive() && this.model.activeCell) { + const event = new StandardKeyboardEvent(e); + if (event.keyCode === KeyCode.Escape) { // first time hitting escape removes the cursor from code cell and changes toolbar in text cells and changes edit mode to false this.toggleEditMode(); - this.setActiveCellEditActionMode(false); } } })); @@ -279,10 +283,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe toolbarEl.style.borderBottomColor = theme.getColor(themeColors.SIDE_BAR_BACKGROUND, true).toString(); } - public selectCell(cell: ICellModel, event?: Event) { - if (event) { - event.stopPropagation(); - } + public selectCell(cell: ICellModel) { if (!this.model.activeCell || this.model.activeCell.id !== cell.id) { this.model.updateActiveCell(cell); this.detectChanges(); @@ -304,6 +305,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe selectedCell = this.codeCells.find(c => c.cellModel.id === this.activeCellId); } selectedCell.toggleEditMode(); + this.setActiveCellEditActionMode(selectedCell.cellModel.isEditMode); } //Saves scrollTop value on scroll change @@ -312,6 +314,21 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe this.model.onScroll.fire(); } + public clickOffCell(event?: MouseEvent) { + event?.stopPropagation(); + this.unselectActiveCell(); + } + + public clickOnCell(cell: ICellModel, event?: MouseEvent) { + event?.stopPropagation(); + if (!this.model.activeCell || this.model.activeCell.id !== cell.id) { + this.selectCell(cell); + if (cell.cellType === CellTypes.Code) { + cell.isEditMode = true; + } + } + } + public unselectActiveCell() { this.model.updateActiveCell(undefined); this.detectChanges(); diff --git a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts index f934d676a4..1be94ee92a 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts @@ -547,7 +547,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return undefined; } let cell = this.createCell(cellType); - return this.insertCell(cell, index); + return this.insertCell(cell, index, true); } public splitCell(cellType: CellType, notebookService: INotebookService, index?: number, addToUndoStack: boolean = true): ICellModel | undefined { @@ -662,9 +662,7 @@ export class NotebookModel extends Disposable implements INotebookModel { this.undoService.pushElement(new SplitCellEdit(this, splitCells)); } //make new cell Active - this.updateActiveCell(activeCell); - activeCell.isEditMode = true; - + this.updateActiveCell(activeCell, true); this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, cells: [activeCell], @@ -686,9 +684,8 @@ export class NotebookModel extends Disposable implements INotebookModel { for (let i = 1; i < cells.length; i++) { firstCell.source = cells[i].prefix ? [...firstCell.source, ...cells[i].prefix, ...cells[i].cell.source] : [...firstCell.source, ...cells[i].cell.source]; } - firstCell.isEditMode = true; // Set newly created cell as active cell - this.updateActiveCell(firstCell); + this.updateActiveCell(firstCell, true); this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, cells: [firstCell], @@ -701,8 +698,7 @@ export class NotebookModel extends Disposable implements INotebookModel { public splitCells(cells: SplitCell[], firstCellOriginalSource: string | string[]): void { cells[0].cell.source = firstCellOriginalSource; - cells[0].cell.isEditMode = true; - this.updateActiveCell(cells[0].cell); + this.updateActiveCell(cells[0].cell, true); this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, cells: [cells[0].cell], @@ -724,11 +720,10 @@ export class NotebookModel extends Disposable implements INotebookModel { this._cells.push(cell); index = undefined; } - cell.isEditMode = true; if (addToUndoStack) { // Only make cell active when inserting the cell. If we update the active cell when undoing/redoing, the user would have to deselect the cell first // and to undo multiple times. - this.updateActiveCell(cell); + this.updateActiveCell(cell, true); this.undoService.pushElement(new AddCellEdit(this, cell, index)); } @@ -804,13 +799,15 @@ export class NotebookModel extends Disposable implements INotebookModel { }); } - public updateActiveCell(cell?: ICellModel): void { + public updateActiveCell(cell?: ICellModel, isEditMode: boolean = false): void { if (this._activeCell) { this._activeCell.active = false; + this._activeCell.isEditMode = false; } this._activeCell = cell; if (this._activeCell) { this._activeCell.active = true; + this._activeCell.isEditMode = isEditMode; } this._onActiveCellChanged.fire(cell); } diff --git a/test/smoke/src/sql/areas/notebook/notebook.test.ts b/test/smoke/src/sql/areas/notebook/notebook.test.ts index a7761aaa23..5ad1edd297 100644 --- a/test/smoke/src/sql/areas/notebook/notebook.test.ts +++ b/test/smoke/src/sql/areas/notebook/notebook.test.ts @@ -18,7 +18,8 @@ export function setup(opts: minimist.ParsedArgs) { await app.workbench.sqlNotebook.addCellFromPlaceholder('Markdown'); await app.workbench.sqlNotebook.waitForPlaceholderGone(); - await app.code.dispatchKeybinding('escape'); + await app.code.dispatchKeybinding('escape'); // first escape sets the cell in edit mode + await app.code.dispatchKeybinding('escape'); // second escape unselects cell completely await app.workbench.sqlNotebook.waitForDoubleClickToEdit(); await app.workbench.sqlNotebook.doubleClickTextCell(); await app.workbench.sqlNotebook.waitForDoubleClickToEditGone();