Fix on click edit mode states (#18321)

* fix click code cell

* modified editmode when updating active cell

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.getControl().focus();
-				this._editor.getContainer().contentEditable = 'true';
+			if (!isEditMode) {
-				this._editor.getControl().focus();
-			} else {
-				this._editor.getContainer().contentEditable = 'false';
 				(document.activeElement as HTMLElement).blur();
 			}
 		}

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<string>;

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

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

@@ -7,10 +7,10 @@
 <div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: column">
 	<div #toolbar class="editor-toolbar actionbar-container" style="flex: 0 0 auto; display: flex; flex-flow: row; width: 100%; align-items: center;">
 	</div>
-	<div #container class="scrollable" style="flex: 1 1 auto; position: relative; outline: none" (click)="unselectActiveCell()" (scroll)="scrollHandler($event)">
+	<div #container class="scrollable" style="flex: 1 1 auto; position: relative; outline: none" (click)="clickOffCell($event)" (scroll)="scrollHandler($event)">
 		<loading-spinner [loading]="isLoading"></loading-spinner>
 		<div *ngFor="let cell of cells">
-			<div id="{{ cell.id }}" class="notebook-cell" (click)="selectCell(cell, $event)" [class.active]="cell.active">
+			<div id="{{ cell.id }}" class="notebook-cell" (click)="clickOnCell(cell, $event)" [class.active]="cell.active">
 				<cell-toolbar-component *ngIf="cell.active" [cellModel]="cell" [model]="model"></cell-toolbar-component>
 				<code-cell-component *ngIf="cell.cellType === 'code'" [cellModel]="cell" [model]="model" [activeCellId]="activeCellId">
 				</code-cell-component>

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
+			// 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. 
+			// 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) {
+	public selectCell(cell: ICellModel) {
-		if (event) {
-			event.stopPropagation();
-		}
 		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();

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);
+				this.updateActiveCell(activeCell, true);
-				activeCell.isEditMode = 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, true);
-		this.updateActiveCell(cells[0].cell);
 		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);
 	}

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();