Maddy/edit mode events cleanup (#17636)

* remove multiple events

* correct preview check

* add test
This commit is contained in:
Maddy
2021-11-10 10:59:29 -08:00
committed by GitHub
parent b6047ad87d
commit 1d3debb897
6 changed files with 52 additions and 33 deletions

View File

@@ -7,7 +7,7 @@ import 'vs/css!./code';
import { OnInit, Component, Input, Inject, ElementRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange, forwardRef, ChangeDetectorRef } from '@angular/core';
import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor';
import { ICellModel, CellExecutionState } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { ICellModel, CellExecutionState, CellEditModes } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { Taskbar } from 'sql/base/browser/ui/taskbar/taskbar';
import { RunCellAction, CellContext } from 'sql/workbench/contrib/notebook/browser/cellViews/codeActions';
import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel';
@@ -262,8 +262,9 @@ export class CodeComponent extends CellView implements OnInit, OnChanges {
this._register(this.cellModel.onCollapseStateChanged(isCollapsed => {
this.onCellCollapse(isCollapsed);
}));
this._register(this.cellModel.onCellPreviewModeChanged((e) => {
if (!e && this._cellModel.cellSourceChanged) {
this._register(this.cellModel.onCurrentEditModeChanged((e) => {
let preview = e !== CellEditModes.MARKDOWN;
if (!preview && this._cellModel.cellSourceChanged) {
this.updateModel();
this._cellModel.cellSourceChanged = false;
}

View File

@@ -215,7 +215,15 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
}
this._changeRef.detectChanges();
}));
this._register(this.cellModel.onCellPreviewModeChanged(preview => {
this._register(this.cellModel.onCurrentEditModeChanged(editMode => {
let markdown: boolean = editMode !== CellEditModes.WYSIWYG;
if (!markdown) {
let editorControl = this.cellEditors.length > 0 ? this.cellEditors[0].getEditor().getControl() : undefined;
if (editorControl) {
let selection = editorControl.getSelection();
this.cellModel.markdownCursorPosition = selection?.getPosition();
}
}
// On preview mode change, get the cursor position (get the position only when the selection node is a text node)
if (window.getSelection() && window.getSelection().focusNode?.nodeName === '#text' && window.getSelection().getRangeAt(0)) {
let selection = window.getSelection().getRangeAt(0);
@@ -247,17 +255,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
this.cellModel.richTextCursorPosition = cursorPosition;
}
}
this.previewMode = preview;
this.focusIfPreviewMode();
}));
this._register(this.cellModel.onCellMarkdownModeChanged(markdown => {
if (!markdown) {
let editorControl = this.cellEditors.length > 0 ? this.cellEditors[0].getEditor().getControl() : undefined;
if (editorControl) {
let selection = editorControl.getSelection();
this.cellModel.markdownCursorPosition = selection?.getPosition();
}
}
this.previewMode = editMode !== CellEditModes.MARKDOWN;
this.markdownMode = markdown;
this.focusIfPreviewMode();
}));

View File

@@ -27,7 +27,7 @@ import { NotebookFindNextAction, NotebookFindPreviousAction } from 'sql/workbenc
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { IWorkbenchThemeService } from 'vs/workbench/services/themes/common/workbenchThemeService';
import { INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { CellEditModes, INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { INotebookFindModel } from 'sql/workbench/contrib/notebook/browser/models/notebookFindModel';
import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
import { IModelDecorationsChangeAccessor, IModelDeltaDecoration } from 'vs/editor/common/model';
@@ -380,8 +380,8 @@ export class NotebookEditor extends EditorPane implements IFindNotebookControlle
this._onFindStateChange(changeEvent).catch(onUnexpectedError);
}
}));
this._register(cell.onCellMarkdownModeChanged(e => {
if (e) {
this._register(cell.onCurrentEditModeChanged(editMode => {
if (editMode !== CellEditModes.WYSIWYG) {
this._onFindStateChange(changeEvent).catch(onUnexpectedError);
}
}));

View File

@@ -13,7 +13,7 @@ import { CellTypes } from 'sql/workbench/services/notebook/common/contracts';
import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory';
import { NotebookModelStub, ClientSessionStub, KernelStub, FutureStub } from 'sql/workbench/contrib/notebook/test/stubs';
import { EmptyFuture } from 'sql/workbench/contrib/notebook/test/emptySessionClasses';
import { ICellModel, ICellModelOptions, IClientSession, INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { CellEditModes, ICellModel, ICellModelOptions, IClientSession, INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
import { Deferred } from 'sql/base/common/promise';
import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
@@ -1233,4 +1233,38 @@ suite('Cell Model', function (): void {
cellModel.addAttachment('image/png', imageFilebase64Value, 'test.png');
assert.deepStrictEqual(cellModel.attachments, attachments, 'addAttachment should not add duplicate images');
});
test('cell should fire onCurrentEditModeChanged on edit', async function () {
let notebookModel = new NotebookModelStub({
name: '',
version: '',
mimetype: ''
});
let contents: nb.ICellContents = {
cell_type: CellTypes.Markdown,
source: '',
metadata: {}
};
let cellModel = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
let editModeChangePromise = () => {
return new Promise((resolve, reject) => {
setTimeout(() => reject(), 2000);
cellModel.onCurrentEditModeChanged(editMode => {
resolve(editMode);
});
});
};
assert(!cellModel.isEditMode);
let editModePromise = editModeChangePromise();
cellModel.isEditMode = true;
let editMode = await editModePromise;
assert(editMode);
assert.strictEqual(editMode, CellEditModes.WYSIWYG, 'Default edit mode should be WYSIWYG.');
});
});

View File

@@ -73,8 +73,6 @@ export class CellModel extends Disposable implements ICellModel {
private _isCollapsed: boolean;
private _onCollapseStateChanged = new Emitter<boolean>();
private _modelContentChangedEvent: IModelContentChangedEvent;
private _onCellPreviewChanged = new Emitter<boolean>();
private _onCellMarkdownChanged = new Emitter<boolean>();
private _isCommandExecutionSettingEnabled: boolean = false;
private _showPreview: boolean = true;
private _showMarkdown: boolean = false;
@@ -422,7 +420,6 @@ export class CellModel extends Disposable implements ICellModel {
public set showPreview(val: boolean) {
this._showPreview = val;
this._onCellPreviewChanged.fire(this._showPreview);
this.doModeUpdates();
}
@@ -432,7 +429,6 @@ export class CellModel extends Disposable implements ICellModel {
public set showMarkdown(val: boolean) {
this._showMarkdown = val;
this._onCellMarkdownChanged.fire(this._showMarkdown);
this.doModeUpdates();
}
@@ -454,14 +450,6 @@ export class CellModel extends Disposable implements ICellModel {
this._cellSourceChanged = val;
}
public get onCellPreviewModeChanged(): Event<boolean> {
return this._onCellPreviewChanged.event;
}
public get onCellMarkdownModeChanged(): Event<boolean> {
return this._onCellMarkdownChanged.event;
}
public get onParameterStateChanged(): Event<boolean> {
return this._onParameterStateChanged.event;
}

View File

@@ -534,8 +534,6 @@ export interface ICellModel {
showPreview: boolean;
showMarkdown: boolean;
defaultTextEditMode: string;
readonly onCellPreviewModeChanged: Event<boolean>;
readonly onCellMarkdownModeChanged: Event<boolean>;
sendChangeToNotebook(change: NotebookChangeType): void;
cellSourceChanged: boolean;
readonly savedConnectionName: string | undefined;