Grid scrolling bugs (#4396)

* better maintense of heights in scrollable splitview

* formatting

* fix issue around views resizing messing with render logic

* remove commented code
This commit is contained in:
Anthony Dresser
2019-03-13 11:51:47 -07:00
committed by GitHub
parent 17901fbf3d
commit ace6012c1c
3 changed files with 54 additions and 53 deletions

View File

@@ -140,7 +140,14 @@ export class HeightMap {
let viewItem = this.heightMap[i]; let viewItem = this.heightMap[i];
let delta = viewItem.height - size;
viewItem.height = size; viewItem.height = size;
// update all items after this item
for (let j = i + 1; j < this.heightMap.length; j++) {
this.heightMap[j].top -= delta;
}
} }
protected updateTop(item: string, top: number): void { protected updateTop(item: string, top: number): void {

View File

@@ -63,7 +63,6 @@ interface ISashEvent {
interface IViewItem extends HeightIViewItem { interface IViewItem extends HeightIViewItem {
view: IView; view: IView;
size: number;
container: HTMLElement; container: HTMLElement;
disposable: IDisposable; disposable: IDisposable;
layout(): void; layout(): void;
@@ -189,7 +188,6 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
this.scrollable = new ScrollableElement(this.el, { vertical: options.verticalScrollbarVisibility }); this.scrollable = new ScrollableElement(this.el, { vertical: options.verticalScrollbarVisibility });
debounceEvent(this.scrollable.onScroll, (l, e) => e, types.isNumber(this.options.scrollDebounce) ? this.options.scrollDebounce : 25)(e => { debounceEvent(this.scrollable.onScroll, (l, e) => e, types.isNumber(this.options.scrollDebounce) ? this.options.scrollDebounce : 25)(e => {
this.render(e.scrollTop, e.height); this.render(e.scrollTop, e.height);
this.relayout();
this._onScroll.fire(e.scrollTop); this._onScroll.fire(e.scrollTop);
}); });
let domNode = this.scrollable.getDomNode(); let domNode = this.scrollable.getDomNode();
@@ -249,12 +247,12 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
const onRemove = view.onRemove ? () => view.onRemove() : () => { }; const onRemove = view.onRemove ? () => view.onRemove() : () => { };
const layoutContainer = this.orientation === Orientation.VERTICAL const layoutContainer = this.orientation === Orientation.VERTICAL
? () => item.container.style.height = `${item.size}px` ? () => item.container.style.height = `${item.height}px`
: () => item.container.style.width = `${item.size}px`; : () => item.container.style.width = `${item.height}px`;
const layout = () => { const layout = () => {
layoutContainer(); layoutContainer();
item.view.layout(item.size, this.orientation); item.view.layout(item.height, this.orientation);
}; };
let viewSize: number; let viewSize: number;
@@ -267,7 +265,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
viewSize = view.minimumSize; viewSize = view.minimumSize;
} }
const item: IViewItem = { onAdd, onRemove, view, container, size: viewSize, layout, disposable, height: viewSize, top: 0, width: 0 }; const item: IViewItem = { onAdd, onRemove, view, container, layout, disposable, height: viewSize, top: 0, width: 0 };
this.viewItems.splice(currentIndex, 0, item); this.viewItems.splice(currentIndex, 0, item);
this.onInsertItems(new ArrayIterator([item]), currentIndex > 0 ? this.viewItems[currentIndex - 1].view.id : undefined); this.onInsertItems(new ArrayIterator([item]), currentIndex > 0 ? this.viewItems[currentIndex - 1].view.id : undefined);
@@ -316,14 +314,6 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
if (!types.isArray(sizes) && sizes.type === 'distribute') { if (!types.isArray(sizes) && sizes.type === 'distribute') {
this.distributeViewSizes(); this.distributeViewSizes();
} }
// Re-render the views. Set lastRenderTop and lastRenderHeight to undefined since
// this isn't actually scrolling up or down
let scrollTop = this.lastRenderTop;
let viewHeight = this.lastRenderHeight;
this.lastRenderTop = undefined;
this.lastRenderHeight = undefined;
this.render(scrollTop, viewHeight);
} }
addView(view: IView, size: number | Sizing, index = this.viewItems.length): void { addView(view: IView, size: number | Sizing, index = this.viewItems.length): void {
@@ -352,12 +342,12 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
const onRemove = view.onRemove ? () => view.onRemove() : () => { }; const onRemove = view.onRemove ? () => view.onRemove() : () => { };
const layoutContainer = this.orientation === Orientation.VERTICAL const layoutContainer = this.orientation === Orientation.VERTICAL
? () => item.container.style.height = `${item.size}px` ? () => item.container.style.height = `${item.height}px`
: () => item.container.style.width = `${item.size}px`; : () => item.container.style.width = `${item.height}px`;
const layout = () => { const layout = () => {
layoutContainer(); layoutContainer();
item.view.layout(item.size, this.orientation); item.view.layout(item.height, this.orientation);
}; };
let viewSize: number; let viewSize: number;
@@ -370,7 +360,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
viewSize = view.minimumSize; viewSize = view.minimumSize;
} }
const item: IViewItem = { onAdd, onRemove, view, container, size: viewSize, layout, disposable, height: viewSize, top: 0, width: 0 }; const item: IViewItem = { onAdd, onRemove, view, container, layout, disposable, height: viewSize, top: 0, width: 0 };
this.viewItems.splice(index, 0, item); this.viewItems.splice(index, 0, item);
this.onInsertItems(new ArrayIterator([item]), index > 0 ? this.viewItems[index - 1].view.id : undefined); this.onInsertItems(new ArrayIterator([item]), index > 0 ? this.viewItems[index - 1].view.id : undefined);
@@ -480,7 +470,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
} }
private relayout(lowPriorityIndex?: number, highPriorityIndex?: number): void { private relayout(lowPriorityIndex?: number, highPriorityIndex?: number): void {
const contentSize = this.viewItems.reduce((r, i) => r + i.size, 0); const contentSize = this.viewItems.reduce((r, i) => r + i.height, 0);
this.resize(this.viewItems.length - 1, this.size - contentSize, undefined, lowPriorityIndex, highPriorityIndex); this.resize(this.viewItems.length - 1, this.size - contentSize, undefined, lowPriorityIndex, highPriorityIndex);
this.distributeEmptySpace(); this.distributeEmptySpace();
@@ -504,7 +494,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
} else { } else {
for (let i = 0; i < this.viewItems.length; i++) { for (let i = 0; i < this.viewItems.length; i++) {
const item = this.viewItems[i]; const item = this.viewItems[i];
item.size = clamp(Math.round(this.proportions[i] * size), item.view.minimumSize, item.view.maximumSize); this.updateSize(item.view.id, clamp(Math.round(this.proportions[i] * size), item.view.minimumSize, item.view.maximumSize));
} }
} }
@@ -514,7 +504,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
private saveProportions(): void { private saveProportions(): void {
if (this.contentSize > 0) { if (this.contentSize > 0) {
this.proportions = this.viewItems.map(i => i.size / this.contentSize); this.proportions = this.viewItems.map(i => i.height / this.contentSize);
} }
} }
@@ -528,7 +518,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
]); ]);
const resetSashDragState = (start: number, alt: boolean) => { const resetSashDragState = (start: number, alt: boolean) => {
const sizes = this.viewItems.map(i => i.size); const sizes = this.viewItems.map(i => i.height);
let minDelta = Number.NEGATIVE_INFINITY; let minDelta = Number.NEGATIVE_INFINITY;
let maxDelta = Number.POSITIVE_INFINITY; let maxDelta = Number.POSITIVE_INFINITY;
@@ -544,12 +534,12 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
if (isLastSash) { if (isLastSash) {
const viewItem = this.viewItems[index]; const viewItem = this.viewItems[index];
minDelta = (viewItem.view.minimumSize - viewItem.size) / 2; minDelta = (viewItem.view.minimumSize - viewItem.height) / 2;
maxDelta = (viewItem.view.maximumSize - viewItem.size) / 2; maxDelta = (viewItem.view.maximumSize - viewItem.height) / 2;
} else { } else {
const viewItem = this.viewItems[index + 1]; const viewItem = this.viewItems[index + 1];
minDelta = (viewItem.size - viewItem.view.maximumSize) / 2; minDelta = (viewItem.height - viewItem.view.maximumSize) / 2;
maxDelta = (viewItem.size - viewItem.view.minimumSize) / 2; maxDelta = (viewItem.height - viewItem.view.minimumSize) / 2;
} }
} }
@@ -568,11 +558,11 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
if (alt) { if (alt) {
const isLastSash = index === this.sashItems.length - 1; const isLastSash = index === this.sashItems.length - 1;
const newSizes = this.viewItems.map(i => i.size); const newSizes = this.viewItems.map(i => i.height);
const viewItemIndex = isLastSash ? index : index + 1; const viewItemIndex = isLastSash ? index : index + 1;
const viewItem = this.viewItems[viewItemIndex]; const viewItem = this.viewItems[viewItemIndex];
const newMinDelta = viewItem.size - viewItem.view.maximumSize; const newMinDelta = viewItem.height - viewItem.view.maximumSize;
const newMaxDelta = viewItem.size - viewItem.view.minimumSize; const newMaxDelta = viewItem.height - viewItem.view.minimumSize;
const resizeIndex = isLastSash ? index - 1 : index + 1; const resizeIndex = isLastSash ? index - 1 : index + 1;
this.resize(resizeIndex, -newDelta, newSizes, undefined, undefined, newMinDelta, newMaxDelta); this.resize(resizeIndex, -newDelta, newSizes, undefined, undefined, newMinDelta, newMaxDelta);
@@ -595,23 +585,23 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
return; return;
} }
size = typeof size === 'number' ? size : item.size; size = typeof size === 'number' ? size : item.height;
size = clamp(size, item.view.minimumSize, item.view.maximumSize); size = clamp(size, item.view.minimumSize, item.view.maximumSize);
if (this.inverseAltBehavior && index > 0) { if (this.inverseAltBehavior && index > 0) {
// In this case, we want the view to grow or shrink both sides equally // In this case, we want the view to grow or shrink both sides equally
// so we just resize the "left" side by half and let `resize` do the clamping magic // so we just resize the "left" side by half and let `resize` do the clamping magic
this.resize(index - 1, Math.floor((item.size - size) / 2)); this.resize(index - 1, Math.floor((item.height - size) / 2));
this.distributeEmptySpace(); this.distributeEmptySpace();
this.layoutViews(); this.layoutViews();
} else { } else {
item.size = size;
this.updateSize(item.view.id, size); this.updateSize(item.view.id, size);
let top = item.top + item.size; this.updateSize(item.view.id, size);
let top = item.top + item.height;
for (let i = index + 1; i < this.viewItems.length; i++) { for (let i = index + 1; i < this.viewItems.length; i++) {
let currentItem = this.viewItems[i]; let currentItem = this.viewItems[i];
this.updateTop(currentItem.view.id, top); this.updateTop(currentItem.view.id, top);
top += currentItem.size; top += currentItem.height;
} }
this.relayout(index); this.relayout(index);
} }
@@ -631,12 +621,12 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
const item = this.viewItems[index]; const item = this.viewItems[index];
size = Math.round(size); size = Math.round(size);
size = clamp(size, item.view.minimumSize, item.view.maximumSize); size = clamp(size, item.view.minimumSize, item.view.maximumSize);
let delta = size - item.size; let delta = size - item.height;
if (delta !== 0 && index < this.viewItems.length - 1) { if (delta !== 0 && index < this.viewItems.length - 1) {
const downIndexes = range(index + 1, this.viewItems.length); const downIndexes = range(index + 1, this.viewItems.length);
const collapseDown = downIndexes.reduce((r, i) => r + (this.viewItems[i].size - this.viewItems[i].view.minimumSize), 0); const collapseDown = downIndexes.reduce((r, i) => r + (this.viewItems[i].height - this.viewItems[i].view.minimumSize), 0);
const expandDown = downIndexes.reduce((r, i) => r + (this.viewItems[i].view.maximumSize - this.viewItems[i].size), 0); const expandDown = downIndexes.reduce((r, i) => r + (this.viewItems[i].view.maximumSize - this.viewItems[i].height), 0);
const deltaDown = clamp(delta, -expandDown, collapseDown); const deltaDown = clamp(delta, -expandDown, collapseDown);
this.resize(index, deltaDown); this.resize(index, deltaDown);
@@ -645,8 +635,8 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
if (delta !== 0 && index > 0) { if (delta !== 0 && index > 0) {
const upIndexes = range(index - 1, -1); const upIndexes = range(index - 1, -1);
const collapseUp = upIndexes.reduce((r, i) => r + (this.viewItems[i].size - this.viewItems[i].view.minimumSize), 0); const collapseUp = upIndexes.reduce((r, i) => r + (this.viewItems[i].height - this.viewItems[i].view.minimumSize), 0);
const expandUp = upIndexes.reduce((r, i) => r + (this.viewItems[i].view.maximumSize - this.viewItems[i].size), 0); const expandUp = upIndexes.reduce((r, i) => r + (this.viewItems[i].view.maximumSize - this.viewItems[i].height), 0);
const deltaUp = clamp(-delta, -collapseUp, expandUp); const deltaUp = clamp(-delta, -collapseUp, expandUp);
this.resize(index - 1, deltaUp); this.resize(index - 1, deltaUp);
@@ -671,7 +661,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
return -1; return -1;
} }
return this.viewItems[index].size; return this.viewItems[index].height;
} }
@@ -766,7 +756,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
private resize( private resize(
index: number, index: number,
delta: number, delta: number,
sizes = this.viewItems.map(i => i.size), sizes = this.viewItems.map(i => i.height),
lowPriorityIndex?: number, lowPriorityIndex?: number,
highPriorityIndex?: number, highPriorityIndex?: number,
overloadMinDelta: number = Number.NEGATIVE_INFINITY, overloadMinDelta: number = Number.NEGATIVE_INFINITY,
@@ -810,8 +800,10 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
const viewDelta = size - upSizes[i]; const viewDelta = size - upSizes[i];
deltaUp -= viewDelta; deltaUp -= viewDelta;
item.size = size; this.updateSize(item.view.id, size);
this.dirtyState = true; this.dirtyState = true;
this.lastRenderTop = 0;
this.lastRenderHeight = 0;
} }
for (let i = 0, deltaDown = delta; i < downItems.length; i++) { for (let i = 0, deltaDown = delta; i < downItems.length; i++) {
@@ -820,30 +812,32 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
const viewDelta = size - downSizes[i]; const viewDelta = size - downSizes[i];
deltaDown += viewDelta; deltaDown += viewDelta;
item.size = size; this.updateSize(item.view.id, size);
this.dirtyState = true; this.dirtyState = true;
this.lastRenderTop = 0;
this.lastRenderHeight = 0;
} }
return delta; return delta;
} }
private distributeEmptySpace(): void { private distributeEmptySpace(): void {
let contentSize = this.viewItems.reduce((r, i) => r + i.size, 0); let contentSize = this.viewItems.reduce((r, i) => r + i.height, 0);
let emptyDelta = this.size - contentSize; let emptyDelta = this.size - contentSize;
for (let i = this.viewItems.length - 1; emptyDelta !== 0 && i >= 0; i--) { for (let i = this.viewItems.length - 1; emptyDelta !== 0 && i >= 0; i--) {
const item = this.viewItems[i]; const item = this.viewItems[i];
const size = clamp(item.size + emptyDelta, item.view.minimumSize, item.view.maximumSize); const size = clamp(item.height + emptyDelta, item.view.minimumSize, item.view.maximumSize);
const viewDelta = size - item.size; const viewDelta = size - item.height;
emptyDelta -= viewDelta; emptyDelta -= viewDelta;
item.size = size; this.updateSize(item.view.id, size);
} }
} }
private layoutViews(): void { private layoutViews(): void {
// Save new content size // Save new content size
this.contentSize = this.viewItems.reduce((r, i) => r + i.size, 0); this.contentSize = this.viewItems.reduce((r, i) => r + i.height, 0);
if (this.dirtyState) { if (this.dirtyState) {
for (let i = this.indexAt(this.lastRenderTop); i <= this.indexAfter(this.lastRenderTop + this.lastRenderHeight) - 1; i++) { for (let i = this.indexAt(this.lastRenderTop); i <= this.indexAfter(this.lastRenderTop + this.lastRenderHeight) - 1; i++) {
@@ -865,7 +859,7 @@ export class ScrollableSplitView extends HeightMap implements IDisposable {
let position = 0; let position = 0;
for (let i = 0; i < this.sashItems.length; i++) { for (let i = 0; i < this.sashItems.length; i++) {
position += this.viewItems[i].size; position += this.viewItems[i].height;
if (this.sashItems[i].sash === sash) { if (this.sashItems[i].sash === sash) {
return position; return position;

View File

@@ -35,7 +35,7 @@ import { IThemeService } from 'vs/platform/theme/common/themeService';
import { ViewletPanel, IViewletPanelOptions } from 'vs/workbench/browser/parts/views/panelViewlet'; import { ViewletPanel, IViewletPanelOptions } from 'vs/workbench/browser/parts/views/panelViewlet';
import { isUndefinedOrNull } from 'vs/base/common/types'; import { isUndefinedOrNull } from 'vs/base/common/types';
import { range } from 'vs/base/common/arrays'; import { range } from 'vs/base/common/arrays';
import { Orientation } from 'vs/base/browser/ui/splitview/splitview'; import { Orientation, Sizing } from 'vs/base/browser/ui/splitview/splitview';
import { Disposable, IDisposable, dispose } from 'vs/base/common/lifecycle'; import { Disposable, IDisposable, dispose } from 'vs/base/common/lifecycle';
import { generateUuid } from 'vs/base/common/uuid'; import { generateUuid } from 'vs/base/common/uuid';
import { TPromise } from 'vs/base/common/winjs.base'; import { TPromise } from 'vs/base/common/winjs.base';
@@ -302,7 +302,7 @@ export class GridPanel extends ViewletPanel {
// possible to need a sort? // possible to need a sort?
if (isUndefinedOrNull(this.maximizedGrid)) { if (isUndefinedOrNull(this.maximizedGrid)) {
this.splitView.addViews(tables, tables.map(i => i.minimumSize), this.splitView.length); this.splitView.addViews(tables, Sizing.Distribute, this.splitView.length);
} }
this.tables = this.tables.concat(tables); this.tables = this.tables.concat(tables);
@@ -345,7 +345,7 @@ export class GridPanel extends ViewletPanel {
this.maximizedGrid.state.maximized = false; this.maximizedGrid.state.maximized = false;
this.maximizedGrid = undefined; this.maximizedGrid = undefined;
this.splitView.removeView(0); this.splitView.removeView(0);
this.splitView.addViews(this.tables, this.tables.map(i => i.minimumSize)); this.splitView.addViews(this.tables, Sizing.Distribute);
} }
} }
@@ -716,7 +716,7 @@ class GridTable<T> extends Disposable implements IView {
} }
public get maximumSize(): number { public get maximumSize(): number {
return Math.max(this.maxSize, ACTIONBAR_HEIGHT + BOTTOM_PADDING); return Number.POSITIVE_INFINITY;
} }
private loadData(offset: number, count: number): Thenable<T[]> { private loadData(offset: number, count: number): Thenable<T[]> {