scroll line into view

src/vs/workbench/contrib/notebook/browser/contrib/notebookFindWidget.ts

@@ -61,7 +61,7 @@ export class NotebookFindWidget extends SimpleFindWidget {
 		const matchIndex = nextIndex.remainder;
 
 		this.setCurrentFindMatchDecoration(cellIndex, matchIndex);
-		this._notebookEditor.revealLineInCenterIfOutsideViewport(this._findMatches[cellIndex].cell, this._findMatches[cellIndex].matches[matchIndex].range.startLineNumber);
+		this._notebookEditor.revealLineInView(this._findMatches[cellIndex].cell, this._findMatches[cellIndex].matches[matchIndex].range.startLineNumber);
 	}
 
 	hide() {

src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts

@@ -97,21 +97,23 @@ export interface INotebookEditor {
 
 	/**
 	 * Reveal cell into viewport.
-	 * If `offset` is provided, `top(cell) + offset` will be scrolled into view.
 	 */
-	revealInView(cell: CellViewModel, offset?: number): void;
+	revealInView(cell: CellViewModel): void;
 
 	/**
 	 * Reveal cell into viewport center.
-	 * If `offset` is provided, `top(cell) + offset` will be scrolled into view.
 	 */
-	revealInCenter(cell: CellViewModel, offset?: number): void;
+	revealInCenter(cell: CellViewModel): void;
 
 	/**
 	 * Reveal cell into viewport center if cell is currently out of the viewport.
-	 * If `offset` is provided, `top(cell) + offset` will be scrolled into view.
 	 */
-	revealInCenterIfOutsideViewport(cell: CellViewModel, offset?: number): void;
+	revealInCenterIfOutsideViewport(cell: CellViewModel): void;
+
+	/**
+	 * Reveal a line in notebook cell into viewport with minimal scrolling.
+	 */
+	revealLineInView(cell: CellViewModel, line: number): void;
 
 	/**
 	 * Reveal a line in notebook cell into viewport center.

src/vs/workbench/contrib/notebook/browser/notebookEditor.ts

@@ -427,27 +427,35 @@ export class NotebookEditor extends BaseEditor implements INotebookEditor {
 
 	//#region Editor Features
 
-	revealInView(cell: CellViewModel, offset?: number) {
+	revealInView(cell: CellViewModel) {
 		const index = this.notebookViewModel?.getViewCellIndex(cell);
 
 		if (index !== undefined) {
-			this.list?.revealInView(index, offset);
+			this.list?.revealInView(index);
 		}
 	}
 
-	revealInCenterIfOutsideViewport(cell: CellViewModel, offset?: number) {
+	revealInCenterIfOutsideViewport(cell: CellViewModel) {
 		const index = this.notebookViewModel?.getViewCellIndex(cell);
 
 		if (index !== undefined) {
-			this.list?.revealInCenterIfOutsideViewport(index, offset);
+			this.list?.revealInCenterIfOutsideViewport(index);
 		}
 	}
 
-	revealInCenter(cell: CellViewModel, offset?: number) {
+	revealInCenter(cell: CellViewModel) {
 		const index = this.notebookViewModel?.getViewCellIndex(cell);
 
 		if (index !== undefined) {
-			this.list?.revealInCenter(index, offset);
+			this.list?.revealInCenter(index);
+		}
+	}
+
+	revealLineInView(cell: CellViewModel, line: number): void {
+		const index = this.notebookViewModel?.getViewCellIndex(cell);
+
+		if (index !== undefined) {
+			this.list?.revealLineInView(index, line);
 		}
 	}
 

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

@@ -14,8 +14,8 @@ import { IListService, IWorkbenchListOptions, WorkbenchList } from 'vs/platform/
 import { IThemeService } from 'vs/platform/theme/common/themeService';
 import { IMouseWheelEvent } from 'vs/base/browser/mouseEvent';
 import { isMacintosh } from 'vs/base/common/platform';
-import { isNumber } from 'vs/base/common/types';
 import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookCellViewModel';
+import { EDITOR_TOP_PADDING } from 'vs/workbench/contrib/notebook/common/notebookCommon';
 
 export class NotebookCellList extends WorkbenchList<CellViewModel> {
 	get onWillScroll(): Event<ScrollEvent> { return this.view.onWillScroll; }
@@ -66,7 +66,6 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> {
 
 	// override
 	domFocus() {
-		// @TODO, custom menu doesn't work
 		if (document.activeElement && this.view.domNode.contains(document.activeElement)) {
 			// for example, when focus goes into monaco editor, if we refocus the list view, the editor will lose focus.
 			return;
@@ -79,10 +78,70 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> {
 		super.domFocus();
 	}
 
+	// TODO@rebornix TEST & Fix potential bugs
+	// List items have real dynamic heights, which means after we set `scrollTop` based on the `elementTop(index)`, the element at `index` might still be removed from the view once all relayouting tasks are done.
+	// For example, we scroll item 10 into the view upwards, in the first round, items 7, 8, 9, 10 are all in the viewport. Then item 7 and 8 resize themselves to be larger and finally item 10 is removed from the view.
+	// To ensure that item 10 is always there, we need to scroll item 10 to the top edge of the viewport.
+	revealLineInView(index: number, line: number) {
+		const revealLine = (scrolledIntoView: boolean, upwards: boolean) => {
+			// reveal the line slightly into view
+			const scrollTop = this.view.getScrollTop();
+			const wrapperBottom = scrollTop + this.view.renderHeight;
+			const lineOffset = element.getLineScrollTopOffset(line);
+			const elementTop = this.view.elementTop(index);
+			const lineTop = elementTop + lineOffset + EDITOR_TOP_PADDING;
+
+			// TODO@rebornix 30 ---> line height * 1.5
+			if (lineTop < scrollTop) {
+				this.view.setScrollTop(lineTop - 30);
+			} else if (lineTop > wrapperBottom) {
+				this.view.setScrollTop(scrollTop + lineTop - wrapperBottom + 30);
+			} else if (scrolledIntoView) {
+				// newly scrolled into view
+				if (upwards) {
+					// align to the bottom
+					this.view.setScrollTop(scrollTop + lineTop - wrapperBottom + 30);
+				} else {
+					// align to to top
+					this.view.setScrollTop(lineTop - 30);
+				}
+			}
+		};
+
+		const scrollTop = this.view.getScrollTop();
+		const wrapperBottom = scrollTop + this.view.renderHeight;
+		const elementTop = this.view.elementTop(index);
+		const element = this.view.element(index);
+
+		if (element.editorAttached) {
+			revealLine(false, false);
+		} else {
+			const elementHeight = this.view.elementHeight(index);
+			let upwards = false;
+
+			if (elementTop + elementHeight < scrollTop) {
+				// scroll downwards
+				this.view.setScrollTop(elementTop);
+				upwards = false;
+			} else if (elementTop > wrapperBottom) {
+				// scroll upwards
+				this.view.setScrollTop(elementTop - this.view.renderHeight / 2);
+				upwards = true;
+			}
+
+			const editorAttachedPromise = new Promise((resolve, reject) => {
+				element.onDidChangeEditorAttachState(state => state ? resolve() : reject());
+			});
+
+			editorAttachedPromise.then(() => {
+				revealLine(true, upwards);
+			});
+		}
+	}
+
 	revealLineInViewCenter(index: number, line: number) {
 		const elementTop = this.view.elementTop(index);
 		const viewItemOffset = elementTop;
-		// TODO@rebornix scroll the bottom to the center if the view is not visible before and it's scrolling upwards
 		this.view.setScrollTop(viewItemOffset - this.view.renderHeight / 2);
 
 		const element = this.view.element(index);
@@ -93,21 +152,8 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> {
 			this.view.setScrollTop(lineOffsetInView - this.view.renderHeight / 2);
 		};
 
-		const editorAttached = element.editorAttached;
-		if (!editorAttached) {
-			const editorAttachedPromise = new Promise((resolve, reject) => {
-				element.onDidChangeEditorAttachState(state => {
-					if (state) {
-						resolve();
-					} else {
-						reject();
-					}
-				});
-			});
-
-			editorAttachedPromise.then(() => {
-				revealLine();
-			});
+		if (!element.editorAttached) {
+			getEditorAttachedPromise(element).then(() => revealLine());
 		} else {
 			revealLine();
 		}
@@ -129,21 +175,8 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> {
 				this.view.setScrollTop(lineOffsetInView - this.view.renderHeight / 2);
 			};
 
-			const editorAttached = element.editorAttached;
-			if (!editorAttached) {
-				const editorAttachedPromise = new Promise((resolve, reject) => {
-					element.onDidChangeEditorAttachState(state => {
-						if (state) {
-							resolve();
-						} else {
-							reject();
-						}
-					});
-				});
-
-				editorAttachedPromise.then(() => {
-					revealLine();
-				});
+			if (!element.editorAttached) {
+				getEditorAttachedPromise(element).then(() => revealLine());
 			} else {
 				// should not happen
 			}
@@ -157,37 +190,43 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> {
 		}
 	}
 
-	revealInView(index: number, offset?: number) {
+	revealInView(index: number) {
 		const scrollTop = this.view.getScrollTop();
 		const wrapperBottom = scrollTop + this.view.renderHeight;
 		const elementTop = this.view.elementTop(index);
 
-		const viewItemOffset = elementTop + (isNumber(offset) ? offset : 0);
+		const viewItemOffset = elementTop;
 
 		if (viewItemOffset < scrollTop || viewItemOffset > wrapperBottom) {
 			this.view.setScrollTop(viewItemOffset);
 		}
 	}
 
-	revealInCenterIfOutsideViewport(index: number, offset?: number) {
+	revealInCenterIfOutsideViewport(index: number) {
 		const scrollTop = this.view.getScrollTop();
 		const wrapperBottom = scrollTop + this.view.renderHeight;
 		const elementTop = this.view.elementTop(index);
-		const viewItemOffset = elementTop + (isNumber(offset) ? offset : 0);
+		const viewItemOffset = elementTop;
 
 		if (viewItemOffset < scrollTop || viewItemOffset > wrapperBottom) {
 			this.view.setScrollTop(viewItemOffset - this.view.renderHeight / 2);
 		}
 	}
 
-	revealInCenter(index: number, offset?: number) {
+	revealInCenter(index: number) {
 		const elementTop = this.view.elementTop(index);
-		const viewItemOffset = elementTop + (isNumber(offset) ? offset : 0);
+		const viewItemOffset = elementTop;
 
 		this.view.setScrollTop(viewItemOffset - this.view.renderHeight / 2);
 	}
 }
 
+function getEditorAttachedPromise(element: CellViewModel) {
+	return new Promise((resolve, reject) => {
+		element.onDidChangeEditorAttachState(state => state ? resolve() : reject());
+	});
+}
+
 function isContextMenuFocused() {
 	return !!DOM.findParentWithClass(<HTMLElement>document.activeElement, 'context-view');
 }

src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts

@@ -131,6 +131,14 @@ export class CodeCell extends Disposable {
 			}
 		}));
 
+		this._register(templateData.editor!.onDidChangeCursorSelection(() => {
+			const primarySelection = templateData.editor!.getSelection();
+
+			if (primarySelection) {
+				this.notebookEditor.revealLineInView(viewCell, primarySelection!.positionLineNumber);
+			}
+		}));
+
 		this._register(viewCell.onDidChangeOutputs((splices) => {
 			if (!splices.length) {
 				return;

src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts

@@ -93,8 +93,10 @@ export class TestNotebookEditor implements INotebookEditor {
 	}
 
 	constructor(
-	) {
+	) { }
 
+	revealLineInView(cell: CellViewModel, line: number): void {
+		throw new Error('Method not implemented.');
 	}
 	getLayoutInfo(): NotebookLayoutInfo {
 		throw new Error('Method not implemented.');
@@ -114,13 +116,13 @@ export class TestNotebookEditor implements INotebookEditor {
 	hideFind(): void {
 		throw new Error('Method not implemented.');
 	}
-	revealInView(cell: CellViewModel, offset?: number | undefined): void {
+	revealInView(cell: CellViewModel): void {
 		throw new Error('Method not implemented.');
 	}
-	revealInCenter(cell: CellViewModel, offset?: number | undefined): void {
+	revealInCenter(cell: CellViewModel): void {
 		throw new Error('Method not implemented.');
 	}
-	revealInCenterIfOutsideViewport(cell: CellViewModel, offset?: number | undefined): void {
+	revealInCenterIfOutsideViewport(cell: CellViewModel): void {
 		throw new Error('Method not implemented.');
 	}
 	async insertEmptyNotebookCell(cell: CellViewModel, type: CellKind, direction: 'above' | 'below'): Promise<void> {