Re #209154. Make execution count sticky. (#224905)

microsoft · Aug 6, 2024 · 6288e5d · 6288e5d
1 parent 145b650
commit 6288e5d
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 25 deletions.
diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts
@@ -499,6 +499,7 @@ export interface INotebookEditor {
 	readonly isDisposed: boolean;
 	readonly activeKernel: INotebookKernel | undefined;
 	readonly scrollTop: number;
+	readonly scrollBottom: number;
 	readonly scopedContextKeyService: IContextKeyService;
 	readonly activeCodeEditor: ICodeEditor | undefined;
 	readonly codeEditors: [ICellViewModel, ICodeEditor][];

diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts
@@ -1065,27 +1065,22 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
 	}
 
 	private _registerNotebookStickyScroll() {
-		this._notebookStickyScroll = this._register(this.instantiationService.createInstance(NotebookStickyScroll, this._notebookStickyScrollContainer, this, this._list));
-
-		const localDisposableStore = this._register(new DisposableStore());
+		this._notebookStickyScroll = this._register(this.instantiationService.createInstance(NotebookStickyScroll, this._notebookStickyScrollContainer, this, this._list, (sizeDelta) => {
+			if (this.isDisposed) {
+				return;
+			}
 
-		this._register(this._notebookStickyScroll.onDidChangeNotebookStickyScroll((sizeDelta) => {
-			const d = localDisposableStore.add(DOM.scheduleAtNextAnimationFrame(DOM.getWindow(this.getDomNode()), () => {
-				if (this.isDisposed) {
-					return;
+			if (this._dimension && this._isVisible) {
+				if (sizeDelta > 0) { // delta > 0 ==> sticky is growing, cell list shrinking
+					this.layout(this._dimension);
+					this.setScrollTop(this.scrollTop + sizeDelta);
+				} else if (sizeDelta < 0) { // delta < 0 ==> sticky is shrinking, cell list growing
+					this.setScrollTop(this.scrollTop + sizeDelta);
+					this.layout(this._dimension);
 				}
+			}
 
-				if (this._dimension && this._isVisible) {
-					if (sizeDelta > 0) { // delta > 0 ==> sticky is growing, cell list shrinking
-						this.layout(this._dimension);
-						this.setScrollTop(this.scrollTop + sizeDelta);
-					} else if (sizeDelta < 0) { // delta < 0 ==> sticky is shrinking, cell list growing
-						this.setScrollTop(this.scrollTop + sizeDelta);
-						this.layout(this._dimension);
-					}
-				}
-				localDisposableStore.delete(d);
-			}));
+			this._onDidScroll.fire();
 		}));
 	}
 
@@ -2100,6 +2095,10 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
 		return this._list.scrollTop;
 	}
 
+	get scrollBottom() {
+		return this._list.scrollTop + this._list.getRenderHeight();
+	}
+
 	getAbsoluteTopOfElement(cell: ICellViewModel) {
 		return this._list.getCellViewScrollTop(cell);
 	}

diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts
@@ -6,9 +6,11 @@
 import * as DOM from 'vs/base/browser/dom';
 import { disposableTimeout } from 'vs/base/common/async';
 import { DisposableStore } from 'vs/base/common/lifecycle';
+import { clamp } from 'vs/base/common/numbers';
 import { ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
 import { CellViewModelStateChangeEvent } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents';
 import { CellContentPart } from 'vs/workbench/contrib/notebook/browser/view/cellPart';
+import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel';
 import { NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
 import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
 
@@ -39,6 +41,10 @@ export class CellExecutionPart extends CellContentPart {
 				this.updateExecutionOrder(this.currentCell.internalMetadata);
 			}
 		}));
+
+		this._register(this._notebookEditor.onDidScroll(() => {
+			this._updatePosition();
+		}));
 	}
 
 	override didRenderCell(element: ICellViewModel): void {
@@ -74,12 +80,38 @@ export class CellExecutionPart extends CellContentPart {
 	}
 
 	override updateInternalLayoutNow(element: ICellViewModel): void {
-		if (element.isInputCollapsed) {
-			DOM.hide(this._executionOrderLabel);
-		} else {
-			DOM.show(this._executionOrderLabel);
-			const top = element.layoutInfo.editorHeight - 22 + element.layoutInfo.statusBarHeight;
-			this._executionOrderLabel.style.top = `${top}px`;
+		this._updatePosition();
+	}
+
+	private _updatePosition() {
+		if (this.currentCell) {
+			if (this.currentCell.isInputCollapsed) {
+				DOM.hide(this._executionOrderLabel);
+			} else {
+				DOM.show(this._executionOrderLabel);
+				let top = this.currentCell.layoutInfo.editorHeight - 22 + this.currentCell.layoutInfo.statusBarHeight;
+
+				if (this.currentCell instanceof CodeCellViewModel) {
+					const elementTop = this._notebookEditor.getAbsoluteTopOfElement(this.currentCell);
+					const editorBottom = elementTop + this.currentCell.layoutInfo.outputContainerOffset;
+					// another approach to avoid the flicker caused by sticky scroll is manually calculate the scrollBottom:
+					// const scrollBottom = this._notebookEditor.scrollTop + this._notebookEditor.getLayoutInfo().height - 26 - this._notebookEditor.getLayoutInfo().stickyHeight;
+					const scrollBottom = this._notebookEditor.scrollBottom;
+
+					const lineHeight = 22;
+					if (scrollBottom <= editorBottom) {
+						const offset = editorBottom - scrollBottom;
+						top -= offset;
+						top = clamp(
+							top,
+							lineHeight + 12, // line height + padding for single line
+							this.currentCell.layoutInfo.editorHeight - lineHeight + this.currentCell.layoutInfo.statusBarHeight
+						);
+					}
+				}
+
+				this._executionOrderLabel.style.top = `${top}px`;
+			}
 		}
 	}
 }
diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts
@@ -106,6 +106,8 @@ export class NotebookStickyScroll extends Disposable {
 	readonly onDidChangeNotebookStickyScroll: Event<number> = this._onDidChangeNotebookStickyScroll.event;
 	private notebookCellOutlineReference?: IReference<NotebookCellOutlineDataSource>;
 
+	private readonly _layoutDisposableStore = this._register(new DisposableStore());
+
 	getDomNode(): HTMLElement {
 		return this.domNode;
 	}
@@ -143,6 +145,7 @@ export class NotebookStickyScroll extends Disposable {
 		private readonly domNode: HTMLElement,
 		private readonly notebookEditor: INotebookEditor,
 		private readonly notebookCellList: INotebookCellList,
+		private readonly layoutFn: (delta: number) => void,
 		@IContextMenuService private readonly _contextMenuService: IContextMenuService,
 		@IInstantiationService private readonly instantiationService: IInstantiationService
 	) {
@@ -269,8 +272,16 @@ export class NotebookStickyScroll extends Disposable {
 		const sizeDelta = this.getCurrentStickyHeight() - oldStickyHeight;
 		if (sizeDelta !== 0) {
 			this._onDidChangeNotebookStickyScroll.fire(sizeDelta);
+
+			const d = this._layoutDisposableStore.add(DOM.scheduleAtNextAnimationFrame(DOM.getWindow(this.getDomNode()), () => {
+				this.layoutFn(sizeDelta);
+				this.updateDisplay();
+
+				this._layoutDisposableStore.delete(d);
+			}));
+		} else {
+			this.updateDisplay();
 		}
-		this.updateDisplay();
 	}
 
 	private updateDisplay() {