Fixes #6710: Correctly model the case where the slider is artificially inflated in size

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

@@ -54,23 +54,13 @@ export class ScrollbarState {
 	 * `visibleSize` - `oppositeScrollbarSize`
 	 */
 	private _computedAvailableSize: number;
-
-	/**
-	 * `computedAvailableSize` - 2 * `arrowSize`
-	 */
-	private _computedRepresentableSize: number;
-
 	/**
-	 * `computedRepresentableSize` / `scrollSize`
-	 */
-	private _computedRatio: number;
-
-	/**
-	 * (`scrollSize` > `visibleSize`)
+	 * (`scrollSize` > 0 && `scrollSize` > `visibleSize`)
 	 */
 	private _computedIsNeeded: boolean;
 
 	private _computedSliderSize: number;
+	private _computedSliderRatio: number;
 	private _computedSliderPosition: number;
 
 	constructor(arrowSize: number, scrollbarSize: number, oppositeScrollbarSize: number) {
@@ -83,10 +73,9 @@ export class ScrollbarState {
 		this._scrollPosition = 0;
 
 		this._computedAvailableSize = 0;
-		this._computedRepresentableSize = 0;
-		this._computedRatio = 0.1;
 		this._computedIsNeeded = false;
 		this._computedSliderSize = 0;
+		this._computedSliderRatio = 0;
 		this._computedSliderPosition = 0;
 
 		this._refreshComputedValues();
@@ -130,56 +119,46 @@ export class ScrollbarState {
 		return false;
 	}
 
-	private _refreshComputedValues(): void {
-		const oppositeScrollbarSize = this._oppositeScrollbarSize;
-		const arrowSize = this._arrowSize;
-		const visibleSize = this._visibleSize;
-		const scrollSize = this._scrollSize;
-		const scrollPosition = this._scrollPosition;
-
-		let computedAvailableSize = Math.max(0, visibleSize - oppositeScrollbarSize);
-		let computedRepresentableSize = Math.max(0, computedAvailableSize - 2 * arrowSize);
-		let computedRatio = scrollSize > 0 ? (computedRepresentableSize / scrollSize) : 0;
-		let computedIsNeeded = (scrollSize > visibleSize);
-
-		let computedSliderSize: number;
-		let computedSliderPosition: number;
+	private static _computeValues(oppositeScrollbarSize: number, arrowSize: number, visibleSize: number, scrollSize: number, scrollPosition: number) {
+		const computedAvailableSize = Math.max(0, visibleSize - oppositeScrollbarSize);
+		const computedRepresentableSize = Math.max(0, computedAvailableSize - 2 * arrowSize);
+		const computedIsNeeded = (scrollSize > 0 && scrollSize > visibleSize);
 
 		if (!computedIsNeeded) {
-			computedSliderSize = computedRepresentableSize;
-			computedSliderPosition = 0;
-		} else {
-			computedSliderSize = Math.floor(visibleSize * computedRatio);
-			computedSliderPosition = Math.floor(scrollPosition * computedRatio);
-
-			if (computedSliderSize < MINIMUM_SLIDER_SIZE) {
-				// We must artificially increase the size of the slider, since the slider would be too small otherwise
-				// The effort is to keep the slider centered around the original position, but we must take into
-				// account the cases when the slider is too close to the top or too close to the bottom
-
-				let sliderArtificialOffset = (MINIMUM_SLIDER_SIZE - computedSliderSize) / 2;
-				computedSliderSize = MINIMUM_SLIDER_SIZE;
-
-				computedSliderPosition -= sliderArtificialOffset;
-
-				if (computedSliderPosition + computedSliderSize > computedRepresentableSize) {
-					// Slider is too close to the bottom, so we glue it to the bottom
-					computedSliderPosition = computedRepresentableSize - computedSliderSize;
-				}
-
-				if (computedSliderPosition < 0) {
-					// Slider is too close to the top, so we glue it to the top
-					computedSliderPosition = 0;
-				}
-			}
+			// There is no need for a slider
+			return {
+				computedAvailableSize: Math.round(computedAvailableSize),
+				computedIsNeeded: computedIsNeeded,
+				computedSliderSize: Math.round(computedRepresentableSize),
+				computedSliderRatio: 0,
+				computedSliderPosition: 0,
+			};
 		}
 
-		this._computedAvailableSize = Math.round(computedAvailableSize);
-		this._computedRepresentableSize = Math.round(computedRepresentableSize);
-		this._computedRatio = computedRatio;
-		this._computedIsNeeded = computedIsNeeded;
-		this._computedSliderSize = Math.round(computedSliderSize);
-		this._computedSliderPosition = Math.round(computedSliderPosition);
+		// We must artificially increase the size of the slider if needed, since the slider would be too small to grab with the mouse otherwise
+		const computedSliderSize = Math.round(Math.max(MINIMUM_SLIDER_SIZE, Math.floor(visibleSize * computedRepresentableSize / scrollSize)));
+
+		// The slider can move from 0 to `computedRepresentableSize` - `computedSliderSize`
+		// in the same way `scrollPosition` can move from 0 to `scrollSize` - `visibleSize`.
+		const computedSliderRatio = (computedRepresentableSize - computedSliderSize) / (scrollSize - visibleSize);
+		const computedSliderPosition = (scrollPosition * computedSliderRatio);
+
+		return {
+			computedAvailableSize: Math.round(computedAvailableSize),
+			computedIsNeeded: computedIsNeeded,
+			computedSliderSize: Math.round(computedSliderSize),
+			computedSliderRatio: computedSliderRatio,
+			computedSliderPosition: Math.round(computedSliderPosition),
+		};
+	}
+
+	private _refreshComputedValues(): void {
+		const r = ScrollbarState._computeValues(this._oppositeScrollbarSize, this._arrowSize, this._visibleSize, this._scrollSize, this._scrollPosition);
+		this._computedAvailableSize = r.computedAvailableSize;
+		this._computedIsNeeded = r.computedIsNeeded;
+		this._computedSliderSize = r.computedSliderSize;
+		this._computedSliderRatio = r.computedSliderRatio;
+		this._computedSliderPosition = r.computedSliderPosition;
 	}
 
 	public getArrowSize(): number {
@@ -214,24 +193,30 @@ export class ScrollbarState {
 		return (this._computedSliderPosition + this._computedSliderSize / 2);
 	}
 
-	private _convertSliderPositionToScrollPosition(desiredSliderPosition: number): number {
-		return desiredSliderPosition / this._computedRatio;
-	}
-
 	/**
 	 * Compute a desired `scrollPosition` such that `offset` ends up in the center of the slider.
 	 * `offset` is based on the same coordinate system as the `sliderPosition`.
 	 */
 	public getDesiredScrollPositionFromOffset(offset: number): number {
+		if (!this._computedIsNeeded) {
+			// no need for a slider
+			return 0;
+		}
+
 		let desiredSliderPosition = offset - this._arrowSize - this._computedSliderSize / 2;
-		return this._convertSliderPositionToScrollPosition(desiredSliderPosition);
+		return Math.round(desiredSliderPosition / this._computedSliderRatio);
 	}
 
 	/**
 	 * Compute a desired `scrollPosition` such that the slider moves by `delta`.
 	 */
 	public getDesiredScrollPositionFromDelta(delta: number): number {
+		if (!this._computedIsNeeded) {
+			// no need for a slider
+			return 0;
+		}
+
 		let desiredSliderPosition = this._computedSliderPosition + delta;
-		return this._convertSliderPositionToScrollPosition(desiredSliderPosition);
+		return Math.round(desiredSliderPosition / this._computedSliderRatio);
 	}
 }

src/vs/base/test/browser/ui/scrollbar/scrollbarState.test.ts

@@ -15,11 +15,24 @@ suite('ScrollbarState', () => {
 		actual.setScrollPosition(32787);
 
 		assert.equal(actual.getArrowSize(), 0);
+		assert.equal(actual.getScrollPosition(), 32787);
 		assert.equal(actual.getRectangleLargeSize(), 339);
 		assert.equal(actual.getRectangleSmallSize(), 14);
 		assert.equal(actual.isNeeded(), true);
 		assert.equal(actual.getSliderSize(), 20);
-		assert.equal(actual.getSliderPosition(), 252);
-		assert.equal(actual.getSliderCenter(), 262);
+		assert.equal(actual.getSliderPosition(), 249);
+		assert.equal(actual.getSliderCenter(), 259);
+
+
+		assert.equal(actual.getDesiredScrollPositionFromOffset(259), 32849);
+		actual.setScrollPosition(32849);
+		assert.equal(actual.getArrowSize(), 0);
+		assert.equal(actual.getScrollPosition(), 32849);
+		assert.equal(actual.getRectangleLargeSize(), 339);
+		assert.equal(actual.getRectangleSmallSize(), 14);
+		assert.equal(actual.isNeeded(), true);
+		assert.equal(actual.getSliderSize(), 20);
+		assert.equal(actual.getSliderPosition(), 249);
+		assert.equal(actual.getSliderCenter(), 259);
 	});
 });