From f7abf5a2d54ebeb8949a42403e61f040eb12a540 Mon Sep 17 00:00:00 2001 From: Anthony Dresser Date: Mon, 17 Sep 2018 17:55:52 -0700 Subject: [PATCH] Fix stating for scrolls (#2615) * nearly working * add accounting for the downsides to slickgrid --- .../scrollableSplitview.ts | 18 +++++++++-- src/sql/parts/query/editor/gridPanel.ts | 32 +++++++++++++++---- src/typings/globals/slickgrid/index.d.ts | 1 + 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/sql/base/browser/ui/scrollableSplitview/scrollableSplitview.ts b/src/sql/base/browser/ui/scrollableSplitview/scrollableSplitview.ts index 8c31e385de..bff075b70f 100644 --- a/src/sql/base/browser/ui/scrollableSplitview/scrollableSplitview.ts +++ b/src/sql/base/browser/ui/scrollableSplitview/scrollableSplitview.ts @@ -34,6 +34,8 @@ export interface IView extends HeightIView { readonly onDidChange: Event; render(container: HTMLElement, orientation: Orientation): void; layout(size: number, orientation: Orientation): void; + onAdd?(): void; + onRemove?(): void; } interface ISashEvent { @@ -48,6 +50,8 @@ interface IViewItem extends HeightIViewItem { container: HTMLElement; disposable: IDisposable; layout(): void; + onRemove: () => void; + onAdd: () => void; } interface ISashItem { @@ -159,6 +163,9 @@ export class ScrollableSplitView extends HeightMap implements IDisposable { }); const disposable = combinedDisposable([onChangeDisposable, containerDisposable]); + const onAdd = view.onAdd ? () => view.onAdd() : () => { }; + const onRemove = view.onRemove ? () => view.onRemove() : () => { }; + const layoutContainer = this.orientation === Orientation.VERTICAL ? size => item.container.style.height = `${item.size}px` : size => item.container.style.width = `${item.size}px`; @@ -169,7 +176,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable { }; size = Math.round(size); - const item: IViewItem = { view, container, size, layout, disposable, height: size, top: 0, width: 0 }; + const item: IViewItem = { onRemove, onAdd, view, container, size, layout, disposable, height: size, top: 0, width: 0 }; this.viewItems.splice(viewIndex, 0, item); this.onInsertItems(new ArrayIterator([item]), viewIndex > 0 ? this.viewItems[viewIndex - 1].view.id : undefined); @@ -224,6 +231,9 @@ export class ScrollableSplitView extends HeightMap implements IDisposable { }); const disposable = combinedDisposable([onChangeDisposable, containerDisposable]); + const onAdd = view.onAdd ? () => view.onAdd() : () => { }; + const onRemove = view.onRemove ? () => view.onRemove() : () => { }; + const layoutContainer = this.orientation === Orientation.VERTICAL ? size => item.container.style.height = `${item.size}px` : size => item.container.style.width = `${item.size}px`; @@ -234,7 +244,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable { }; size = Math.round(size); - const item: IViewItem = { view, container, size, layout, disposable, height: size, top: 0, width: 0 }; + const item: IViewItem = { onAdd, onRemove, view, container, size, layout, disposable, height: size, top: 0, width: 0 }; this.viewItems.splice(index, 0, item); this.onInsertItems(new ArrayIterator([item]), index > 0 ? this.viewItems[index - 1].view.id : undefined); @@ -495,6 +505,8 @@ export class ScrollableSplitView extends HeightMap implements IDisposable { } item.layout(); + + item.onAdd(); return true; } @@ -504,6 +516,8 @@ export class ScrollableSplitView extends HeightMap implements IDisposable { } this.el.removeChild(item.container); + + item.onRemove(); return true; } diff --git a/src/sql/parts/query/editor/gridPanel.ts b/src/sql/parts/query/editor/gridPanel.ts index f446b8891b..f3ef4c11ba 100644 --- a/src/sql/parts/query/editor/gridPanel.ts +++ b/src/sql/parts/query/editor/gridPanel.ts @@ -36,7 +36,7 @@ import { $ } from 'vs/base/browser/builder'; import { generateUuid } from 'vs/base/common/uuid'; import { TPromise } from 'vs/base/common/winjs.base'; import { Separator, ActionBar, ActionsOrientation } from 'vs/base/browser/ui/actionbar/actionbar'; -import { Dimension, getContentWidth } from 'vs/base/browser/dom'; +import { Dimension, getContentWidth, isInDOM } from 'vs/base/browser/dom'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; @@ -168,7 +168,12 @@ export class GridPanel extends ViewletPanel { this.queryRunnerDisposables = []; this.runner = runner; this.queryRunnerDisposables.push(this.runner.onResultSet(e => this.onResultSet(e))); - this.queryRunnerDisposables.push(this.runner.onQueryStart(() => this.reset())); + this.queryRunnerDisposables.push(this.runner.onQueryStart(() => { + if (this.state) { + this.state.tableStates = []; + } + this.reset(); + })); } private onResultSet(resultSet: sqlops.ResultSetSummary | sqlops.ResultSetSummary[]) { @@ -235,7 +240,6 @@ export class GridPanel extends ViewletPanel { for (let i = this.splitView.length - 1; i >= 0; i--) { this.splitView.removeView(i); } - dispose(this.tables); this.tables = []; @@ -306,6 +310,8 @@ class GridTable extends Disposable implements IView { private _state: GridTableState; + private scrolled = false; + // this handles if the row count is small, like 4-5 rows private readonly maxSize = ((this.resultSet.rowCount) * ROW_HEIGHT) + HEADER_HEIGHT + ESTIMATED_SCROLL_BAR_HEIGHT; @@ -337,6 +343,11 @@ class GridTable extends Disposable implements IView { }); } + public onRemove() { + // when we are removed slickgrid acts badly so we need to account for that + this.scrolled = false; + } + public render(container: HTMLElement, orientation: Orientation): void { container.appendChild(this.container); } @@ -409,9 +420,13 @@ class GridTable extends Disposable implements IView { } }); - this.table.grid.onScroll.subscribe(e => { - if (this.state) { - this.state.scrollPosition = this.table.grid.getViewport().top; + this.table.grid.onScroll.subscribe((e, data) => { + if (!this.scrolled && this.state.scrollPosition && isInDOM(this.container)) { + this.scrolled = true; + this.table.grid.scrollTo(this.state.scrollPosition); + } + if (this.state && isInDOM(this.container)) { + this.state.scrollPosition = data.scrollTop; } }); @@ -431,7 +446,10 @@ class GridTable extends Disposable implements IView { this.state.onCanBeMaximizedChange(this.rebuildActionBar, this); if (this.state.scrollPosition) { - this.table.grid.scrollRowToTop(this.state.scrollPosition); + // most of the time this won't do anything + this.table.grid.scrollTo(this.state.scrollPosition); + // the problem here is that the scrolling state slickgrid uses + // doesn't work with it offDOM. } if (this.state.selection) { diff --git a/src/typings/globals/slickgrid/index.d.ts b/src/typings/globals/slickgrid/index.d.ts index d36180847c..4f181e38de 100644 --- a/src/typings/globals/slickgrid/index.d.ts +++ b/src/typings/globals/slickgrid/index.d.ts @@ -1233,6 +1233,7 @@ declare namespace Slick { public scrollRowIntoView(row: number, doPaging: boolean): void; public scrollRowToTop(row: number): void; public scrollCellIntoView(row: number, cell: number, doPaging: boolean): void; + public scrollTo(y: number); public getCanvasNode(): HTMLCanvasElement; public focus(): void;