list widget should not remove rows from DOM when reusing

fix #112621

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

@@ -432,8 +432,24 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 		const deleteRange = { start, end: start + deleteCount };
 		const removeRange = Range.intersect(previousRenderRange, deleteRange);
 
+		// try to reuse rows, avoid removing them from DOM
+		const rowsToDispose = new Map<string, [IRow, T, number, number][]>();
 		for (let i = removeRange.start; i < removeRange.end; i++) {
-			this.removeItemFromDOM(i);
+			const item = this.items[i];
+			item.dragStartDisposable.dispose();
+
+			if (item.row) {
+				let rows = rowsToDispose.get(item.templateId);
+
+				if (!rows) {
+					rows = [];
+					rowsToDispose.set(item.templateId, rows);
+				}
+
+				rows.push([item.row, item.element, i, item.size]);
+			}
+
+			item.row = null;
 		}
 
 		const previousRestRange: IRange = { start: start + deleteCount, end: this.items.length };
@@ -491,7 +507,24 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 
 		for (const range of insertRanges) {
 			for (let i = range.start; i < range.end; i++) {
-				this.insertItemInDOM(i, beforeElement);
+				const item = this.items[i];
+				const rows = rowsToDispose.get(item.templateId);
+				const rowData = rows?.pop();
+				const row = rowData && rowData[0]; // try to reuse a row
+
+				this.insertItemInDOM(i, beforeElement, row);
+			}
+		}
+
+		for (const [templateId, rows] of rowsToDispose) {
+			for (const [row, element, index, size] of rows) {
+				const renderer = this.renderers.get(templateId);
+
+				if (renderer && renderer.disposeElement) {
+					renderer.disposeElement(element, index, row.templateData, size);
+				}
+
+				this.cache.release(row);
 			}
 		}
 
@@ -699,24 +732,26 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 
 	// DOM operations
 
-	private insertItemInDOM(index: number, beforeElement: HTMLElement | null): void {
+	private insertItemInDOM(index: number, beforeElement: HTMLElement | null, row?: IRow): void {
 		const item = this.items[index];
 
 		if (!item.row) {
-			item.row = this.cache.alloc(item.templateId);
-			const role = this.accessibilityProvider.getRole(item.element) || 'listitem';
-			item.row!.domNode!.setAttribute('role', role);
-			const checked = this.accessibilityProvider.isChecked(item.element);
-			if (typeof checked !== 'undefined') {
-				item.row!.domNode!.setAttribute('aria-checked', String(!!checked));
-			}
+			item.row = row ?? this.cache.alloc(item.templateId);
+		}
+
+		const role = this.accessibilityProvider.getRole(item.element) || 'listitem';
+		item.row.domNode.setAttribute('role', role);
+
+		const checked = this.accessibilityProvider.isChecked(item.element);
+		if (typeof checked !== 'undefined') {
+			item.row.domNode.setAttribute('aria-checked', String(!!checked));
 		}
 
-		if (!item.row.domNode!.parentElement) {
+		if (!item.row.domNode.parentElement) {
 			if (beforeElement) {
-				this.rowsContainer.insertBefore(item.row.domNode!, beforeElement);
+				this.rowsContainer.insertBefore(item.row.domNode, beforeElement);
 			} else {
-				this.rowsContainer.appendChild(item.row.domNode!);
+				this.rowsContainer.appendChild(item.row.domNode);
 			}
 		}
 
@@ -734,10 +769,10 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 
 		const uri = this.dnd.getDragURI(item.element);
 		item.dragStartDisposable.dispose();
-		item.row.domNode!.draggable = !!uri;
+		item.row.domNode.draggable = !!uri;
 
 		if (uri) {
-			const onDragStart = domEvent(item.row.domNode!, 'dragstart');
+			const onDragStart = domEvent(item.row.domNode, 'dragstart');
 			item.dragStartDisposable = onDragStart(event => this.onDragStart(item.element, uri, event));
 		}
 
@@ -768,36 +803,39 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 	}
 
 	private updateItemInDOM(item: IItem<T>, index: number): void {
-		item.row!.domNode!.style.top = `${this.elementTop(index)}px`;
+		item.row!.domNode.style.top = `${this.elementTop(index)}px`;
 
 		if (this.setRowHeight) {
-			item.row!.domNode!.style.height = `${item.size}px`;
+			item.row!.domNode.style.height = `${item.size}px`;
 		}
 
 		if (this.setRowLineHeight) {
-			item.row!.domNode!.style.lineHeight = `${item.size}px`;
+			item.row!.domNode.style.lineHeight = `${item.size}px`;
 		}
 
-		item.row!.domNode!.setAttribute('data-index', `${index}`);
-		item.row!.domNode!.setAttribute('data-last-element', index === this.length - 1 ? 'true' : 'false');
-		item.row!.domNode!.setAttribute('aria-setsize', String(this.accessibilityProvider.getSetSize(item.element, index, this.length)));
-		item.row!.domNode!.setAttribute('aria-posinset', String(this.accessibilityProvider.getPosInSet(item.element, index)));
-		item.row!.domNode!.setAttribute('id', this.getElementDomId(index));
+		item.row!.domNode.setAttribute('data-index', `${index}`);
+		item.row!.domNode.setAttribute('data-last-element', index === this.length - 1 ? 'true' : 'false');
+		item.row!.domNode.setAttribute('aria-setsize', String(this.accessibilityProvider.getSetSize(item.element, index, this.length)));
+		item.row!.domNode.setAttribute('aria-posinset', String(this.accessibilityProvider.getPosInSet(item.element, index)));
+		item.row!.domNode.setAttribute('id', this.getElementDomId(index));
 
-		item.row!.domNode!.classList.toggle('drop-target', item.dropTarget);
+		item.row!.domNode.classList.toggle('drop-target', item.dropTarget);
 	}
 
 	private removeItemFromDOM(index: number): void {
 		const item = this.items[index];
 		item.dragStartDisposable.dispose();
 
-		const renderer = this.renderers.get(item.templateId);
-		if (item.row && renderer && renderer.disposeElement) {
-			renderer.disposeElement(item.element, index, item.row.templateData, item.size);
-		}
+		if (item.row) {
+			const renderer = this.renderers.get(item.templateId);
+
+			if (renderer && renderer.disposeElement) {
+				renderer.disposeElement(item.element, index, item.row.templateData, item.size);
+			}
 
-		this.cache.release(item.row!);
-		item.row = null;
+			this.cache.release(item.row);
+			item.row = null;
+		}
 
 		if (this.horizontalScrolling) {
 			this.eventuallyUpdateScrollWidth();
@@ -1025,7 +1063,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 				const item = this.items[index]!;
 				item.dropTarget = true;
 
-				if (item.row && item.row.domNode) {
+				if (item.row) {
 					item.row.domNode.classList.add('drop-target');
 				}
 			}
@@ -1035,7 +1073,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 					const item = this.items[index]!;
 					item.dropTarget = false;
 
-					if (item.row && item.row.domNode) {
+					if (item.row) {
 						item.row.domNode.classList.remove('drop-target');
 					}
 				}
@@ -1261,7 +1299,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 
 		const size = item.size;
 
-		if (!this.setRowHeight && item.row && item.row.domNode) {
+		if (!this.setRowHeight && item.row) {
 			let newSize = item.row.domNode.offsetHeight;
 			item.size = newSize;
 			item.lastDynamicHeightWidth = this.renderWidth;
@@ -1270,8 +1308,8 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 
 		const row = this.cache.alloc(item.templateId);
 
-		row.domNode!.style.height = '';
-		this.rowsContainer.appendChild(row.domNode!);
+		row.domNode.style.height = '';
+		this.rowsContainer.appendChild(row.domNode);
 
 		const renderer = this.renderers.get(item.templateId);
 		if (renderer) {
@@ -1282,14 +1320,14 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 			}
 		}
 
-		item.size = row.domNode!.offsetHeight;
+		item.size = row.domNode.offsetHeight;
 
 		if (this.virtualDelegate.setDynamicHeight) {
 			this.virtualDelegate.setDynamicHeight(item.element, item.size);
 		}
 
 		item.lastDynamicHeightWidth = this.renderWidth;
-		this.rowsContainer.removeChild(row.domNode!);
+		this.rowsContainer.removeChild(row.domNode);
 		this.cache.release(row);
 
 		return item.size - size;

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

@@ -8,7 +8,7 @@ import { IDisposable } from 'vs/base/common/lifecycle';
 import { $ } from 'vs/base/browser/dom';
 
 export interface IRow {
-	domNode: HTMLElement | null;
+	domNode: HTMLElement;
 	templateId: string;
 	templateData: any;
 }
@@ -84,7 +84,6 @@ export class RowCache<T> implements IDisposable {
 			for (const cachedRow of cachedRows) {
 				const renderer = this.getRenderer(templateId);
 				renderer.disposeTemplate(cachedRow.templateData);
-				cachedRow.domNode = null;
 				cachedRow.templateData = null;
 			}
 		});

src/vs/base/test/browser/ui/tree/objectTree.test.ts

@@ -59,30 +59,30 @@ suite('ObjectTree', function () {
 
 			const navigator = tree.navigate();
 
-			assert.equal(navigator.current(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.current(), 0);
-			assert.equal(navigator.next(), 10);
-			assert.equal(navigator.current(), 10);
-			assert.equal(navigator.next(), 11);
-			assert.equal(navigator.current(), 11);
-			assert.equal(navigator.next(), 12);
-			assert.equal(navigator.current(), 12);
-			assert.equal(navigator.next(), 1);
-			assert.equal(navigator.current(), 1);
-			assert.equal(navigator.next(), 2);
-			assert.equal(navigator.current(), 2);
-			assert.equal(navigator.previous(), 1);
-			assert.equal(navigator.current(), 1);
-			assert.equal(navigator.previous(), 12);
-			assert.equal(navigator.previous(), 11);
-			assert.equal(navigator.previous(), 10);
-			assert.equal(navigator.previous(), 0);
-			assert.equal(navigator.previous(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.next(), 10);
-			assert.equal(navigator.first(), 0);
-			assert.equal(navigator.last(), 2);
+			assert.strictEqual(navigator.current(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.current(), 0);
+			assert.strictEqual(navigator.next(), 10);
+			assert.strictEqual(navigator.current(), 10);
+			assert.strictEqual(navigator.next(), 11);
+			assert.strictEqual(navigator.current(), 11);
+			assert.strictEqual(navigator.next(), 12);
+			assert.strictEqual(navigator.current(), 12);
+			assert.strictEqual(navigator.next(), 1);
+			assert.strictEqual(navigator.current(), 1);
+			assert.strictEqual(navigator.next(), 2);
+			assert.strictEqual(navigator.current(), 2);
+			assert.strictEqual(navigator.previous(), 1);
+			assert.strictEqual(navigator.current(), 1);
+			assert.strictEqual(navigator.previous(), 12);
+			assert.strictEqual(navigator.previous(), 11);
+			assert.strictEqual(navigator.previous(), 10);
+			assert.strictEqual(navigator.previous(), 0);
+			assert.strictEqual(navigator.previous(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.next(), 10);
+			assert.strictEqual(navigator.first(), 0);
+			assert.strictEqual(navigator.last(), 2);
 		});
 
 		test('should skip collapsed nodes', () => {
@@ -100,18 +100,18 @@ suite('ObjectTree', function () {
 
 			const navigator = tree.navigate();
 
-			assert.equal(navigator.current(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.next(), 1);
-			assert.equal(navigator.next(), 2);
-			assert.equal(navigator.next(), null);
-			assert.equal(navigator.previous(), 2);
-			assert.equal(navigator.previous(), 1);
-			assert.equal(navigator.previous(), 0);
-			assert.equal(navigator.previous(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.first(), 0);
-			assert.equal(navigator.last(), 2);
+			assert.strictEqual(navigator.current(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.next(), 1);
+			assert.strictEqual(navigator.next(), 2);
+			assert.strictEqual(navigator.next(), null);
+			assert.strictEqual(navigator.previous(), 2);
+			assert.strictEqual(navigator.previous(), 1);
+			assert.strictEqual(navigator.previous(), 0);
+			assert.strictEqual(navigator.previous(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.first(), 0);
+			assert.strictEqual(navigator.last(), 2);
 		});
 
 		test('should skip filtered elements', () => {
@@ -131,21 +131,21 @@ suite('ObjectTree', function () {
 
 			const navigator = tree.navigate();
 
-			assert.equal(navigator.current(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.next(), 10);
-			assert.equal(navigator.next(), 12);
-			assert.equal(navigator.next(), 2);
-			assert.equal(navigator.next(), null);
-			assert.equal(navigator.previous(), 2);
-			assert.equal(navigator.previous(), 12);
-			assert.equal(navigator.previous(), 10);
-			assert.equal(navigator.previous(), 0);
-			assert.equal(navigator.previous(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.next(), 10);
-			assert.equal(navigator.first(), 0);
-			assert.equal(navigator.last(), 2);
+			assert.strictEqual(navigator.current(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.next(), 10);
+			assert.strictEqual(navigator.next(), 12);
+			assert.strictEqual(navigator.next(), 2);
+			assert.strictEqual(navigator.next(), null);
+			assert.strictEqual(navigator.previous(), 2);
+			assert.strictEqual(navigator.previous(), 12);
+			assert.strictEqual(navigator.previous(), 10);
+			assert.strictEqual(navigator.previous(), 0);
+			assert.strictEqual(navigator.previous(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.next(), 10);
+			assert.strictEqual(navigator.first(), 0);
+			assert.strictEqual(navigator.last(), 2);
 		});
 
 		test('should be able to start from node', () => {
@@ -163,20 +163,20 @@ suite('ObjectTree', function () {
 
 			const navigator = tree.navigate(1);
 
-			assert.equal(navigator.current(), 1);
-			assert.equal(navigator.next(), 2);
-			assert.equal(navigator.current(), 2);
-			assert.equal(navigator.previous(), 1);
-			assert.equal(navigator.current(), 1);
-			assert.equal(navigator.previous(), 12);
-			assert.equal(navigator.previous(), 11);
-			assert.equal(navigator.previous(), 10);
-			assert.equal(navigator.previous(), 0);
-			assert.equal(navigator.previous(), null);
-			assert.equal(navigator.next(), 0);
-			assert.equal(navigator.next(), 10);
-			assert.equal(navigator.first(), 0);
-			assert.equal(navigator.last(), 2);
+			assert.strictEqual(navigator.current(), 1);
+			assert.strictEqual(navigator.next(), 2);
+			assert.strictEqual(navigator.current(), 2);
+			assert.strictEqual(navigator.previous(), 1);
+			assert.strictEqual(navigator.current(), 1);
+			assert.strictEqual(navigator.previous(), 12);
+			assert.strictEqual(navigator.previous(), 11);
+			assert.strictEqual(navigator.previous(), 10);
+			assert.strictEqual(navigator.previous(), 0);
+			assert.strictEqual(navigator.previous(), null);
+			assert.strictEqual(navigator.next(), 0);
+			assert.strictEqual(navigator.next(), 10);
+			assert.strictEqual(navigator.first(), 0);
+			assert.strictEqual(navigator.last(), 2);
 		});
 	});
 
@@ -219,10 +219,10 @@ suite('ObjectTree', function () {
 	});
 });
 
-function toArray(list: NodeList): Node[] {
-	const result: Node[] = [];
-	list.forEach(node => result.push(node));
-	return result;
+function getRowsTextContent(container: HTMLElement): string[] {
+	const rows = [...container.querySelectorAll('.monaco-list-row')];
+	rows.sort((a, b) => parseInt(a.getAttribute('data-index')!) - parseInt(b.getAttribute('data-index')!));
+	return rows.map(row => row.querySelector('.monaco-tl-contents')!.textContent!);
 }
 
 suite('CompressibleObjectTree', function () {
@@ -254,8 +254,7 @@ suite('CompressibleObjectTree', function () {
 		const tree = new CompressibleObjectTree<number>('test', container, new Delegate(), [new Renderer()]);
 		tree.layout(200);
 
-		const rows = toArray(container.querySelectorAll('.monaco-tl-contents'));
-		assert.equal(rows.length, 0);
+		assert.strictEqual(getRowsTextContent(container).length, 0);
 	});
 
 	test('simple', function () {
@@ -278,8 +277,7 @@ suite('CompressibleObjectTree', function () {
 			{ element: 2 }
 		]);
 
-		const rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['0', '10', '11', '12', '1', '2']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['0', '10', '11', '12', '1', '2']);
 	});
 
 	test('compressed', () => {
@@ -304,8 +302,7 @@ suite('CompressibleObjectTree', function () {
 			}
 		]);
 
-		let rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11/111', '1111', '1112', '1113']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11/111', '1111', '1112', '1113']);
 
 		tree.setChildren(11, [
 			{ element: 111 },
@@ -313,30 +310,26 @@ suite('CompressibleObjectTree', function () {
 			{ element: 113 },
 		]);
 
-		rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11', '111', '112', '113']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11', '111', '112', '113']);
 
 		tree.setChildren(113, [
 			{ element: 1131 }
 		]);
 
-		rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11', '111', '112', '113/1131']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11', '111', '112', '113/1131']);
 
 		tree.setChildren(1131, [
 			{ element: 1132 }
 		]);
 
-		rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11', '111', '112', '113/1131/1132']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11', '111', '112', '113/1131/1132']);
 
 		tree.setChildren(1131, [
 			{ element: 1132 },
 			{ element: 1133 },
 		]);
 
-		rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11', '111', '112', '113/1131', '1132', '1133']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11', '111', '112', '113/1131', '1132', '1133']);
 	});
 
 	test('enableCompression', () => {
@@ -361,15 +354,12 @@ suite('CompressibleObjectTree', function () {
 			}
 		]);
 
-		let rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11/111', '1111', '1112', '1113']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11/111', '1111', '1112', '1113']);
 
 		tree.updateOptions({ compressionEnabled: false });
-		rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1', '11', '111', '1111', '1112', '1113']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1', '11', '111', '1111', '1112', '1113']);
 
 		tree.updateOptions({ compressionEnabled: true });
-		rows = toArray(container.querySelectorAll('.monaco-tl-contents')).map(row => row.textContent);
-		assert.deepEqual(rows, ['1/11/111', '1111', '1112', '1113']);
+		assert.deepStrictEqual(getRowsTextContent(container), ['1/11/111', '1111', '1112', '1113']);
 	});
 });

src/vs/workbench/contrib/scm/browser/scmViewPane.ts

@@ -954,8 +954,6 @@ class ViewModel {
 	}
 
 	private refresh(item?: IRepositoryItem | IGroupItem): void {
-		const focusedInput = this.inputRenderer.getFocusedInput();
-
 		if (!this.alwaysShowRepositories && (this.items.size === 1 && (!item || isRepositoryItem(item)))) {
 			const item = Iterable.first(this.items.values())!;
 			this.tree.setChildren(null, this.render(item, this._treeViewState).children);
@@ -966,14 +964,6 @@ class ViewModel {
 			this.tree.setChildren(null, items.map(item => this.render(item, this._treeViewState)));
 		}
 
-		if (focusedInput) {
-			const inputWidget = this.inputRenderer.getRenderedInputWidget(focusedInput);
-
-			if (inputWidget) {
-				inputWidget.focus();
-			}
-		}
-
 		this._onDidChangeRepositoryCollapseState.fire();
 	}