Fix Notebook Cell Editor height calc when pasting and scrolling (#284554)

Fix Notebook Cell Editor heigh calc when pasting and scrolling
This commit is contained in:
Don Jayamanne
2025-12-20 14:56:29 +11:00
committed by GitHub
parent d5df40d240
commit d87d06f0e6
2 changed files with 136 additions and 4 deletions
@@ -744,6 +744,7 @@ export class CodeCellLayout {
public _lastChangedEditorScrolltop?: number;
private _initialized: boolean = false;
private _pointerDown: boolean = false;
private _establishedContentHeight?: number;
constructor(
private readonly _enabled: boolean,
private readonly notebookEditor: IActiveNotebookEditorDelegate,
@@ -770,8 +771,11 @@ export class CodeCellLayout {
* - 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`.
* while navigating/scrolling), we store the actual content height in `_establishedContentHeight`
* and reuse it for all layout reasons except `onDidContentSizeChange`. This prevents the editor
* from shrinking back to its initial height after content has been added (e.g., pasting text).
* When `onDidContentSizeChange` fires, we update `_establishedContentHeight` to reflect the new
* content size, which subsequent scroll events will then reuse.
* - 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.
@@ -868,9 +872,16 @@ export class CodeCellLayout {
const elementHeight = this.notebookEditor.getHeightOfElement(this.viewCell);
const gotContentHeight = editor.getContentHeight();
// 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;
let editorContentHeight: number;
if (this._initialized && reason !== 'onDidContentSizeChange') {
// Reuse the previously established content height to avoid transient Monaco content height changes during scroll
editorContentHeight = this._establishedContentHeight ?? fallbackEditorContentHeight;
} else {
// Update the established content height when content actually changes or during initialization
editorContentHeight = fallbackEditorContentHeight;
this._establishedContentHeight = editorContentHeight;
}
const editorBottom = elementTop + this.viewCell.layoutInfo.outputContainerOffset;
const scrollBottom = this.notebookEditor.scrollBottom;
// When loading, scrollBottom -scrollTop === 0;
@@ -496,6 +496,127 @@ suite('CellPart', () => {
);
});
test('CodeCellLayout maintains content height after paste when scrolling', () => {
/**
* Regression test for https://github.com/microsoft/vscode/issues/284524
*
* Scenario: Cell starts with 1 line (37px), user pastes text (grows to 679px),
* then scrolls. During scroll, Monaco may report a transient smaller height (39px)
* due to the clipped layout. The fix uses _establishedContentHeight to maintain
* the actual content height (679px) instead of using the transient or initial values.
*/
const LINE_HEIGHT = 21;
const CELL_TOP_MARGIN = 6;
const CELL_OUTLINE_WIDTH = 1;
const STATUSBAR_HEIGHT = 22;
const VIEWPORT_HEIGHT = 1000;
const ELEMENT_TOP = 100;
const ELEMENT_HEIGHT = 1200;
const INITIAL_CONTENT_HEIGHT = 37; // 1 line
const INITIAL_EDITOR_HEIGHT = INITIAL_CONTENT_HEIGHT;
const OUTPUT_CONTAINER_OFFSET = 300;
const PASTED_CONTENT_HEIGHT = 679;
let contentHeight = INITIAL_CONTENT_HEIGHT;
const stubEditor = {
layoutCalls: [] as { width: number; height: number }[],
_lastScrollTopSet: -1,
getLayoutInfo: () => ({ width: 600, height: INITIAL_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 layoutInfo = {
statusBarHeight: STATUSBAR_HEIGHT,
topMargin: CELL_TOP_MARGIN,
outlineWidth: CELL_OUTLINE_WIDTH,
editorHeight: INITIAL_EDITOR_HEIGHT,
outputContainerOffset: OUTPUT_CONTAINER_OFFSET,
editorWidth: 600,
};
const viewCell: Partial<CodeCellViewModel> = {
isInputCollapsed: false,
layoutInfo: layoutInfo as unknown as CodeCellLayoutInfo,
};
const notebookEditor = {
scrollTop: 0,
get scrollBottom() {
return notebookEditor.scrollTop + VIEWPORT_HEIGHT;
},
setScrollTop: (v: number) => {
notebookEditor.scrollTop = v;
},
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: INITIAL_EDITOR_HEIGHT }
);
// Initial layout
layout.layoutEditor('init');
// Simulate pasting content - content grows to 679px
contentHeight = PASTED_CONTENT_HEIGHT;
layoutInfo.editorHeight = PASTED_CONTENT_HEIGHT;
layout.layoutEditor('onDidContentSizeChange');
// Now scroll and Monaco reports transient smaller height (39px)
// The fix should use the established 679px, not the transient 39px or initial 37px
contentHeight = 39;
notebookEditor.scrollTop = 200;
layout.layoutEditor('nbDidScroll');
const finalHeight = stubEditor.layoutCalls.at(-1)?.height;
// Verify the layout doesn't use the transient 39px value from Monaco
assert.notStrictEqual(
finalHeight,
39,
'Should not use Monaco\'s transient value (39px)'
);
// Verify the layout doesn't shrink back to the initial 37px value
assert.notStrictEqual(
finalHeight,
37,
'Should not use initial content height (37px)'
);
// The layout should be based on the established 679px content height
// The exact height will be calculated based on viewport, scroll position, etc.
// but should be significantly larger than 39px or 37px
assert.ok(
finalHeight && finalHeight > 100,
`Layout height (${finalHeight}px) should be calculated from established 679px content, not transient 39px or initial 37px`
);
});
test('CodeCellLayout does not programmatically scroll editor while pointer down', () => {
const LINE_HEIGHT = 21;
const CELL_TOP_MARGIN = 6;