From 4191ef8aa5c5d3369b50de34a7b218d39ac6a12f Mon Sep 17 00:00:00 2001 From: Maddy <12754347+MaddyDev@users.noreply.github.com> Date: Fri, 8 Apr 2022 12:31:30 -0700 Subject: [PATCH] Fix active cell update on tabbing (#18614) * listen on focus_in of toolbar * update styles on focus_in * listen for active cell change on notebook componen * add tabbing order to textcells * remove duplicate listener * clean up * undo * remove visible check from cellToolbar * remove duplicate detectChanges on updateActiveCell * only update active cell if it's already not * add aria-label for accessibility * localize the aria label * refactor * add cellLabel property to CellModel * remove updateActiveCell from code component * regression from merge fix * set edit mode as true when focusing on cell * moce check to model * merge changes correctly * update edit mode if code cell * fixes Co-authored-by: barbaravaldez Co-authored-by: chgagnon --- .../browser/cellViews/code.component.html | 2 +- .../browser/cellViews/code.component.ts | 9 +++++++-- .../browser/cellViews/collapse.component.html | 2 +- .../browser/cellViews/collapse.component.ts | 4 ++++ .../notebook/browser/notebook.component.html | 2 +- .../notebook/browser/notebook.component.ts | 7 +++++-- .../services/notebook/browser/models/cell.ts | 6 ++++++ .../browser/models/modelInterfaces.ts | 1 + .../notebook/browser/models/notebookModel.ts | 20 ++++++++++--------- 9 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.html b/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.html index 3bf746e095..2b50ab0b3a 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.html +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.html @@ -14,7 +14,7 @@
-
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 16bf51ac77..dedc80a9e0 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts @@ -452,8 +452,9 @@ export class CodeComponent extends CellView implements OnInit, OnChanges { private onCellModeChanged(isEditMode: boolean): void { if (this.cellModel.id === this._activeCellId || this._activeCellId === '') { - this._editor.getControl().focus(); - if (!isEditMode) { + if (isEditMode) { + this._editor.getControl().focus(); + } else { (document.activeElement as HTMLElement).blur(); } } @@ -470,6 +471,10 @@ export class CodeComponent extends CellView implements OnInit, OnChanges { .catch(err => onUnexpectedError(err)); } + public onCellLanguageFocus(): void { + this._model.updateActiveCell(this._cellModel); + } + private pickCellLanguage(languages: string[]): Promise { if (languages.length === 0) { languages = [this._cellModel.language]; diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.html b/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.html index 73fcf69d1e..a2ef06ce9c 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.html +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.html @@ -5,5 +5,5 @@ *--------------------------------------------------------------------------------------------*/ -->
- +
diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.ts index 32745c21d0..09e0ea30f0 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/collapse.component.ts @@ -68,6 +68,10 @@ export class CollapseComponent extends CellView implements OnInit, OnChanges { this.cellModel.isCollapsed = !this.cellModel.isCollapsed; } + public onFocus(): void { + this.cellModel.notebookModel.updateActiveCell(this.cellModel); + } + public cellGuid(): string { return this.cellModel.cellGuid; } diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.component.html b/src/sql/workbench/contrib/notebook/browser/notebook.component.html index fed09e7703..83a95de622 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.component.html +++ b/src/sql/workbench/contrib/notebook/browser/notebook.component.html @@ -9,7 +9,7 @@
-
+
diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts index 648a50db76..b660225e9e 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts @@ -297,7 +297,6 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe public selectCell(cell: ICellModel) { if (!this.model.activeCell || this.model.activeCell.id !== cell.id) { this.model.updateActiveCell(cell); - this.detectChanges(); } } @@ -340,7 +339,10 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe public unselectActiveCell() { this.model.updateActiveCell(undefined); - this.detectChanges(); + } + + public updateActiveCell(cell: ICellModel) { + this._model.updateActiveCell(cell); } // Handles double click to edit icon change @@ -446,6 +448,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe this._register(this._model.contentChanged((change) => this.handleContentChanged(change))); this._register(this._model.onProviderIdChange((provider) => this.handleProviderIdChanged(provider))); this._register(this._model.kernelChanged((kernelArgs) => this.handleKernelChanged(kernelArgs))); + this._register(this._model.onActiveCellChanged(() => this.detectChanges())); this._register(this._model.onCellTypeChanged(() => this.detectChanges())); this._register(this._model.layoutChanged(() => this.detectChanges())); this._register(this.model.onScroll.event(() => this._onScroll.fire())); diff --git a/src/sql/workbench/services/notebook/browser/models/cell.ts b/src/sql/workbench/services/notebook/browser/models/cell.ts index 48140bbc6f..fbff21764b 100644 --- a/src/sql/workbench/services/notebook/browser/models/cell.ts +++ b/src/sql/workbench/services/notebook/browser/models/cell.ts @@ -46,6 +46,7 @@ export interface QueryResultId { export class CellModel extends Disposable implements ICellModel { public id: string; + public cellLabel: string; private _cellType: nb.CellType; private _source: string | string[]; @@ -120,6 +121,11 @@ export class CellModel extends Disposable implements ICellModel { } // if the fromJson() method was already called and _cellGuid was previously set, don't generate another UUID unnecessarily this._cellGuid = this._cellGuid || generateUuid(); + if (this._cellType === 'code') { + this.cellLabel = localize('codeCellLabel', "Code Cell {0}", this.id); + } else { + this.cellLabel = localize('mdCellLabel', "Markdown Cell {0}", this.id); + } this.createUri(); this.populatePropertiesFromSettings(); } diff --git a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts index 8c7eddf413..4c2ca7739a 100644 --- a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts +++ b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts @@ -499,6 +499,7 @@ export interface ITableUpdatedEvent { export interface ICellModel { cellUri: URI; id: string; + cellLabel: string; readonly language: string; readonly displayLanguage: string; readonly cellGuid: string; diff --git a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts index 1cf6065e6c..c72bf28daa 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts @@ -802,16 +802,18 @@ export class NotebookModel extends Disposable implements INotebookModel { } public updateActiveCell(cell?: ICellModel, isEditMode: boolean = false): void { - if (this._activeCell) { - this._activeCell.active = false; - this._activeCell.isEditMode = false; + if (this._activeCell !== cell) { + 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); } - this._activeCell = cell; - if (this._activeCell) { - this._activeCell.active = true; - this._activeCell.isEditMode = isEditMode; - } - this._onActiveCellChanged.fire(cell); } public convertCellType(cell: ICellModel, addToUndoStack: boolean = true): void {