From 2043283a650dd7b3f9789d63770e9c5dad558fc0 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 29 Mar 2023 14:21:47 -0700 Subject: [PATCH 1/5] List View top padding. --- src/vs/base/browser/ui/list/listView.ts | 24 +++++- src/vs/base/browser/ui/list/listWidget.ts | 1 + src/vs/base/browser/ui/list/rangeMap.ts | 25 +++++- .../test/browser/ui/list/rangeMap.test.ts | 80 +++++++++++++++++++ .../notebook/browser/notebookEditorWidget.ts | 19 ++--- 5 files changed, 130 insertions(+), 19 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index abb5957922f..eaf35198f7d 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -62,6 +62,7 @@ export interface IListViewOptionsUpdate { readonly scrollByPage?: boolean; readonly mouseWheelScrollSensitivity?: number; readonly fastScrollSensitivity?: number; + readonly topPadding?: number; } export interface IListViewOptions extends IListViewOptionsUpdate { @@ -278,6 +279,7 @@ export class ListView implements IListView { private items: IItem[]; private itemId: number; private rangeMap: RangeMap; + private topPadding: number; private cache: RowCache; private renderers = new Map>(); private lastRenderTop: number; @@ -359,7 +361,8 @@ export class ListView implements IListView { this.items = []; this.itemId = 0; - this.rangeMap = new RangeMap(); + this.topPadding = options.topPadding ?? 0; + this.rangeMap = new RangeMap(this.topPadding); for (const renderer of renderers) { this.renderers.set(renderer.templateId, renderer); @@ -466,6 +469,23 @@ export class ListView implements IListView { if (scrollableOptions) { this.scrollableElement.updateOptions(scrollableOptions); } + + if (options.topPadding !== undefined && options.topPadding !== this.topPadding) { + // trigger a rerender + this.topPadding = options.topPadding; + const lastRenderRange = this.getRenderRange(this.lastRenderTop, this.lastRenderHeight); + const offset = options.topPadding - this.rangeMap.topPadding; + this.rangeMap.topPadding = options.topPadding; + + this.render(lastRenderRange, Math.max(0, this.lastRenderTop + offset), this.lastRenderHeight, undefined, undefined, true); + this.setScrollTop(this.lastRenderTop); + + this.eventuallyUpdateScrollDimensions(); + + if (this.supportDynamicHeights) { + this._rerender(this.lastRenderTop, this.lastRenderHeight); + } + } } delegateScrollFromMouseWheelEvent(browserEvent: IMouseWheelEvent) { @@ -597,7 +617,7 @@ export class ListView implements IListView { // TODO@joao: improve this optimization to catch even more cases if (start === 0 && deleteCount >= this.items.length) { - this.rangeMap = new RangeMap(); + this.rangeMap = new RangeMap(this.topPadding); this.rangeMap.splice(0, 0, inserted); deleted = this.items; this.items = inserted; diff --git a/src/vs/base/browser/ui/list/listWidget.ts b/src/vs/base/browser/ui/list/listWidget.ts index c8ff425cbb9..59c49687c15 100644 --- a/src/vs/base/browser/ui/list/listWidget.ts +++ b/src/vs/base/browser/ui/list/listWidget.ts @@ -972,6 +972,7 @@ export interface IListOptions extends IListOptionsUpdate { readonly scrollableElementChangeOptions?: ScrollableElementChangeOptions; readonly alwaysConsumeMouseWheel?: boolean; readonly initialSize?: Dimension; + readonly topPadding?: number; } export interface IListStyles { diff --git a/src/vs/base/browser/ui/list/rangeMap.ts b/src/vs/base/browser/ui/list/rangeMap.ts index 5656a3b32de..bd77b9a24b2 100644 --- a/src/vs/base/browser/ui/list/rangeMap.ts +++ b/src/vs/base/browser/ui/list/rangeMap.ts @@ -91,6 +91,21 @@ export class RangeMap { private groups: IRangedGroup[] = []; private _size = 0; + private _topPadding = 0; + + get topPadding() { + return this._topPadding; + } + + set topPadding(topPadding: number) { + this._topPadding = topPadding; + this._size = this._topPadding + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); + } + + constructor(topPadding?: number) { + this._topPadding = topPadding || 0; + this._size = this._topPadding; + } splice(index: number, deleteCount: number, items: IItem[] = []): void { const diff = items.length - deleteCount; @@ -104,7 +119,7 @@ export class RangeMap { })); this.groups = concat(before, middle, after); - this._size = this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); + this._size = this._topPadding + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); } /** @@ -135,8 +150,12 @@ export class RangeMap { return -1; } + if (position < this._topPadding) { + return 0; + } + let index = 0; - let size = 0; + let size = this._topPadding; for (const group of this.groups) { const count = group.range.end - group.range.start; @@ -177,7 +196,7 @@ export class RangeMap { const newCount = count + groupCount; if (index < newCount) { - return position + ((index - count) * group.size); + return this._topPadding + position + ((index - count) * group.size); } position += groupCount * group.size; diff --git a/src/vs/base/test/browser/ui/list/rangeMap.test.ts b/src/vs/base/test/browser/ui/list/rangeMap.test.ts index 3b451445a63..518b250a943 100644 --- a/src/vs/base/test/browser/ui/list/rangeMap.test.ts +++ b/src/vs/base/test/browser/ui/list/rangeMap.test.ts @@ -343,3 +343,83 @@ suite('RangeMap', () => { }); }); }); + +suite('RangeMap with top padding', () => { + let rangeMap: RangeMap; + + setup(() => { + rangeMap = new RangeMap(10); + }); + + test('empty', () => { + assert.strictEqual(rangeMap.size, 10); + assert.strictEqual(rangeMap.count, 0); + }); + + const one = { size: 1 }; + const five = { size: 5 }; + const ten = { size: 10 }; + + test('length & count', () => { + rangeMap.splice(0, 0, [one]); + assert.strictEqual(rangeMap.size, 11); + assert.strictEqual(rangeMap.count, 1); + }); + + test('length & count #2', () => { + rangeMap.splice(0, 0, [one, one, one, one, one]); + assert.strictEqual(rangeMap.size, 15); + assert.strictEqual(rangeMap.count, 5); + }); + + test('length & count #3', () => { + rangeMap.splice(0, 0, [five]); + assert.strictEqual(rangeMap.size, 15); + assert.strictEqual(rangeMap.count, 1); + }); + + test('length & count #4', () => { + rangeMap.splice(0, 0, [five, five, five, five, five]); + assert.strictEqual(rangeMap.size, 35); + assert.strictEqual(rangeMap.count, 5); + }); + + test('insert', () => { + rangeMap.splice(0, 0, [five, five, five, five, five]); + assert.strictEqual(rangeMap.size, 35); + assert.strictEqual(rangeMap.count, 5); + + rangeMap.splice(0, 0, [five, five, five, five, five]); + assert.strictEqual(rangeMap.size, 60); + assert.strictEqual(rangeMap.count, 10); + + rangeMap.splice(5, 0, [ten, ten]); + assert.strictEqual(rangeMap.size, 80); + assert.strictEqual(rangeMap.count, 12); + + rangeMap.splice(12, 0, [{ size: 200 }]); + assert.strictEqual(rangeMap.size, 280); + assert.strictEqual(rangeMap.count, 13); + }); + + suite('indexAt, positionAt', () => { + test('empty', () => { + assert.strictEqual(rangeMap.indexAt(0), 0); + assert.strictEqual(rangeMap.indexAt(10), 0); + assert.strictEqual(rangeMap.indexAt(-1), -1); + assert.strictEqual(rangeMap.positionAt(0), -1); + assert.strictEqual(rangeMap.positionAt(10), -1); + assert.strictEqual(rangeMap.positionAt(-1), -1); + }); + + test('simple', () => { + rangeMap.splice(0, 0, [one]); + assert.strictEqual(rangeMap.indexAt(0), 0); + assert.strictEqual(rangeMap.indexAt(1), 0); + assert.strictEqual(rangeMap.indexAt(10), 0); + assert.strictEqual(rangeMap.indexAt(11), 1); + assert.strictEqual(rangeMap.positionAt(0), 10); + assert.strictEqual(rangeMap.positionAt(1), -1); + }); + }); +}); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index f69d5876a14..4ff0b3fe0d6 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -749,15 +749,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD }`); } - // top insert toolbar - const topInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType); - styleSheets.push(`.notebookOverlay .cell-list-top-cell-toolbar-container { top: -${topInsertToolbarHeight - 3}px }`); - styleSheets.push(`.notebookOverlay > .cell-list-container > .monaco-list > .monaco-scrollable-element, - .notebookOverlay > .cell-list-container > .notebook-gutter > .monaco-list > .monaco-scrollable-element { - padding-top: ${topInsertToolbarHeight}px !important; - box-sizing: border-box; - }`); - styleSheets.push(`.notebookOverlay .cell-list-container > .monaco-list > .monaco-scrollable-element > .monaco-list-rows > .code-cell-row div.cell.code { margin-left: ${codeCellLeftMargin + cellRunGutter}px; }`); styleSheets.push(`.notebookOverlay .cell-list-container > .monaco-list > .monaco-scrollable-element > .monaco-list-rows > .monaco-list-row div.cell { margin-right: ${cellRightMargin}px; }`); styleSheets.push(`.notebookOverlay .cell-list-container > .monaco-list > .monaco-scrollable-element > .monaco-list-rows > .monaco-list-row > .cell-inner-container { padding-top: ${cellTopMargin}px; }`); @@ -869,6 +860,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD additionalScrollHeight: 0, transformOptimization: false, //(isMacintosh && isNative) || getTitleBarStyle(this.configurationService, this.environmentService) === 'native', initialSize: this._dimension, + topPadding: this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType), styleController: (_suffix: string) => { return this._list; }, overrideStyles: { listBackground: notebookEditorBackground, @@ -1462,8 +1454,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD })); if (this._dimension) { - const topInserToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType); - this._list.layout(this._dimension.height - topInserToolbarHeight, this._dimension.width); + this._list.layout(this._dimension.height, this._dimension.width); } else { this._list.layout(); } @@ -1757,15 +1748,15 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD DOM.size(this._body, dimension.width, newBodyHeight); const topInserToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType); - const newCellListHeight = Math.max(newBodyHeight - topInserToolbarHeight, 0); + const newCellListHeight = newBodyHeight; if (this._list.getRenderHeight() < newCellListHeight) { // the new dimension is larger than the list viewport, update its additional height first, otherwise the list view will move down a bit (as the `scrollBottom` will move down) - this._list.updateOptions({ additionalScrollHeight: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : topInserToolbarHeight }); + this._list.updateOptions({ additionalScrollHeight: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : 0, topPadding: topInserToolbarHeight }); this._list.layout(newCellListHeight, dimension.width); } else { // the new dimension is smaller than the list viewport, if we update the additional height, the `scrollBottom` will move up, which moves the whole list view upwards a bit. So we run a layout first. this._list.layout(newCellListHeight, dimension.width); - this._list.updateOptions({ additionalScrollHeight: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : topInserToolbarHeight }); + this._list.updateOptions({ additionalScrollHeight: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : 0, topPadding: topInserToolbarHeight }); } this._overlayContainer.style.visibility = 'visible'; From a2daa8d69e1925de3a65e3e2c6549d97d66787b1 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 10 Jul 2023 14:54:41 -0700 Subject: [PATCH 2/5] paddingTop/Bottom --- src/vs/base/browser/ui/list/listPaging.ts | 2 +- src/vs/base/browser/ui/list/listView.ts | 30 +++++++++---------- src/vs/base/browser/ui/list/listWidget.ts | 4 +-- src/vs/base/browser/ui/list/rangeMap.ts | 24 +++++++-------- .../browser/diff/notebookDiffEditor.ts | 2 +- .../notebook/browser/notebookEditorWidget.ts | 8 ++--- .../test/browser/notebookCellList.test.ts | 4 +-- .../terminal/browser/terminalTabsList.ts | 2 +- 8 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/vs/base/browser/ui/list/listPaging.ts b/src/vs/base/browser/ui/list/listPaging.ts index 3701207d187..a3d2eb16cfc 100644 --- a/src/vs/base/browser/ui/list/listPaging.ts +++ b/src/vs/base/browser/ui/list/listPaging.ts @@ -108,7 +108,7 @@ export interface IPagedListOptions { readonly mouseSupport?: boolean; readonly horizontalScrolling?: boolean; readonly scrollByPage?: boolean; - readonly additionalScrollHeight?: number; + readonly paddingBottom?: number; } function fromPagedListOptions(modelProvider: () => IPagedModel, options: IPagedListOptions): IListOptions { diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index d3ff8d806bd..312b1ec6fb5 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -56,13 +56,13 @@ export interface IListViewAccessibilityProvider { } export interface IListViewOptionsUpdate { - readonly additionalScrollHeight?: number; readonly smoothScrolling?: boolean; readonly horizontalScrolling?: boolean; readonly scrollByPage?: boolean; readonly mouseWheelScrollSensitivity?: number; readonly fastScrollSensitivity?: number; - readonly topPadding?: number; + readonly paddingTop?: number; + readonly paddingBottom?: number; } export interface IListViewOptions extends IListViewOptionsUpdate { @@ -281,7 +281,7 @@ export class ListView implements IListView { private items: IItem[]; private itemId: number; private rangeMap: RangeMap; - private topPadding: number; + private paddingTop: number; private cache: RowCache; private renderers = new Map>(); private lastRenderTop: number; @@ -300,7 +300,7 @@ export class ListView implements IListView { private setRowLineHeight: boolean; private setRowHeight: boolean; private supportDynamicHeights: boolean; - private additionalScrollHeight: number; + private paddingBottom: number; private accessibilityProvider: ListViewAccessibilityProvider; private scrollWidth: number | undefined; @@ -366,8 +366,8 @@ export class ListView implements IListView { this.items = []; this.itemId = 0; - this.topPadding = options.topPadding ?? 0; - this.rangeMap = new RangeMap(this.topPadding); + this.paddingTop = options.paddingTop ?? 0; + this.rangeMap = new RangeMap(this.paddingTop); for (const renderer of renderers) { this.renderers.set(renderer.templateId, renderer); @@ -389,7 +389,7 @@ export class ListView implements IListView { this._horizontalScrolling = options.horizontalScrolling ?? DefaultOptions.horizontalScrolling; this.domNode.classList.toggle('horizontal-scrolling', this._horizontalScrolling); - this.additionalScrollHeight = typeof options.additionalScrollHeight === 'undefined' ? 0 : options.additionalScrollHeight; + this.paddingBottom = typeof options.paddingBottom === 'undefined' ? 0 : options.paddingBottom; this.accessibilityProvider = new ListViewAccessibilityProvider(options.accessibilityProvider); @@ -444,8 +444,8 @@ export class ListView implements IListView { } updateOptions(options: IListViewOptionsUpdate) { - if (options.additionalScrollHeight !== undefined) { - this.additionalScrollHeight = options.additionalScrollHeight; + if (options.paddingBottom !== undefined) { + this.paddingBottom = options.paddingBottom; this.scrollableElement.setScrollDimensions({ scrollHeight: this.scrollHeight }); } @@ -475,12 +475,12 @@ export class ListView implements IListView { this.scrollableElement.updateOptions(scrollableOptions); } - if (options.topPadding !== undefined && options.topPadding !== this.topPadding) { + if (options.paddingTop !== undefined && options.paddingTop !== this.paddingTop) { // trigger a rerender - this.topPadding = options.topPadding; + this.paddingTop = options.paddingTop; const lastRenderRange = this.getRenderRange(this.lastRenderTop, this.lastRenderHeight); - const offset = options.topPadding - this.rangeMap.topPadding; - this.rangeMap.topPadding = options.topPadding; + const offset = options.paddingTop - this.rangeMap.paddingTop; + this.rangeMap.paddingTop = options.paddingTop; this.render(lastRenderRange, Math.max(0, this.lastRenderTop + offset), this.lastRenderHeight, undefined, undefined, true); this.setScrollTop(this.lastRenderTop); @@ -622,7 +622,7 @@ export class ListView implements IListView { // TODO@joao: improve this optimization to catch even more cases if (start === 0 && deleteCount >= this.items.length) { - this.rangeMap = new RangeMap(this.topPadding); + this.rangeMap = new RangeMap(this.paddingTop); this.rangeMap.splice(0, 0, inserted); deleted = this.items; this.items = inserted; @@ -1037,7 +1037,7 @@ export class ListView implements IListView { } get scrollHeight(): number { - return this._scrollHeight + (this.horizontalScrolling ? 10 : 0) + this.additionalScrollHeight; + return this._scrollHeight + (this.horizontalScrolling ? 10 : 0) + this.paddingBottom; } // Events diff --git a/src/vs/base/browser/ui/list/listWidget.ts b/src/vs/base/browser/ui/list/listWidget.ts index 64bd1aa8e0f..75806511d49 100644 --- a/src/vs/base/browser/ui/list/listWidget.ts +++ b/src/vs/base/browser/ui/list/listWidget.ts @@ -991,13 +991,13 @@ export interface IListOptions extends IListOptionsUpdate { readonly mouseSupport?: boolean; readonly horizontalScrolling?: boolean; readonly scrollByPage?: boolean; - readonly additionalScrollHeight?: number; + readonly paddingBottom?: number; readonly transformOptimization?: boolean; readonly smoothScrolling?: boolean; readonly scrollableElementChangeOptions?: ScrollableElementChangeOptions; readonly alwaysConsumeMouseWheel?: boolean; readonly initialSize?: Dimension; - readonly topPadding?: number; + readonly paddingTop?: number; } export interface IListStyles { diff --git a/src/vs/base/browser/ui/list/rangeMap.ts b/src/vs/base/browser/ui/list/rangeMap.ts index bd77b9a24b2..5743bcad2c9 100644 --- a/src/vs/base/browser/ui/list/rangeMap.ts +++ b/src/vs/base/browser/ui/list/rangeMap.ts @@ -91,20 +91,20 @@ export class RangeMap { private groups: IRangedGroup[] = []; private _size = 0; - private _topPadding = 0; + private _paddingTop = 0; - get topPadding() { - return this._topPadding; + get paddingTop() { + return this._paddingTop; } - set topPadding(topPadding: number) { - this._topPadding = topPadding; - this._size = this._topPadding + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); + set paddingTop(topPadding: number) { + this._paddingTop = topPadding; + this._size = this._paddingTop + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); } constructor(topPadding?: number) { - this._topPadding = topPadding || 0; - this._size = this._topPadding; + this._paddingTop = topPadding || 0; + this._size = this._paddingTop; } splice(index: number, deleteCount: number, items: IItem[] = []): void { @@ -119,7 +119,7 @@ export class RangeMap { })); this.groups = concat(before, middle, after); - this._size = this._topPadding + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); + this._size = this._paddingTop + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); } /** @@ -150,12 +150,12 @@ export class RangeMap { return -1; } - if (position < this._topPadding) { + if (position < this._paddingTop) { return 0; } let index = 0; - let size = this._topPadding; + let size = this._paddingTop; for (const group of this.groups) { const count = group.range.end - group.range.start; @@ -196,7 +196,7 @@ export class RangeMap { const newCount = count + groupCount; if (index < newCount) { - return this._topPadding + position + ((index - count) * group.size); + return this._paddingTop + position + ((index - count) * group.size); } position += groupCount * group.size; diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index cf7cdc955bc..e21aba1d2ec 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -268,7 +268,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD mouseSupport: true, multipleSelectionSupport: false, typeNavigationEnabled: true, - additionalScrollHeight: 0, + paddingBottom: 0, // transformOptimization: (isMacintosh && isNative) || getTitleBarStyle(this.configurationService, this.environmentService) === 'native', styleController: (_suffix: string) => { return this._list!; }, overrideStyles: { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 32ad43d1b1c..ac28637eebb 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -869,10 +869,10 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD multipleSelectionSupport: true, selectionNavigation: true, typeNavigationEnabled: true, - additionalScrollHeight: 0, + paddingBottom: 0, transformOptimization: false, //(isMacintosh && isNative) || getTitleBarStyle(this.configurationService, this.environmentService) === 'native', initialSize: this._dimension, - topPadding: this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType), + paddingTop: this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType), styleController: (_suffix: string) => { return this._list; }, overrideStyles: { listBackground: notebookEditorBackground, @@ -1766,12 +1766,12 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD const newCellListHeight = newBodyHeight; if (this._list.getRenderHeight() < newCellListHeight) { // the new dimension is larger than the list viewport, update its additional height first, otherwise the list view will move down a bit (as the `scrollBottom` will move down) - this._list.updateOptions({ additionalScrollHeight: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : 0, topPadding: topInserToolbarHeight }); + this._list.updateOptions({ paddingBottom: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : 0, paddingTop: topInserToolbarHeight }); this._list.layout(newCellListHeight, dimension.width); } else { // the new dimension is smaller than the list viewport, if we update the additional height, the `scrollBottom` will move up, which moves the whole list view upwards a bit. So we run a layout first. this._list.layout(newCellListHeight, dimension.width); - this._list.updateOptions({ additionalScrollHeight: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : 0, topPadding: topInserToolbarHeight }); + this._list.updateOptions({ paddingBottom: this._allowScrollBeyondLastLine() ? Math.max(0, (newCellListHeight - 50)) : 0, paddingTop: topInserToolbarHeight }); } this._overlayContainer.style.visibility = 'visible'; diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts index 047e8baa9aa..b8f060ed210 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts @@ -139,8 +139,8 @@ suite('NotebookCellList', () => { }); const cellList = createNotebookCellList(instantiationService); - // without additionalscrollheight, the last 20 px will always be hidden due to `topInsertToolbarHeight` - cellList.updateOptions({ additionalScrollHeight: 100 }); + // without paddingBottom, the last 20 px will always be hidden due to `topInsertToolbarHeight` + cellList.updateOptions({ paddingBottom: 100 }); cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell diff --git a/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts b/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts index 29ff72688d2..4a3fb064e99 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts @@ -97,7 +97,7 @@ export class TerminalTabList extends WorkbenchList { accessibilityProvider: instantiationService.createInstance(TerminalTabsAccessibilityProvider), smoothScrolling: _configurationService.getValue('workbench.list.smoothScrolling'), multipleSelectionSupport: true, - additionalScrollHeight: TerminalTabsListSizes.TabHeight, + paddingBottom: TerminalTabsListSizes.TabHeight, dnd: instantiationService.createInstance(TerminalTabsDragAndDrop), openOnSingleClick: true }, From ef62d8e8fc4dd2b6e9fb0fded25ef8feee308905 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 10 Jul 2023 14:56:24 -0700 Subject: [PATCH 3/5] Remove paddingTop from ListView --- src/vs/base/browser/ui/list/listView.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index 312b1ec6fb5..b73e5811121 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -281,7 +281,6 @@ export class ListView implements IListView { private items: IItem[]; private itemId: number; private rangeMap: RangeMap; - private paddingTop: number; private cache: RowCache; private renderers = new Map>(); private lastRenderTop: number; @@ -366,8 +365,7 @@ export class ListView implements IListView { this.items = []; this.itemId = 0; - this.paddingTop = options.paddingTop ?? 0; - this.rangeMap = new RangeMap(this.paddingTop); + this.rangeMap = new RangeMap(options.paddingTop ?? 0); for (const renderer of renderers) { this.renderers.set(renderer.templateId, renderer); @@ -475,9 +473,8 @@ export class ListView implements IListView { this.scrollableElement.updateOptions(scrollableOptions); } - if (options.paddingTop !== undefined && options.paddingTop !== this.paddingTop) { + if (options.paddingTop !== undefined && options.paddingTop !== this.rangeMap.paddingTop) { // trigger a rerender - this.paddingTop = options.paddingTop; const lastRenderRange = this.getRenderRange(this.lastRenderTop, this.lastRenderHeight); const offset = options.paddingTop - this.rangeMap.paddingTop; this.rangeMap.paddingTop = options.paddingTop; @@ -622,7 +619,7 @@ export class ListView implements IListView { // TODO@joao: improve this optimization to catch even more cases if (start === 0 && deleteCount >= this.items.length) { - this.rangeMap = new RangeMap(this.paddingTop); + this.rangeMap = new RangeMap(this.rangeMap.paddingTop); this.rangeMap.splice(0, 0, inserted); deleted = this.items; this.items = inserted; From 30d566800169c48652b6ca68f66b9d97c8ff0dd6 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 10 Jul 2023 14:58:41 -0700 Subject: [PATCH 4/5] Resolve comments --- src/vs/base/browser/ui/list/rangeMap.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/base/browser/ui/list/rangeMap.ts b/src/vs/base/browser/ui/list/rangeMap.ts index 5743bcad2c9..91f435cd822 100644 --- a/src/vs/base/browser/ui/list/rangeMap.ts +++ b/src/vs/base/browser/ui/list/rangeMap.ts @@ -97,13 +97,13 @@ export class RangeMap { return this._paddingTop; } - set paddingTop(topPadding: number) { - this._paddingTop = topPadding; - this._size = this._paddingTop + this.groups.reduce((t, g) => t + (g.size * (g.range.end - g.range.start)), 0); + set paddingTop(paddingTop: number) { + this._size = this._size + paddingTop - this._paddingTop; + this._paddingTop = paddingTop; } constructor(topPadding?: number) { - this._paddingTop = topPadding || 0; + this._paddingTop = topPadding ?? 0; this._size = this._paddingTop; } From fa823baa3cbd35c01b5c4a7e77d8e41dfe178c8c Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 10 Jul 2023 15:01:33 -0700 Subject: [PATCH 5/5] :lipstick: --- src/vs/base/browser/ui/list/listWidget.ts | 2 +- .../workbench/contrib/notebook/browser/notebookEditorWidget.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/base/browser/ui/list/listWidget.ts b/src/vs/base/browser/ui/list/listWidget.ts index 75806511d49..e8ec5372e18 100644 --- a/src/vs/base/browser/ui/list/listWidget.ts +++ b/src/vs/base/browser/ui/list/listWidget.ts @@ -991,13 +991,13 @@ export interface IListOptions extends IListOptionsUpdate { readonly mouseSupport?: boolean; readonly horizontalScrolling?: boolean; readonly scrollByPage?: boolean; - readonly paddingBottom?: number; readonly transformOptimization?: boolean; readonly smoothScrolling?: boolean; readonly scrollableElementChangeOptions?: ScrollableElementChangeOptions; readonly alwaysConsumeMouseWheel?: boolean; readonly initialSize?: Dimension; readonly paddingTop?: number; + readonly paddingBottom?: number; } export interface IListStyles { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index ac28637eebb..801f7d93f61 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -869,10 +869,10 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD multipleSelectionSupport: true, selectionNavigation: true, typeNavigationEnabled: true, + paddingTop: this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType), paddingBottom: 0, transformOptimization: false, //(isMacintosh && isNative) || getTitleBarStyle(this.configurationService, this.environmentService) === 'native', initialSize: this._dimension, - paddingTop: this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType), styleController: (_suffix: string) => { return this._list; }, overrideStyles: { listBackground: notebookEditorBackground,