Minor improvements in notebook edit mode handling (#21839)

* rename + remove duplicate calls

* only fire onCellEditModeChanged when changed
This commit is contained in:
Lucy Zhang
2023-02-06 10:42:19 -08:00
committed by GitHub
parent 2767ff819d
commit b3048213e8
8 changed files with 27 additions and 31 deletions

View File

@@ -298,8 +298,8 @@ export class CodeComponent extends CellView implements OnInit, OnChanges {
} }
this._layoutEmitter.fire(); this._layoutEmitter.fire();
})); }));
this._register(this.cellModel.onCellModeChanged((isEditMode) => { this._register(this.cellModel.onCellEditModeChanged((isEditMode) => {
this.onCellModeChanged(isEditMode); this.onCellEditModeChanged(isEditMode);
})); }));
this.layout(); this.layout();
@@ -453,7 +453,7 @@ export class CodeComponent extends CellView implements OnInit, OnChanges {
this._editor.setHeightToScrollHeight(false, isCollapsed); this._editor.setHeightToScrollHeight(false, isCollapsed);
} }
private onCellModeChanged(isEditMode: boolean): void { private onCellEditModeChanged(isEditMode: boolean): void {
if (this.cellModel.id === this._activeCellId || this._activeCellId === '') { if (this.cellModel.id === this._activeCellId || this._activeCellId === '') {
if (isEditMode) { if (isEditMode) {
this._editor.getControl().focus(); this._editor.getControl().focus();

View File

@@ -53,7 +53,7 @@ export class CodeCellComponent extends CellView implements OnInit, OnChanges {
this._register(this.cellModel.onOutputsChanged(() => { this._register(this.cellModel.onOutputsChanged(() => {
this._changeRef.detectChanges(); this._changeRef.detectChanges();
})); }));
this._register(this.cellModel.onCellModeChanged(mode => { this._register(this.cellModel.onCellEditModeChanged(mode => {
if (mode !== this.cellModel.isEditMode) { if (mode !== this.cellModel.isEditMode) {
this._changeRef.detectChanges(); this._changeRef.detectChanges();
} }

View File

@@ -209,11 +209,10 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
this._register(this.cellModel.onOutputsChanged(e => { this._register(this.cellModel.onOutputsChanged(e => {
this.updatePreview(); this.updatePreview();
})); }));
this._register(this.cellModel.onCellModeChanged(mode => { this._register(this.cellModel.onCellEditModeChanged(mode => {
if (mode !== this.isEditMode) { if (mode !== this.isEditMode) {
this.toggleEditMode(mode); this.toggleEditMode(mode);
} }
this._changeRef.detectChanges();
})); }));
this._register(this.cellModel.onCurrentEditModeChanged(editMode => { this._register(this.cellModel.onCurrentEditModeChanged(editMode => {
let markdown: boolean = editMode !== CellEditModes.WYSIWYG; let markdown: boolean = editMode !== CellEditModes.WYSIWYG;
@@ -268,11 +267,6 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
let changedProp = changes[propName]; let changedProp = changes[propName];
this._activeCellId = changedProp.currentValue; this._activeCellId = changedProp.currentValue;
this.toggleUserSelect(this.isActive()); this.toggleUserSelect(this.isActive());
// If the activeCellId is undefined (i.e. in an active cell update), don't unnecessarily set editMode to false;
// it will be set to true in a subsequent call to toggleEditMode()
if (changedProp.previousValue !== undefined) {
this.toggleEditMode(false);
}
break; break;
} }
} }

View File

@@ -374,8 +374,8 @@ export class NotebookEditor extends EditorPane implements IFindNotebookControlle
filters: false filters: false
}; };
this._notebookModel.cells?.forEach(cell => { this._notebookModel.cells?.forEach(cell => {
this._register(cell.onCellModeChanged((state) => { this._register(cell.onCellEditModeChanged((isEditMode) => {
if (state) { if (isEditMode) {
this._onFindStateChange(changeEvent).catch(onUnexpectedError); this._onFindStateChange(changeEvent).catch(onUnexpectedError);
} }
})); }));

View File

@@ -463,7 +463,7 @@ suite('Test class NotebookEditor:', () => {
const notebookModel = <NotebookModelStub>await notebookEditor.getNotebookModel(); const notebookModel = <NotebookModelStub>await notebookEditor.getNotebookModel();
notebookModel['_cells'] = [new CellModel({ cell_type: 'code', source: '' }, { isTrusted: true, notebook: notebookModel })]; notebookModel['_cells'] = [new CellModel({ cell_type: 'code', source: '' }, { isTrusted: true, notebook: notebookModel })];
notebookEditor['registerModelChanges'](); notebookEditor['registerModelChanges']();
notebookModel.cells[0]['_onCellModeChanged'].fire(true); //fire cellModeChanged event on the first sell of our test notebookModel notebookModel.cells[0]['_onCellEditModeChanged'].fire(true); //fire cellEditModeChanged event on the first sell of our test notebookModel
notebookModel.contentChangedEmitter.fire({ changeType: NotebookChangeType.Saved }); notebookModel.contentChangedEmitter.fire({ changeType: NotebookChangeType.Saved });
(<NotebookService>notebookService)['_onNotebookEditorAdd'].fire(<INotebookEditor>{}); (<NotebookService>notebookService)['_onNotebookEditorAdd'].fire(<INotebookEditor>{});
notebookFindModelMock.verify(x => x.find( notebookFindModelMock.verify(x => x.find(

View File

@@ -998,7 +998,7 @@ suite('Cell Model', function (): void {
let createCellModePromise = () => { let createCellModePromise = () => {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
setTimeout((error) => reject(error), 2000); setTimeout((error) => reject(error), 2000);
model.onCellModeChanged(isEditMode => { model.onCellEditModeChanged(isEditMode => {
resolve(isEditMode); resolve(isEditMode);
}); });
}); });

View File

@@ -60,7 +60,7 @@ export class CellModel extends Disposable implements ICellModel {
private _isEditMode: boolean; private _isEditMode: boolean;
private _onOutputsChanged = new Emitter<IOutputChangedEvent>(); private _onOutputsChanged = new Emitter<IOutputChangedEvent>();
private _onTableUpdated = new Emitter<ITableUpdatedEvent>(); private _onTableUpdated = new Emitter<ITableUpdatedEvent>();
private _onCellModeChanged = new Emitter<boolean>(); private _onCellEditModeChanged = new Emitter<boolean>();
private _onExecutionStateChanged = new Emitter<CellExecutionState>(); private _onExecutionStateChanged = new Emitter<CellExecutionState>();
private _onCurrentEditModeChanged = new Emitter<CellEditModes>(); private _onCurrentEditModeChanged = new Emitter<CellEditModes>();
private _isTrusted: boolean; private _isTrusted: boolean;
@@ -149,8 +149,8 @@ export class CellModel extends Disposable implements ICellModel {
return this._onTableUpdated.event; return this._onTableUpdated.event;
} }
public get onCellModeChanged(): Event<boolean> { public get onCellEditModeChanged(): Event<boolean> {
return this._onCellModeChanged.event; return this._onCellEditModeChanged.event;
} }
public set metadata(data: any) { public set metadata(data: any) {
@@ -241,6 +241,7 @@ export class CellModel extends Disposable implements ICellModel {
} }
public set isEditMode(isEditMode: boolean) { public set isEditMode(isEditMode: boolean) {
if (this._isEditMode !== isEditMode) {
this._isEditMode = isEditMode; this._isEditMode = isEditMode;
if (this._isEditMode) { if (this._isEditMode) {
const newEditMode = this._lastEditMode ?? this._defaultTextEditMode; const newEditMode = this._lastEditMode ?? this._defaultTextEditMode;
@@ -252,9 +253,10 @@ export class CellModel extends Disposable implements ICellModel {
this._showMarkdown = false; this._showMarkdown = false;
this._showPreview = true; this._showPreview = true;
} }
this._onCellModeChanged.fire(this._isEditMode); this._onCellEditModeChanged.fire(this._isEditMode);
// Note: this does not require a notebook update as it does not change overall state // Note: this does not require a notebook update as it does not change overall state
} }
}
public get trustedMode(): boolean { public get trustedMode(): boolean {
return this._isTrusted; return this._isTrusted;

View File

@@ -455,7 +455,7 @@ export interface ICellModel {
readonly onLanguageChanged: Event<string>; readonly onLanguageChanged: Event<string>;
readonly onCollapseStateChanged: Event<boolean>; readonly onCollapseStateChanged: Event<boolean>;
readonly onParameterStateChanged: Event<boolean>; readonly onParameterStateChanged: Event<boolean>;
readonly onCellModeChanged: Event<boolean>; readonly onCellEditModeChanged: Event<boolean>;
modelContentChangedEvent: IModelContentChangedEvent; modelContentChangedEvent: IModelContentChangedEvent;
isEditMode: boolean; isEditMode: boolean;
showPreview: boolean; showPreview: boolean;