Skip to content

Commit a1a29cf

Browse files
authored
Fix list view npe when inserting whitespaces at top (microsoft#205311)
1 parent 233fd79 commit a1a29cf

File tree

5 files changed

+25
-6
lines changed

5 files changed

+25
-6
lines changed

src/vs/base/browser/ui/list/listView.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,8 @@ export class ListView<T> implements IListView<T> {
295295
protected rangeMap: IRangeMap;
296296
private cache: RowCache<T>;
297297
private renderers = new Map<string, IListRenderer<any /* TODO@joao */, any>>();
298-
private lastRenderTop: number;
299-
private lastRenderHeight: number;
298+
protected lastRenderTop: number;
299+
protected lastRenderHeight: number;
300300
private renderWidth = 0;
301301
private rowsContainer: HTMLElement;
302302
private scrollable: Scrollable;
@@ -1400,7 +1400,7 @@ export class ListView<T> implements IListView<T> {
14001400
return undefined;
14011401
}
14021402

1403-
private getRenderRange(renderTop: number, renderHeight: number): IRange {
1403+
protected getRenderRange(renderTop: number, renderHeight: number): IRange {
14041404
return {
14051405
start: this.rangeMap.indexAt(renderTop),
14061406
end: this.rangeMap.indexAfter(renderTop + renderHeight - 1)

src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
594594

595595
if (modelIndex >= this.hiddenRangesPrefixSum.getTotalSum()) {
596596
// it's already after the last hidden range
597-
return this.hiddenRangesPrefixSum.getTotalSum();
597+
return Math.min(this.length, this.hiddenRangesPrefixSum.getTotalSum());
598598
}
599599

600600
return this.hiddenRangesPrefixSum.getIndexOf(modelIndex).index;

src/vs/workbench/contrib/notebook/browser/view/notebookCellListView.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,14 @@ export class NotebookCellListView<T> extends ListView<T> {
276276
insertWhitespace(afterPosition: number, size: number): string {
277277
const scrollTop = this.scrollTop;
278278
const id = `${++this._lastWhitespaceId}`;
279+
const previousRenderRange = this.getRenderRange(this.lastRenderTop, this.lastRenderHeight);
280+
const elementPosition = this.elementTop(afterPosition);
281+
const aboveScrollTop = scrollTop > elementPosition;
279282
this.notebookRangeMap.insertWhitespace(id, afterPosition, size);
280283

281-
this._rerender(scrollTop, this.renderHeight, false);
284+
const newScrolltop = aboveScrollTop ? scrollTop + size : scrollTop;
285+
this.render(previousRenderRange, newScrolltop, this.lastRenderHeight, undefined, undefined, false);
286+
this._rerender(newScrolltop, this.renderHeight, false);
282287
this.eventuallyUpdateScrollDimensions();
283288

284289
return id;

src/vs/workbench/contrib/notebook/browser/viewParts/notebookViewZones.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ export class NotebookViewZones extends Disposable {
120120
}
121121

122122
private _addZone(zone: INotebookViewZone): string {
123-
const whitespaceId = this.listView.insertWhitespace(zone.afterModelPosition, zone.heightInPx);
123+
const viewPosition = this.coordinator.convertModelIndexToViewIndex(zone.afterModelPosition);
124+
const whitespaceId = this.listView.insertWhitespace(viewPosition, zone.heightInPx);
124125
const isInHiddenArea = this._isInHiddenRanges(zone);
125126
const myZone: IZoneWidget = {
126127
whitespaceId: whitespaceId,

src/vs/workbench/contrib/notebook/test/browser/notebookViewZones.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,19 @@ suite('NotebookRangeMap with whitesspaces', () => {
346346
instantiationService.stub(IConfigurationService, config);
347347
});
348348

349+
test('simple', () => {
350+
const rangeMap = new NotebookCellsLayout(0);
351+
rangeMap.splice(0, 0, [{ size: 479 }, { size: 163 }, { size: 182 }, { size: 106 }, { size: 106 }, { size: 106 }, { size: 87 }]);
352+
353+
const start = rangeMap.indexAt(650);
354+
const end = rangeMap.indexAfter(650 + 890 - 1);
355+
assert.strictEqual(start, 2);
356+
assert.strictEqual(end, 7);
357+
358+
rangeMap.insertWhitespace('1', 0, 18);
359+
assert.strictEqual(rangeMap.indexAt(650), 1);
360+
});
361+
349362
test('Whitespace CRUD', async function () {
350363
const twenty = { size: 20 };
351364

0 commit comments

Comments
 (0)