From a518c4a529a5d6c118bf6b6828a17575b83e05eb Mon Sep 17 00:00:00 2001 From: Gene Lee Date: Wed, 5 Jun 2019 12:07:31 -0700 Subject: [PATCH] Making Notebook to scroll to output area only when Notebook command is executed (#5893) --- .../notebook/cellViews/output.component.ts | 14 ++--------- .../cellViews/outputArea.component.ts | 12 +++++++++- .../workbench/parts/notebook/models/cell.ts | 23 ++++++++++++------- .../parts/notebook/models/modelInterfaces.ts | 7 +++++- src/sqltest/parts/notebook/model/cell.test.ts | 4 ++-- 5 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/sql/workbench/parts/notebook/cellViews/output.component.ts b/src/sql/workbench/parts/notebook/cellViews/output.component.ts index c5b19280eb..dba650aada 100644 --- a/src/sql/workbench/parts/notebook/cellViews/output.component.ts +++ b/src/sql/workbench/parts/notebook/cellViews/output.component.ts @@ -43,7 +43,7 @@ export class OutputComponent extends AngularDisposable implements OnInit { } ngOnInit() { - this.renderOutput(true); + this.renderOutput(); this._initialized = true; this._register(Event.debounce(this.cellModel.notebookModel.layoutChanged, (l, e) => e, 50, /*leading=*/false) (() => this.renderOutput())); @@ -70,21 +70,11 @@ export class OutputComponent extends AngularDisposable implements OnInit { } } - private renderOutput(focusAndScroll: boolean = false): void { + private renderOutput(): void { let options = outputProcessor.getBundleOptions({ value: this.cellOutput, trusted: this.trustedMode }); options.themeService = this._themeService; // TODO handle safe/unsafe mapping this.createRenderedMimetype(options, this.outputElement.nativeElement); - if (focusAndScroll) { - this.setFocusAndScroll(this.outputElement.nativeElement); - } - } - - private setFocusAndScroll(node: HTMLElement): void { - if (node) { - node.focus(); - node.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); - } } public layout(): void { diff --git a/src/sql/workbench/parts/notebook/cellViews/outputArea.component.ts b/src/sql/workbench/parts/notebook/cellViews/outputArea.component.ts index 5cbe44ce4a..cdb7386115 100644 --- a/src/sql/workbench/parts/notebook/cellViews/outputArea.component.ts +++ b/src/sql/workbench/parts/notebook/cellViews/outputArea.component.ts @@ -33,14 +33,24 @@ export class OutputAreaComponent extends AngularDisposable implements OnInit { this._register(this.themeService.onDidColorThemeChange(this.updateTheme, this)); this.updateTheme(this.themeService.getColorTheme()); if (this.cellModel) { - this._register(this.cellModel.onOutputsChanged(() => { + this._register(this.cellModel.onOutputsChanged(e => { if (!(this._changeRef['destroyed'])) { this._changeRef.detectChanges(); + if (e && e.shouldScroll) { + this.setFocusAndScroll(this.outputArea.nativeElement); + } } })); } } + private setFocusAndScroll(node: HTMLElement): void { + if (node) { + node.focus(); + node.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); + } + } + @Input() set activeCellId(value: string) { this._activeCellId = value; } diff --git a/src/sql/workbench/parts/notebook/models/cell.ts b/src/sql/workbench/parts/notebook/models/cell.ts index 2d6f92f444..c7e55a4b2a 100644 --- a/src/sql/workbench/parts/notebook/models/cell.ts +++ b/src/sql/workbench/parts/notebook/models/cell.ts @@ -12,7 +12,7 @@ import { localize } from 'vs/nls'; import * as notebookUtils from '../notebookUtils'; import { CellTypes, CellType, NotebookChangeType } from 'sql/workbench/parts/notebook/models/contracts'; import { NotebookModel } from 'sql/workbench/parts/notebook/models/notebookModel'; -import { ICellModel, notebookConstants } from 'sql/workbench/parts/notebook/models/modelInterfaces'; +import { ICellModel, notebookConstants, IOutputChangedEvent } from 'sql/workbench/parts/notebook/models/modelInterfaces'; import { ICellModelOptions, IModelFactory, FutureInternal, CellExecutionState } from './modelInterfaces'; import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement'; import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; @@ -20,7 +20,6 @@ import { INotificationService, Severity } from 'vs/platform/notification/common/ import { Schemas } from 'vs/base/common/network'; let modelId = 0; - export class CellModel implements ICellModel { private _cellType: nb.CellType; private _source: string; @@ -28,7 +27,7 @@ export class CellModel implements ICellModel { private _future: FutureInternal; private _outputs: nb.ICellOutput[] = []; private _isEditMode: boolean; - private _onOutputsChanged = new Emitter>(); + private _onOutputsChanged = new Emitter(); private _onCellModeChanged = new Emitter(); private _onExecutionStateChanged = new Emitter(); private _isTrusted: boolean; @@ -62,7 +61,7 @@ export class CellModel implements ICellModel { return other && other.id === this.id; } - public get onOutputsChanged(): Event> { + public get onOutputsChanged(): Event { return this._onOutputsChanged.event; } @@ -91,7 +90,11 @@ export class CellModel implements ICellModel { public set trustedMode(isTrusted: boolean) { if (this._isTrusted !== isTrusted) { this._isTrusted = isTrusted; - this._onOutputsChanged.fire(this._outputs); + let outputEvent: IOutputChangedEvent = { + outputs: this._outputs, + shouldScroll: false + }; + this._onOutputsChanged.fire(outputEvent); } } @@ -317,8 +320,12 @@ export class CellModel implements ICellModel { this.fireOutputsChanged(); } - private fireOutputsChanged(): void { - this._onOutputsChanged.fire(this.outputs); + private fireOutputsChanged(shouldScroll: boolean = false): void { + let outputEvent: IOutputChangedEvent = { + outputs: this.outputs, + shouldScroll: !!shouldScroll + }; + this._onOutputsChanged.fire(outputEvent); this.sendChangeToNotebook(NotebookChangeType.CellOutputUpdated); } @@ -383,7 +390,7 @@ export class CellModel implements ICellModel { // deletes transient node in the serialized JSON delete output['transient']; this._outputs.push(this.rewriteOutputUrls(output)); - this.fireOutputsChanged(); + this.fireOutputsChanged(true); } } diff --git a/src/sql/workbench/parts/notebook/models/modelInterfaces.ts b/src/sql/workbench/parts/notebook/models/modelInterfaces.ts index 3f3177b311..397a73310f 100644 --- a/src/sql/workbench/parts/notebook/models/modelInterfaces.ts +++ b/src/sql/workbench/parts/notebook/models/modelInterfaces.ts @@ -435,6 +435,11 @@ export enum CellExecutionState { Error = 3 } +export interface IOutputChangedEvent { + outputs: ReadonlyArray; + shouldScroll: boolean; +} + export interface ICellModel { cellUri: URI; id: string; @@ -447,7 +452,7 @@ export interface ICellModel { executionCount: number | undefined; readonly future: FutureInternal; readonly outputs: ReadonlyArray; - readonly onOutputsChanged: Event>; + readonly onOutputsChanged: Event; readonly onExecutionStateChange: Event; readonly executionState: CellExecutionState; readonly notebookModel: NotebookModel; diff --git a/src/sqltest/parts/notebook/model/cell.test.ts b/src/sqltest/parts/notebook/model/cell.test.ts index 6197609ce5..07b4f84736 100644 --- a/src/sqltest/parts/notebook/model/cell.test.ts +++ b/src/sqltest/parts/notebook/model/cell.test.ts @@ -162,7 +162,7 @@ suite('Cell Model', function (): void { future.setup(f => f.setReplyHandler(TypeMoq.It.isAny())).callback((handler) => onReply = handler); future.setup(f => f.setIOPubHandler(TypeMoq.It.isAny())).callback((handler) => onIopub = handler); let outputs: ReadonlyArray = undefined; - cell.onOutputsChanged((o => outputs = o)); + cell.onOutputsChanged((o => outputs = o.outputs)); // When I set it on the cell cell.setFuture(future.object); @@ -265,7 +265,7 @@ suite('Cell Model', function (): void { let onIopub: nb.MessageHandler; future.setup(f => f.setIOPubHandler(TypeMoq.It.isAny())).callback((handler) => onIopub = handler); let outputs: ReadonlyArray = undefined; - cell.onOutputsChanged((o => outputs = o)); + cell.onOutputsChanged((o => outputs = o.outputs)); //Set the future cell.setFuture(future.object);