Use the future scroll position to compute the scroll top and reuse the current...

Use the future scroll position to compute the scroll top and reuse the current smooth scrolling animation (#104144, #107704, #104284)
Co-authored-by: NJoao Moreno <joao.moreno@microsoft.com>

src/vs/base/browser/ui/list/listView.ts

@@ -809,14 +809,14 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 		return scrollPosition.scrollTop;
 	}
 
-	setScrollTop(scrollTop: number): void {
+	setScrollTop(scrollTop: number, reuseAnimation?: boolean): void {
 		if (this.scrollableElementUpdateDisposable) {
 			this.scrollableElementUpdateDisposable.dispose();
 			this.scrollableElementUpdateDisposable = null;
 			this.scrollableElement.setScrollDimensions({ scrollHeight: this.scrollHeight });
 		}
 
-		this.scrollableElement.setScrollPosition({ scrollTop });
+		this.scrollableElement.setScrollPosition({ scrollTop, reuseAnimation });
 	}
 
 	getScrollLeft(): number {
@@ -895,9 +895,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 			this.render(previousRenderRange, e.scrollTop, e.height, e.scrollLeft, e.scrollWidth);
 
 			if (this.supportDynamicHeights) {
-				// Don't update scrollTop from within an scroll event
-				// so we don't break smooth scrolling. #104144
-				this._rerender(e.scrollTop, e.height, false);
+				this._rerender(e.scrollTop, e.height, e.inSmoothScrolling);
 			}
 		} catch (err) {
 			console.error('Got bad scroll event:', e);
@@ -1167,7 +1165,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 	 * Given a stable rendered state, checks every rendered element whether it needs
 	 * to be probed for dynamic height. Adjusts scroll height and top if necessary.
 	 */
-	private _rerender(renderTop: number, renderHeight: number, updateScrollTop: boolean = true): void {
+	private _rerender(renderTop: number, renderHeight: number, inSmoothScrolling?: boolean): void {
 		const previousRenderRange = this.getRenderRange(renderTop, renderHeight);
 
 		// Let's remember the second element's position, this helps in scrolling up
@@ -1233,8 +1231,15 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 					}
 				}
 
-				if (updateScrollTop && typeof anchorElementIndex === 'number') {
-					this.scrollTop = this.elementTop(anchorElementIndex) - anchorElementTopDelta!;
+				if (typeof anchorElementIndex === 'number') {
+					// To compute a destination scroll top, we need to take into account the current smooth scrolling
+					// animation, and then reuse it with a new target (to avoid prolonging the scroll)
+					// See https://github.com/microsoft/vscode/issues/104144
+					// See https://github.com/microsoft/vscode/pull/104284
+					// See https://github.com/microsoft/vscode/issues/107704
+					const deltaScrollTop = this.scrollable.getFutureScrollPosition().scrollTop - renderTop;
+					const newScrollTop = this.elementTop(anchorElementIndex) - anchorElementTopDelta! + deltaScrollTop;
+					this.setScrollTop(newScrollTop, inSmoothScrolling);
 				}
 
 				this._onDidChangeContentHeight.fire(this.contentHeight);

src/vs/base/browser/ui/scrollbar/scrollableElement.ts

@@ -549,8 +549,12 @@ export class SmoothScrollableElement extends AbstractScrollableElement {
 		super(element, options, scrollable);
 	}
 
-	public setScrollPosition(update: INewScrollPosition): void {
-		this._scrollable.setScrollPositionNow(update);
+	public setScrollPosition(update: INewScrollPosition & { reuseAnimation?: boolean }): void {
+		if (update.reuseAnimation) {
+			this._scrollable.setScrollPositionSmooth(update, update.reuseAnimation);
+		} else {
+			this._scrollable.setScrollPositionNow(update);
+		}
 	}
 
 	public getScrollPosition(): IScrollPosition {

src/vs/base/common/scrollable.ts

@@ -13,6 +13,8 @@ export const enum ScrollbarVisibility {
 }
 
 export interface ScrollEvent {
+	inSmoothScrolling: boolean;
+
 	oldWidth: number;
 	oldScrollWidth: number;
 	oldScrollLeft: number;
@@ -132,7 +134,7 @@ export class ScrollState implements IScrollDimensions, IScrollPosition {
 		);
 	}
 
-	public createScrollEvent(previous: ScrollState): ScrollEvent {
+	public createScrollEvent(previous: ScrollState, inSmoothScrolling: boolean): ScrollEvent {
 		const widthChanged = (this.width !== previous.width);
 		const scrollWidthChanged = (this.scrollWidth !== previous.scrollWidth);
 		const scrollLeftChanged = (this.scrollLeft !== previous.scrollLeft);
@@ -142,6 +144,7 @@ export class ScrollState implements IScrollDimensions, IScrollPosition {
 		const scrollTopChanged = (this.scrollTop !== previous.scrollTop);
 
 		return {
+			inSmoothScrolling: inSmoothScrolling,
 			oldWidth: previous.width,
 			oldScrollWidth: previous.scrollWidth,
 			oldScrollLeft: previous.scrollLeft,
@@ -242,7 +245,7 @@ export class Scrollable extends Disposable {
 
 	public setScrollDimensions(dimensions: INewScrollDimensions, useRawScrollPositions: boolean): void {
 		const newState = this._state.withScrollDimensions(dimensions, useRawScrollPositions);
-		this._setState(newState);
+		this._setState(newState, Boolean(this._smoothScrolling));
 
 		// Validate outstanding animated scroll position target
 		if (this._smoothScrolling) {
@@ -279,10 +282,10 @@ export class Scrollable extends Disposable {
 			this._smoothScrolling = null;
 		}
 
-		this._setState(newState);
+		this._setState(newState, false);
 	}
 
-	public setScrollPositionSmooth(update: INewScrollPosition): void {
+	public setScrollPositionSmooth(update: INewScrollPosition, reuseAnimation?: boolean): void {
 		if (this._smoothScrollDuration === 0) {
 			// Smooth scrolling not supported.
 			return this.setScrollPositionNow(update);
@@ -302,8 +305,12 @@ export class Scrollable extends Disposable {
 				// No need to interrupt or extend the current animation since we're going to the same place
 				return;
 			}
-
-			const newSmoothScrolling = this._smoothScrolling.combine(this._state, validTarget, this._smoothScrollDuration);
+			let newSmoothScrolling: SmoothScrollingOperation;
+			if (reuseAnimation) {
+				newSmoothScrolling = new SmoothScrollingOperation(this._smoothScrolling.from, validTarget, this._smoothScrolling.startTime, this._smoothScrolling.duration);
+			} else {
+				newSmoothScrolling = this._smoothScrolling.combine(this._state, validTarget, this._smoothScrollDuration);
+			}
 			this._smoothScrolling.dispose();
 			this._smoothScrolling = newSmoothScrolling;
 		} else {
@@ -330,7 +337,7 @@ export class Scrollable extends Disposable {
 		const update = this._smoothScrolling.tick();
 		const newState = this._state.withScrollPosition(update);
 
-		this._setState(newState);
+		this._setState(newState, true);
 
 		if (!this._smoothScrolling) {
 			// Looks like someone canceled the smooth scrolling
@@ -354,14 +361,14 @@ export class Scrollable extends Disposable {
 		});
 	}
 
-	private _setState(newState: ScrollState): void {
+	private _setState(newState: ScrollState, inSmoothScrolling: boolean): void {
 		const oldState = this._state;
 		if (oldState.equals(newState)) {
 			// no change
 			return;
 		}
 		this._state = newState;
-		this._onScroll.fire(this._state.createScrollEvent(oldState));
+		this._onScroll.fire(this._state.createScrollEvent(oldState, inSmoothScrolling));
 	}
 }
 
@@ -404,17 +411,17 @@ export class SmoothScrollingOperation {
 	public readonly from: ISmoothScrollPosition;
 	public to: ISmoothScrollPosition;
 	public readonly duration: number;
-	private readonly _startTime: number;
+	public readonly startTime: number;
 	public animationFrameDisposable: IDisposable | null;
 
 	private scrollLeft!: IAnimation;
 	private scrollTop!: IAnimation;
 
-	protected constructor(from: ISmoothScrollPosition, to: ISmoothScrollPosition, startTime: number, duration: number) {
+	constructor(from: ISmoothScrollPosition, to: ISmoothScrollPosition, startTime: number, duration: number) {
 		this.from = from;
 		this.to = to;
 		this.duration = duration;
-		this._startTime = startTime;
+		this.startTime = startTime;
 
 		this.animationFrameDisposable = null;
 
@@ -460,7 +467,7 @@ export class SmoothScrollingOperation {
 	}
 
 	protected _tick(now: number): SmoothScrollingUpdate {
-		const completion = (now - this._startTime) / this.duration;
+		const completion = (now - this.startTime) / this.duration;
 
 		if (completion < 1) {
 			const newScrollLeft = this.scrollLeft(completion);