Fix Notebook cell layout issue when navigating cells (#284341)

* Fix Notebook cell layout issue when navigating cells * Update src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-12-20 02:08:47 +00:00 · 2025-12-19 22:35:28 +11:00
parent 7d1e2d4bad
commit 5ad0c7c621
2 changed files with 202 additions and 2 deletions
--- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts
+++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts
@@ -766,6 +766,16 @@ export class CodeCellLayout {
 	 *    crop content when the cell is partially visible (top or bottom clipped) or when content is
 	 *    taller than the viewport.
 	 *
+	 * Additional invariants:
+	 *  - Content height stability: once the layout has been initialized, scroll-driven re-layouts can
+	 *    observe transient Monaco content heights that reflect the current clipped layout (rather than
+	 *    the full input height). To keep the notebook list layout stable (avoiding overlapping cells
+	 *    while navigating/scrolling), we reuse the previously established content height for all reasons
+	 *    except `onDidContentSizeChange`.
+	 *  - Pointer-drag gating: while the user is holding the mouse button down in the editor (drag
+	 *    selection or potential drag selection), we avoid programmatic `editor.setScrollTop(...)` updates
+	 *    to prevent selection/scroll feedback loops and "stuck selection" behavior.
+	 *
 	 * ---------------------------------------------------------------------------
 	 * SECTION 1. OVERALL NOTEBOOK VIEW (EACH CELL HAS AN 18px GAP ABOVE IT)
 	 * Legend:
@@ -857,7 +867,10 @@ export class CodeCellLayout {
 		const elementBottom = this.notebookEditor.getAbsoluteBottomOfElement(this.viewCell);
 		const elementHeight = this.notebookEditor.getHeightOfElement(this.viewCell);
 		const gotContentHeight = editor.getContentHeight();
-		const editorContentHeight = Math.max((gotContentHeight === -1 ? editor.getLayoutInfo().height : gotContentHeight), gotContentHeight === -1 ? this._initialEditorDimension.height : gotContentHeight); // || this.calculatedEditorHeight || 0;
+		// If we've already calculated the editor content height once before and the contents haven't changed, use that.
+		const previouslyCalculatedHeight = this._initialized && reason !== 'onDidContentSizeChange' ? this._initialEditorDimension.height : undefined;
+		const fallbackEditorContentHeight = gotContentHeight === -1 ? Math.max(editor.getLayoutInfo().height, this._initialEditorDimension.height) : gotContentHeight;
+		const editorContentHeight = previouslyCalculatedHeight ?? fallbackEditorContentHeight; // || this.calculatedEditorHeight || 0;
 		const editorBottom = elementTop + this.viewCell.layoutInfo.outputContainerOffset;
 		const scrollBottom = this.notebookEditor.scrollBottom;
 		// When loading, scrollBottom -scrollTop === 0;
@@ -903,7 +916,7 @@ export class CodeCellLayout {
 			}
 		}

-		this._logService.debug(`${reason} (${this._editorVisibility})`);
+		this._logService.debug(`${reason} (${this._editorVisibility}, ${this._initialized})`);
 		this._logService.debug(`=> Editor Top = ${top}px (editHeight = ${editorHeight}, editContentHeight: ${editorContentHeight})`);
 		this._logService.debug(`=> eleTop = ${elementTop}, eleBottom = ${elementBottom}, eleHeight = ${elementHeight}`);
 		this._logService.debug(`=> scrollTop = ${scrollTop}, top = ${top}`);
--- a/src/vs/workbench/contrib/notebook/test/browser/view/cellPart.test.ts
+++ b/src/vs/workbench/contrib/notebook/test/browser/view/cellPart.test.ts
@@ -401,4 +401,191 @@ suite('CellPart', () => {
 			);
 		}
 	});
+
+	test('CodeCellLayout reuses content height after init', () => {
+		const LINE_HEIGHT = 21;
+		const STATUSBAR_HEIGHT = 22;
+		const CELL_TOP_MARGIN = 6;
+		const CELL_OUTLINE_WIDTH = 1;
+		const VIEWPORT_HEIGHT = 1000;
+		const ELEMENT_TOP = 100;
+		const ELEMENT_HEIGHT = 1200;
+		const OUTPUT_CONTAINER_OFFSET = 300;
+		const EDITOR_HEIGHT = 800;
+
+		let contentHeight = 800;
+		const stubEditor = {
+			layoutCalls: [] as { width: number; height: number }[],
+			_lastScrollTopSet: -1,
+			getLayoutInfo: () => ({ width: 600, height: EDITOR_HEIGHT }),
+			getContentHeight: () => contentHeight,
+			layout: (dim: { width: number; height: number }) => {
+				stubEditor.layoutCalls.push(dim);
+			},
+			setScrollTop: (v: number) => {
+				stubEditor._lastScrollTopSet = v;
+			},
+			hasModel: () => true,
+		};
+		const editorPart = { style: { top: '' } };
+		const template: Partial<CodeCellRenderTemplate> = {
+			editor: stubEditor as unknown as ICodeEditor,
+			editorPart: editorPart as unknown as HTMLElement,
+		};
+		const viewCell: Partial<CodeCellViewModel> = {
+			isInputCollapsed: false,
+			layoutInfo: {
+				statusBarHeight: STATUSBAR_HEIGHT,
+				topMargin: CELL_TOP_MARGIN,
+				outlineWidth: CELL_OUTLINE_WIDTH,
+				editorHeight: EDITOR_HEIGHT,
+				outputContainerOffset: OUTPUT_CONTAINER_OFFSET,
+				editorWidth: 600,
+			} as unknown as CodeCellLayoutInfo,
+		};
+		const notebookEditor = {
+			scrollTop: 0,
+			get scrollBottom() {
+				return VIEWPORT_HEIGHT;
+			},
+			setScrollTop: (v: number) => {
+				/* no-op */
+			},
+			getLayoutInfo: () => ({
+				fontInfo: { lineHeight: LINE_HEIGHT },
+				height: VIEWPORT_HEIGHT,
+				stickyHeight: 0,
+			}),
+			getAbsoluteTopOfElement: () => ELEMENT_TOP,
+			getAbsoluteBottomOfElement: () => ELEMENT_TOP + OUTPUT_CONTAINER_OFFSET,
+			getHeightOfElement: () => ELEMENT_HEIGHT,
+			notebookOptions: {
+				getLayoutConfiguration: () => ({ editorTopPadding: 6 }),
+			},
+		};
+
+		const layout = new CodeCellLayout(
+			true,
+			notebookEditor as unknown as IActiveNotebookEditorDelegate,
+			viewCell as CodeCellViewModel,
+			template as CodeCellRenderTemplate,
+			{ debug: () => { } },
+			{ width: 600, height: EDITOR_HEIGHT }
+		);
+
+		layout.layoutEditor('init');
+		assert.strictEqual(layout.editorVisibility, 'Full');
+		assert.strictEqual(stubEditor.layoutCalls.at(-1)?.height, 800);
+
+		// Simulate Monaco reporting a transient smaller content height on scroll.
+		contentHeight = 200;
+		layout.layoutEditor('nbDidScroll');
+		assert.strictEqual(layout.editorVisibility, 'Full');
+		assert.strictEqual(
+			stubEditor.layoutCalls.at(-1)?.height,
+			800,
+			'nbDidScroll should reuse the established content height'
+		);
+
+		layout.layoutEditor('onDidContentSizeChange');
+		assert.strictEqual(layout.editorVisibility, 'Full');
+		assert.strictEqual(
+			stubEditor.layoutCalls.at(-1)?.height,
+			200,
+			'onDidContentSizeChange should refresh the content height'
+		);
+	});
+
+	test('CodeCellLayout does not programmatically scroll editor while pointer down', () => {
+		const LINE_HEIGHT = 21;
+		const CELL_TOP_MARGIN = 6;
+		const CELL_OUTLINE_WIDTH = 1;
+		const STATUSBAR_HEIGHT = 22;
+		const VIEWPORT_HEIGHT = 220;
+		const ELEMENT_TOP = 100;
+		const EDITOR_CONTENT_HEIGHT = 500;
+		const EDITOR_HEIGHT = EDITOR_CONTENT_HEIGHT;
+		const OUTPUT_CONTAINER_OFFSET = 600;
+		const ELEMENT_HEIGHT = 900;
+		const scrollTop = ELEMENT_TOP + CELL_TOP_MARGIN + 20;
+		const scrollBottom = scrollTop + VIEWPORT_HEIGHT;
+
+		const stubEditor = {
+			_lastScrollTopSet: -1,
+			getLayoutInfo: () => ({ width: 600, height: EDITOR_HEIGHT }),
+			getContentHeight: () => EDITOR_CONTENT_HEIGHT,
+			layout: () => {
+				/* no-op */
+			},
+			setScrollTop: (v: number) => {
+				stubEditor._lastScrollTopSet = v;
+			},
+			hasModel: () => true,
+		};
+		const editorPart = { style: { top: '' } };
+		const template: Partial<CodeCellRenderTemplate> = {
+			editor: stubEditor as unknown as ICodeEditor,
+			editorPart: editorPart as unknown as HTMLElement,
+		};
+		const viewCell: Partial<CodeCellViewModel> = {
+			isInputCollapsed: false,
+			layoutInfo: {
+				statusBarHeight: STATUSBAR_HEIGHT,
+				topMargin: CELL_TOP_MARGIN,
+				outlineWidth: CELL_OUTLINE_WIDTH,
+				editorHeight: EDITOR_HEIGHT,
+				outputContainerOffset: OUTPUT_CONTAINER_OFFSET,
+			} as unknown as CodeCellLayoutInfo,
+		};
+		const notebookEditor = {
+			scrollTop,
+			get scrollBottom() {
+				return scrollBottom;
+			},
+			setScrollTop: (v: number) => {
+				/* no-op */
+			},
+			getLayoutInfo: () => ({
+				fontInfo: { lineHeight: LINE_HEIGHT },
+				height: VIEWPORT_HEIGHT,
+				stickyHeight: 0,
+			}),
+			getAbsoluteTopOfElement: () => ELEMENT_TOP,
+			getAbsoluteBottomOfElement: () => ELEMENT_TOP + OUTPUT_CONTAINER_OFFSET,
+			getHeightOfElement: () => ELEMENT_HEIGHT,
+			notebookOptions: {
+				getLayoutConfiguration: () => ({ editorTopPadding: 6 }),
+			},
+		};
+
+		const layout = new CodeCellLayout(
+			true,
+			notebookEditor as unknown as IActiveNotebookEditorDelegate,
+			viewCell as CodeCellViewModel,
+			template as CodeCellRenderTemplate,
+			{ debug: () => { } },
+			{ width: 600, height: EDITOR_HEIGHT }
+		);
+
+		layout.layoutEditor('init');
+		stubEditor._lastScrollTopSet = -1;
+
+		layout.setPointerDown(true);
+		layout.layoutEditor('nbDidScroll');
+		assert.strictEqual(layout.editorVisibility, 'Full (Small Viewport)');
+		assert.strictEqual(
+			stubEditor._lastScrollTopSet,
+			-1,
+			'Expected no programmatic editor.setScrollTop while pointer is down'
+		);
+
+		layout.setPointerDown(false);
+		layout.layoutEditor('nbDidScroll');
+		assert.strictEqual(layout.editorVisibility, 'Full (Small Viewport)');
+		assert.notStrictEqual(
+			stubEditor._lastScrollTopSet,
+			-1,
+			'Expected editor.setScrollTop to resume once pointer is released'
+		);
+	});
 });