Merge pull request #287050 from AshtonYoon/fix/janky-scrolling-in-markdown-preview

Fix janky scrolling in markdown preview with code blocks

extensions/markdown-language-features/preview-src/index.ts

@@ -14,6 +14,7 @@ import type { ToWebviewMessage } from '../types/previewMessaging';
 import { isOfScheme, Schemes } from '../src/util/schemes';
 
 let scrollDisabledCount = 0;
+let scrollDisabledTimer: number | undefined;
 
 const marker = new ActiveLineMarker();
 const settings = new SettingsManager();
@@ -131,7 +132,13 @@ onceDocumentLoaded(() => {
 
 const onUpdateView = (() => {
 	const doScroll = throttle((line: number) => {
-		scrollDisabledCount += 1;
+		scrollDisabledCount = 1;
+		if (scrollDisabledTimer) {
+			clearTimeout(scrollDisabledTimer);
+		}
+		scrollDisabledTimer = window.setTimeout(() => {
+			scrollDisabledCount = 0;
+		}, 50);
 		doAfterImagesLoaded(() => scrollToRevealSourceLine(line, documentVersion, settings));
 	}, 50);
 
@@ -371,12 +378,12 @@ window.addEventListener('scroll', throttle(() => {
 	updateScrollProgress();
 
 	if (scrollDisabledCount > 0) {
-		scrollDisabledCount -= 1;
-	} else {
-		const line = getEditorLineNumberForPageOffset(window.scrollY, documentVersion);
-		if (typeof line === 'number' && !isNaN(line)) {
-			messaging.postMessage('revealLine', { line });
-		}
+		return;
+	}
+
+	const line = getEditorLineNumberForPageOffset(window.scrollY, documentVersion);
+	if (typeof line === 'number' && !isNaN(line)) {
+		messaging.postMessage('revealLine', { line });
 	}
 }, 50));
 

extensions/markdown-language-features/preview-src/scroll-sync.ts

@@ -15,6 +15,7 @@ export class CodeLineElement {
 		readonly element: HTMLElement,
 		readonly line: number,
 		readonly codeElement?: HTMLElement,
+		readonly endLine?: number,
 	) {
 		this.#detailParentElements = Array.from(getParentsWithTagName<HTMLDetailsElement>(element, 'DETAILS'));
 	}
@@ -55,11 +56,17 @@ const getCodeLineElements = (() => {
 					continue;
 				}
 
-
 				if (element.tagName === 'CODE' && element.parentElement && element.parentElement.tagName === 'PRE') {
 					// Fenced code blocks are a special case since the `code-line` can only be marked on
 					// the `<code>` element and not the parent `<pre>` element.
-					cachedElements.push(new CodeLineElement(element.parentElement, line, element));
+					// Calculate the end line by counting newlines in the code block
+					const text = element.textContent || '';
+					const lineCount = (text.match(/\n/g) || []).length + 1;
+					const endLine = line + lineCount - 1;
+					cachedElements.push(new CodeLineElement(element.parentElement, line, element, endLine));
+				} else if (element.tagName === 'PRE') {
+					// Skip PRE elements as they will be handled via their CODE children
+					// This prevents duplicate entries for the same line number
 				} else if (element.tagName === 'UL' || element.tagName === 'OL') {
 					// Skip adding list elements since the first child has the same code line (and should be preferred)
 				} else {
@@ -112,6 +119,7 @@ export function getLineElementsAtPageOffset(offset: number, documentVersion: num
 	}
 	const hiElement = lines[hi];
 	const hiBounds = getElementBounds(hiElement);
+
 	if (hi >= 1 && hiBounds.top > position) {
 		const loElement = lines[lo];
 		return { previous: loElement, next: hiElement };
@@ -122,9 +130,16 @@ export function getLineElementsAtPageOffset(offset: number, documentVersion: num
 	return { previous: hiElement };
 }
 
-function getElementBounds({ element }: CodeLineElement): { top: number; height: number } {
+function getElementBounds(codeLineElement: CodeLineElement): { top: number; height: number } {
+	const { element, codeElement } = codeLineElement;
 	const myBounds = element.getBoundingClientRect();
 
+	// For fenced code blocks (PRE elements containing CODE), use the full height
+	// Don't look for children as the CODE element itself would be found as a child
+	if (codeElement) {
+		return myBounds;
+	}
+
 	// Some code line elements may contain other code line elements.
 	// In those cases, only take the height up to that child.
 	const codeLineChild = element.querySelector(`.${codeLineClass}`);
@@ -140,6 +155,43 @@ function getElementBounds({ element }: CodeLineElement): { top: number; height:
 	return myBounds;
 }
 
+/**
+ * Get the content bounds for a code line element, accounting for padding.
+ * For code blocks, returns the bounds of the content area (excluding padding).
+ * For other elements, returns the same as getElementBounds.
+ */
+function getContentBounds(codeLineElement: CodeLineElement): {
+	top: number;
+	height: number;
+	paddingTop: number;
+	paddingBottom: number;
+} {
+	const { element, codeElement } = codeLineElement;
+	const bounds = getElementBounds(codeLineElement);
+
+	// For code blocks (PRE elements), account for padding
+	if (codeElement) {
+		const computedStyle = window.getComputedStyle(element);
+		const paddingTop = parseFloat(computedStyle.paddingTop) || 0;
+		const paddingBottom = parseFloat(computedStyle.paddingBottom) || 0;
+
+		return {
+			top: bounds.top + paddingTop,
+			height: bounds.height - paddingTop - paddingBottom,
+			paddingTop,
+			paddingBottom
+		};
+	}
+
+	// For non-code elements, no padding adjustment needed
+	return {
+		top: bounds.top,
+		height: bounds.height,
+		paddingTop: 0,
+		paddingBottom: 0
+	};
+}
+
 /**
  * Attempt to reveal the element for a source line in the editor.
  */
@@ -160,17 +212,45 @@ export function scrollToRevealSourceLine(line: number, documentVersion: number,
 	let scrollTo = 0;
 	const rect = getElementBounds(previous);
 	const previousTop = rect.top;
-	if (next && next.line !== previous.line) {
-		// Between two elements. Go to percentage offset between them.
+
+
+	// Check if previous is a multi-line code block
+	if (previous.endLine && previous.endLine > previous.line) {
+		if (line < previous.endLine) {
+			// We're inside the code block - scroll proportionally through its content height (excluding padding)
+			const contentBounds = getContentBounds(previous);
+			const progressInCodeBlock = (line - previous.line) / (previous.endLine - previous.line);
+
+
+			// Calculate absolute position to content area
+			const contentAbsoluteTop = window.scrollY + contentBounds.top;
+			const targetAbsoluteY = contentAbsoluteTop + (contentBounds.height * progressInCodeBlock);
+			scrollTo = targetAbsoluteY;
+
+		} else if (next && next.line !== previous.line) {
+			// We're after the code block but before the next element
+			const betweenProgress = (line - previous.endLine) / (next.line - previous.endLine);
+			const elementAbsoluteEnd = window.scrollY + previousTop + rect.height;
+			const nextAbsoluteTop = window.scrollY + next.element.getBoundingClientRect().top;
+			const betweenHeight = nextAbsoluteTop - elementAbsoluteEnd;
+			scrollTo = elementAbsoluteEnd + betweenProgress * betweenHeight;
+		} else {
+			// Shouldn't happen, but fall back to end of element
+			scrollTo = window.scrollY + previousTop + rect.height;
+		}
+	} else if (next && next.line !== previous.line) {
+		// Original logic: Between two elements. Go to percentage offset between them.
 		const betweenProgress = (line - previous.line) / (next.line - previous.line);
-		const previousEnd = previousTop + rect.height;
-		const betweenHeight = next.element.getBoundingClientRect().top - previousEnd;
-		scrollTo = previousEnd + betweenProgress * betweenHeight;
+		const elementAbsoluteEnd = window.scrollY + previousTop + rect.height;
+		const nextAbsoluteTop = window.scrollY + next.element.getBoundingClientRect().top;
+		const betweenHeight = nextAbsoluteTop - elementAbsoluteEnd;
+		scrollTo = elementAbsoluteEnd + betweenProgress * betweenHeight;
 	} else {
 		const progressInElement = line - Math.floor(line);
-		scrollTo = previousTop + (rect.height * progressInElement);
+		scrollTo = window.scrollY + previousTop + (rect.height * progressInElement);
 	}
-	window.scroll(window.scrollX, Math.max(1, window.scrollY + scrollTo));
+
+	window.scroll(window.scrollX, Math.max(1, scrollTo));
 }
 
 export function getEditorLineNumberForPageOffset(offset: number, documentVersion: number): number | null {
@@ -181,12 +261,44 @@ export function getEditorLineNumberForPageOffset(offset: number, documentVersion
 		}
 		const previousBounds = getElementBounds(previous);
 		const offsetFromPrevious = (offset - window.scrollY - previousBounds.top);
+
+
+		// Check if previous is a multi-line code block
+		if (previous.endLine && previous.endLine > previous.line) {
+			// Use content bounds to exclude padding from the calculation
+			const contentBounds = getContentBounds(previous);
+			const offsetFromContent = offset - window.scrollY - contentBounds.top;
+
+
+			// Check if we're within the code block's content area (excluding padding)
+			if (offsetFromContent >= 0 && offsetFromContent <= contentBounds.height) {
+				const progressWithinCodeBlock = offsetFromContent / contentBounds.height;
+				const calculatedLine = previous.line + progressWithinCodeBlock * (previous.endLine - previous.line);
+				return calculatedLine;
+			} else if (next && offsetFromContent > contentBounds.height) {
+				// We're in the gap after the code block content (including bottom padding)
+				const gapOffset = offsetFromContent - contentBounds.height;
+				const nextBounds = getElementBounds(next);
+				const contentEnd = contentBounds.top + contentBounds.height;
+				const gapHeight = nextBounds.top - contentEnd;
+				const progressInGap = gapOffset / gapHeight;
+				const calculatedLine = previous.endLine + progressInGap * (next.line - previous.endLine);
+				return calculatedLine;
+			} else if (offsetFromContent < 0) {
+				// We're in the top padding area
+				// Fall through to original logic
+			}
+		}
+
+		// Original logic
 		if (next) {
 			const progressBetweenElements = offsetFromPrevious / (getElementBounds(next).top - previousBounds.top);
-			return previous.line + progressBetweenElements * (next.line - previous.line);
+			const calculatedLine = previous.line + progressBetweenElements * (next.line - previous.line);
+			return calculatedLine;
 		} else {
 			const progressWithinElement = offsetFromPrevious / (previousBounds.height);
-			return previous.line + progressWithinElement;
+			const calculatedLine = previous.line + progressWithinElement;
+			return calculatedLine;
 		}
 	}
 	return null;

extensions/markdown-language-features/src/preview/preview.ts

@@ -57,6 +57,7 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider {
 	#firstUpdate = true;
 	#currentVersion?: PreviewDocumentVersion;
 	#isScrolling = false;
+	#scrollingTimer?: NodeJS.Timeout;
 
 	#imageInfo: readonly ImageInfo[] = [];
 	readonly #fileWatchersBySrc = new Map</* src: */ string, vscode.FileSystemWatcher>();
@@ -313,6 +314,12 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider {
 			}
 
 			this.#isScrolling = true;
+			if (this.#scrollingTimer) {
+				clearTimeout(this.#scrollingTimer);
+			}
+			this.#scrollingTimer = setTimeout(() => {
+				this.#isScrolling = false;
+			}, 200);
 			scrollEditorToLine(line, editor);
 		}
 	}