From 931acdf65b0171ba4f52fdc874fd3a4ec217d2a5 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Sat, 4 Feb 2017 13:41:37 +0100 Subject: [PATCH] Introduce and adopt ScrollState --- src/vs/base/browser/ui/list/listView.ts | 6 +- .../ui/scrollbar/horizontalScrollbar.ts | 3 +- .../browser/ui/scrollbar/scrollableElement.ts | 32 +-- .../browser/ui/scrollbar/verticalScrollbar.ts | 3 +- src/vs/base/common/scrollable.ts | 196 ++++++++++-------- src/vs/base/parts/tree/browser/treeView.ts | 6 +- .../browser/viewLayout/layoutProvider.ts | 32 +-- .../browser/viewLayout/scrollManager.ts | 5 +- .../extensions/browser/extensionEditor.ts | 3 +- .../electron-browser/walkThroughPart.ts | 34 +-- 10 files changed, 174 insertions(+), 146 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index f7d533ee787..eb42ed73028 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -133,7 +133,8 @@ export class ListView implements IDisposable { } get renderHeight(): number { - return this.scrollableElement.getHeight(); + const scrollState = this.scrollableElement.getScrollState(); + return scrollState.height; } element(index: number): T { @@ -209,7 +210,8 @@ export class ListView implements IDisposable { } getScrollTop(): number { - return this.scrollableElement.getScrollTop(); + const scrollState = this.scrollableElement.getScrollState(); + return scrollState.scrollTop; } setScrollTop(scrollTop: number): void { diff --git a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts index ea214d1e905..bb0d255d2de 100644 --- a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts @@ -97,7 +97,8 @@ export class HorizontalScrollbar extends AbstractScrollbar { } protected _getScrollPosition(): number { - return this._scrollable.getScrollLeft(); + const scrollState = this._scrollable.getState(); + return scrollState.scrollLeft; } protected _setScrollPosition(scrollPosition: number) { diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 8d59e014e29..9802e82b1c2 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -14,7 +14,7 @@ import { HorizontalScrollbar } from 'vs/base/browser/ui/scrollbar/horizontalScro import { VerticalScrollbar } from 'vs/base/browser/ui/scrollbar/verticalScrollbar'; import { ScrollableElementCreationOptions, ScrollableElementChangeOptions, ScrollableElementResolvedOptions } from 'vs/base/browser/ui/scrollbar/scrollableElementOptions'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -import { Scrollable, ScrollEvent, INewScrollState, ScrollbarVisibility } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollState, ScrollEvent, INewScrollState, ScrollbarVisibility } from 'vs/base/common/scrollable'; import { Widget } from 'vs/base/browser/ui/widget'; import { TimeoutTimer } from 'vs/base/common/async'; import { FastDomNode, createFastDomNode } from 'vs/base/browser/styleMutator'; @@ -149,24 +149,8 @@ export class ScrollableElement extends Widget { this._scrollable.updateState(newState); } - public getWidth(): number { - return this._scrollable.getWidth(); - } - public getScrollWidth(): number { - return this._scrollable.getScrollWidth(); - } - public getScrollLeft(): number { - return this._scrollable.getScrollLeft(); - } - - public getHeight(): number { - return this._scrollable.getHeight(); - } - public getScrollHeight(): number { - return this._scrollable.getScrollHeight(); - } - public getScrollTop(): number { - return this._scrollable.getScrollTop(); + public getScrollState(): ScrollState { + return this._scrollable.getState(); } /** @@ -263,15 +247,16 @@ export class ScrollableElement extends Widget { } } + const scrollState = this._scrollable.getState(); if (deltaY) { - let currentScrollTop = this._scrollable.getScrollTop(); + let currentScrollTop = scrollState.scrollTop; desiredScrollTop = this._verticalScrollbar.validateScrollPosition((desiredScrollTop !== -1 ? desiredScrollTop : currentScrollTop) - SCROLL_WHEEL_SENSITIVITY * deltaY); if (desiredScrollTop === currentScrollTop) { desiredScrollTop = -1; } } if (deltaX) { - let currentScrollLeft = this._scrollable.getScrollLeft(); + let currentScrollLeft = scrollState.scrollLeft; desiredScrollLeft = this._horizontalScrollbar.validateScrollPosition((desiredScrollLeft !== -1 ? desiredScrollLeft : currentScrollLeft) - SCROLL_WHEEL_SENSITIVITY * deltaX); if (desiredScrollLeft === currentScrollLeft) { desiredScrollLeft = -1; @@ -334,8 +319,9 @@ export class ScrollableElement extends Widget { this._verticalScrollbar.render(); if (this._options.useShadows) { - let enableTop = this._scrollable.getScrollTop() > 0; - let enableLeft = this._scrollable.getScrollLeft() > 0; + const scrollState = this._scrollable.getState(); + let enableTop = scrollState.scrollTop > 0; + let enableLeft = scrollState.scrollLeft > 0; this._leftShadowDomNode.setClassName('shadow' + (enableLeft ? ' left' : '')); this._topShadowDomNode.setClassName('shadow' + (enableTop ? ' top' : '')); diff --git a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts index 3fce9dbc8df..5a1ed5f16bd 100644 --- a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts @@ -98,7 +98,8 @@ export class VerticalScrollbar extends AbstractScrollbar { } protected _getScrollPosition(): number { - return this._scrollable.getScrollTop(); + const scrollState = this._scrollable.getState(); + return scrollState.scrollTop; } protected _setScrollPosition(scrollPosition: number): void { diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index 877c85f0a2b..f7d327627f0 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -31,71 +31,30 @@ export interface ScrollEvent { scrollTopChanged: boolean; } -export interface INewScrollState { - width?: number; - scrollWidth?: number; - scrollLeft?: number; - - height?: number; - scrollHeight?: number; - scrollTop?: number; -} - -export class Scrollable extends Disposable { - - _scrollableBrand: void; - - private _width: number; - private _scrollWidth: number; - private _scrollLeft: number; - - private _height: number; - private _scrollHeight: number; - private _scrollTop: number; - - private _onScroll = this._register(new Emitter()); - public onScroll: Event = this._onScroll.event; - - constructor() { - super(); - - this._width = 0; - this._scrollWidth = 0; - this._scrollLeft = 0; - - this._height = 0; - this._scrollHeight = 0; - this._scrollTop = 0; - } - - public getWidth(): number { - return this._width; - } - public getScrollWidth(): number { - return this._scrollWidth; - } - public getScrollLeft(): number { - return this._scrollLeft; - } - - public getHeight(): number { - return this._height; - } - public getScrollHeight(): number { - return this._scrollHeight; - } - public getScrollTop(): number { - return this._scrollTop; - } - - public updateState(newState: INewScrollState): void { - let width = (typeof newState.width !== 'undefined' ? newState.width | 0 : this._width); - let scrollWidth = (typeof newState.scrollWidth !== 'undefined' ? newState.scrollWidth | 0 : this._scrollWidth); - let scrollLeft = (typeof newState.scrollLeft !== 'undefined' ? newState.scrollLeft | 0 : this._scrollLeft); - - let height = (typeof newState.height !== 'undefined' ? newState.height | 0 : this._height); - let scrollHeight = (typeof newState.scrollHeight !== 'undefined' ? newState.scrollHeight | 0 : this._scrollHeight); - let scrollTop = (typeof newState.scrollTop !== 'undefined' ? newState.scrollTop | 0 : this._scrollTop); +export class ScrollState { + _scrollStateBrand: void; + + public readonly width: number; + public readonly scrollWidth: number; + public readonly scrollLeft: number; + public readonly height: number; + public readonly scrollHeight: number; + public readonly scrollTop: number; + + constructor( + width: number, + scrollWidth: number, + scrollLeft: number, + height: number, + scrollHeight: number, + scrollTop: number + ) { + width = width | 0; + scrollWidth = scrollWidth | 0; + scrollLeft = scrollLeft | 0; + height = height | 0; + scrollHeight = scrollHeight | 0; + scrollTop = scrollTop | 0; if (width < 0) { width = 0; @@ -117,34 +76,42 @@ export class Scrollable extends Disposable { scrollTop = 0; } - let widthChanged = (this._width !== width); - let scrollWidthChanged = (this._scrollWidth !== scrollWidth); - let scrollLeftChanged = (this._scrollLeft !== scrollLeft); - - let heightChanged = (this._height !== height); - let scrollHeightChanged = (this._scrollHeight !== scrollHeight); - let scrollTopChanged = (this._scrollTop !== scrollTop); + this.width = width; + this.scrollWidth = scrollWidth; + this.scrollLeft = scrollLeft; + this.height = height; + this.scrollHeight = scrollHeight; + this.scrollTop = scrollTop; + } - if (!widthChanged && !scrollWidthChanged && !scrollLeftChanged && !heightChanged && !scrollHeightChanged && !scrollTopChanged) { - return; - } + public equals(other: ScrollState): boolean { + return ( + this.width === other.width + && this.scrollWidth === other.scrollWidth + && this.scrollLeft === other.scrollLeft + && this.height === other.height + && this.scrollHeight === other.scrollHeight + && this.scrollTop === other.scrollTop + ); + } - this._width = width; - this._scrollWidth = scrollWidth; - this._scrollLeft = scrollLeft; + public createScrollEvent(previous: ScrollState): ScrollEvent { + let widthChanged = (this.width !== previous.width); + let scrollWidthChanged = (this.scrollWidth !== previous.scrollWidth); + let scrollLeftChanged = (this.scrollLeft !== previous.scrollLeft); - this._height = height; - this._scrollHeight = scrollHeight; - this._scrollTop = scrollTop; + let heightChanged = (this.height !== previous.height); + let scrollHeightChanged = (this.scrollHeight !== previous.scrollHeight); + let scrollTopChanged = (this.scrollTop !== previous.scrollTop); - this._onScroll.fire({ - width: this._width, - scrollWidth: this._scrollWidth, - scrollLeft: this._scrollLeft, + return { + width: this.width, + scrollWidth: this.scrollWidth, + scrollLeft: this.scrollLeft, - height: this._height, - scrollHeight: this._scrollHeight, - scrollTop: this._scrollTop, + height: this.height, + scrollHeight: this.scrollHeight, + scrollTop: this.scrollTop, widthChanged: widthChanged, scrollWidthChanged: scrollWidthChanged, @@ -153,6 +120,57 @@ export class Scrollable extends Disposable { heightChanged: heightChanged, scrollHeightChanged: scrollHeightChanged, scrollTopChanged: scrollTopChanged, - }); + }; + } + +} + +export interface INewScrollState { + width?: number; + scrollWidth?: number; + scrollLeft?: number; + + height?: number; + scrollHeight?: number; + scrollTop?: number; +} + +export class Scrollable extends Disposable { + + _scrollableBrand: void; + + private _state: ScrollState; + + private _onScroll = this._register(new Emitter()); + public onScroll: Event = this._onScroll.event; + + constructor() { + super(); + + this._state = new ScrollState(0, 0, 0, 0, 0, 0); + } + + public getState(): ScrollState { + return this._state; + } + + public updateState(update: INewScrollState): void { + const oldState = this._state; + const newState = new ScrollState( + (typeof update.width !== 'undefined' ? update.width : oldState.width), + (typeof update.scrollWidth !== 'undefined' ? update.scrollWidth : oldState.scrollWidth), + (typeof update.scrollLeft !== 'undefined' ? update.scrollLeft : oldState.scrollLeft), + (typeof update.height !== 'undefined' ? update.height : oldState.height), + (typeof update.scrollHeight !== 'undefined' ? update.scrollHeight : oldState.scrollHeight), + (typeof update.scrollTop !== 'undefined' ? update.scrollTop : oldState.scrollTop) + ); + + if (oldState.equals(newState)) { + // no change + return; + } + + this._state = newState; + this._onScroll.fire(this._state.createScrollEvent(oldState)); } } diff --git a/src/vs/base/parts/tree/browser/treeView.ts b/src/vs/base/parts/tree/browser/treeView.ts index e8b6fa09405..674b7883ba2 100644 --- a/src/vs/base/parts/tree/browser/treeView.ts +++ b/src/vs/base/parts/tree/browser/treeView.ts @@ -791,7 +791,8 @@ export class TreeView extends HeightMap { } public get viewHeight() { - return this.scrollableElement.getHeight(); + const scrollState = this.scrollableElement.getScrollState(); + return scrollState.height; } public set viewHeight(viewHeight: number) { @@ -802,7 +803,8 @@ export class TreeView extends HeightMap { } public get scrollTop(): number { - return this.scrollableElement.getScrollTop(); + const scrollState = this.scrollableElement.getScrollState(); + return scrollState.scrollTop; } public set scrollTop(scrollTop: number) { diff --git a/src/vs/editor/browser/viewLayout/layoutProvider.ts b/src/vs/editor/browser/viewLayout/layoutProvider.ts index 4b8e00981e6..afc17a7dcce 100644 --- a/src/vs/editor/browser/viewLayout/layoutProvider.ts +++ b/src/vs/editor/browser/viewLayout/layoutProvider.ts @@ -194,11 +194,12 @@ export class LayoutProvider extends ViewEventHandler implements IDisposable, ILa // ---- Layouting logic public getCurrentViewport(): editorCommon.Viewport { + const scrollState = this._scrollable.getState(); return new editorCommon.Viewport( - this._scrollable.getScrollTop(), - this._scrollable.getScrollLeft(), - this._scrollable.getWidth(), - this._scrollable.getHeight() + scrollState.scrollTop, + scrollState.scrollLeft, + scrollState.width, + scrollState.height ); } @@ -231,13 +232,14 @@ export class LayoutProvider extends ViewEventHandler implements IDisposable, ILa // ---- view state public saveState(): editorCommon.IViewState { - let scrollTop = this._scrollable.getScrollTop(); + const scrollState = this._scrollable.getState(); + let scrollTop = scrollState.scrollTop; let firstLineNumberInViewport = this._linesLayout.getLineNumberAtOrAfterVerticalOffset(scrollTop); let whitespaceAboveFirstLine = this._linesLayout.getWhitespaceAccumulatedHeightBeforeLineNumber(firstLineNumberInViewport); return { scrollTop: scrollTop, scrollTopWithoutViewZones: scrollTop - whitespaceAboveFirstLine, - scrollLeft: this._scrollable.getScrollLeft() + scrollLeft: scrollState.scrollLeft }; } @@ -295,8 +297,9 @@ export class LayoutProvider extends ViewEventHandler implements IDisposable, ILa } public getTotalHeight(): number { + const scrollState = this._scrollable.getState(); let reserveHorizontalScrollbarHeight = 0; - if (this._scrollable.getScrollWidth() > this._scrollable.getWidth()) { + if (scrollState.scrollWidth > scrollState.width) { if (this._configuration.editor.viewInfo.scrollbar.horizontal !== ScrollbarVisibility.Hidden) { reserveHorizontalScrollbarHeight = this._configuration.editor.viewInfo.scrollbar.horizontalScrollbarSize; } @@ -322,23 +325,28 @@ export class LayoutProvider extends ViewEventHandler implements IDisposable, ILa public getScrollWidth(): number { - return this._scrollable.getScrollWidth(); + const scrollState = this._scrollable.getState(); + return scrollState.scrollWidth; } public getScrollLeft(): number { - return this._scrollable.getScrollLeft(); + const scrollState = this._scrollable.getState(); + return scrollState.scrollLeft; } public getScrollHeight(): number { - return this._scrollable.getScrollHeight(); + const scrollState = this._scrollable.getState(); + return scrollState.scrollHeight; } public getScrollTop(): number { - return this._scrollable.getScrollTop(); + const scrollState = this._scrollable.getState(); + return scrollState.scrollTop; } public setScrollPosition(position: editorCommon.INewScrollPosition): void { this._scrollable.updateState(position); } public getScrolledTopFromAbsoluteTop(top: number): number { - return top - this._scrollable.getScrollTop(); + const scrollState = this._scrollable.getState(); + return top - scrollState.scrollTop; } public getOverviewRulerInsertData(): { parent: HTMLElement; insertBefore: HTMLElement; } { diff --git a/src/vs/editor/browser/viewLayout/scrollManager.ts b/src/vs/editor/browser/viewLayout/scrollManager.ts index 35da7d55a34..c8d1b0751d9 100644 --- a/src/vs/editor/browser/viewLayout/scrollManager.ts +++ b/src/vs/editor/browser/viewLayout/scrollManager.ts @@ -84,12 +84,13 @@ export class EditorScrollbar implements IDisposable { // changing the .scrollTop of this.linesContent let onBrowserDesperateReveal = (domNode: HTMLElement, lookAtScrollTop: boolean, lookAtScrollLeft: boolean) => { + const scrollState = this.scrollable.getState(); let newScrollPosition: INewScrollPosition = {}; if (lookAtScrollTop) { let deltaTop = domNode.scrollTop; if (deltaTop) { - newScrollPosition.scrollTop = this.scrollable.getScrollTop() + deltaTop; + newScrollPosition.scrollTop = scrollState.scrollTop + deltaTop; domNode.scrollTop = 0; } } @@ -97,7 +98,7 @@ export class EditorScrollbar implements IDisposable { if (lookAtScrollLeft) { let deltaLeft = domNode.scrollLeft; if (deltaLeft) { - newScrollPosition.scrollLeft = this.scrollable.getScrollLeft() + deltaLeft; + newScrollPosition.scrollLeft = scrollState.scrollLeft + deltaLeft; domNode.scrollLeft = 0; } } diff --git a/src/vs/workbench/parts/extensions/browser/extensionEditor.ts b/src/vs/workbench/parts/extensions/browser/extensionEditor.ts index afce75b9404..b12860c6bfe 100644 --- a/src/vs/workbench/parts/extensions/browser/extensionEditor.ts +++ b/src/vs/workbench/parts/extensions/browser/extensionEditor.ts @@ -391,7 +391,8 @@ export class ExtensionEditor extends BaseEditor { const tree = ExtensionEditor.renderDependencies(content, extensionDependencies, this.instantiationService); const layout = () => { scrollableContent.scanDomNode(); - tree.layout(scrollableContent.getHeight()); + const scrollState = scrollableContent.getScrollState(); + tree.layout(scrollState.height); }; const removeLayoutParticipant = arrays.insert(this.layoutParticipants, { layout }); this.contentDisposables.push(toDisposable(removeLayoutParticipant)); diff --git a/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts b/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts index cf7eb5a8492..86e55d20ca0 100644 --- a/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts +++ b/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts @@ -107,10 +107,11 @@ export class WalkThroughPart extends BaseEditor { } private updatedScrollPosition() { - const scrollHeight = this.scrollbar.getScrollHeight(); + const scrollState = this.scrollbar.getScrollState(); + const scrollHeight = scrollState.scrollHeight; if (scrollHeight && this.input instanceof WalkThroughInput) { - const scrollTop = this.scrollbar.getScrollTop(); - const height = this.scrollbar.getHeight(); + const scrollTop = scrollState.scrollTop; + const height = scrollState.height; this.input.relativeScrollPosition(scrollTop / scrollHeight, (scrollTop + height) / scrollHeight); } } @@ -135,8 +136,9 @@ export class WalkThroughPart extends BaseEditor { this.disposables.push(this.addEventListener(this.content, 'focusin', e => { // Work around scrolling as side-effect of setting focus on the offscreen zone widget (#18929) if (e.target instanceof HTMLElement && e.target.classList.contains('zone-widget-container')) { - this.content.scrollTop = this.scrollbar.getScrollTop(); - this.content.scrollLeft = this.scrollbar.getScrollLeft(); + let scrollState = this.scrollbar.getScrollState(); + this.content.scrollTop = scrollState.scrollTop; + this.content.scrollLeft = scrollState.scrollLeft; } })); } @@ -228,11 +230,13 @@ export class WalkThroughPart extends BaseEditor { } arrowUp() { - this.scrollbar.updateState({ scrollTop: this.scrollbar.getScrollTop() - this.getArrowScrollHeight() }); + const scrollState = this.scrollbar.getScrollState(); + this.scrollbar.updateState({ scrollTop: scrollState.scrollTop - this.getArrowScrollHeight() }); } arrowDown() { - this.scrollbar.updateState({ scrollTop: this.scrollbar.getScrollTop() + this.getArrowScrollHeight() }); + const scrollState = this.scrollbar.getScrollState(); + this.scrollbar.updateState({ scrollTop: scrollState.scrollTop + this.getArrowScrollHeight() }); } private getArrowScrollHeight() { @@ -244,11 +248,13 @@ export class WalkThroughPart extends BaseEditor { } pageUp() { - this.scrollbar.updateState({ scrollTop: this.scrollbar.getScrollTop() - this.scrollbar.getHeight() }); + const scrollState = this.scrollbar.getScrollState(); + this.scrollbar.updateState({ scrollTop: scrollState.scrollTop - scrollState.height }); } pageDown() { - this.scrollbar.updateState({ scrollTop: this.scrollbar.getScrollTop() + this.scrollbar.getHeight() }); + const scrollState = this.scrollbar.getScrollState(); + this.scrollbar.updateState({ scrollTop: scrollState.scrollTop + scrollState.height }); } setInput(input: WalkThroughInput, options: EditorOptions): TPromise { @@ -327,8 +333,9 @@ export class WalkThroughPart extends BaseEditor { const lineHeight = editor.getConfiguration().lineHeight; const lineTop = (targetTop + (e.position.lineNumber - 1) * lineHeight) - containerTop; const lineBottom = lineTop + lineHeight; - const scrollTop = this.scrollbar.getScrollTop(); - const height = this.scrollbar.getHeight(); + const scrollState = this.scrollbar.getScrollState(); + const scrollTop = scrollState.scrollTop; + const height = scrollState.height; if (scrollTop > lineTop) { this.scrollbar.updateState({ scrollTop: lineTop }); } else if (scrollTop < lineBottom - height) { @@ -428,10 +435,11 @@ export class WalkThroughPart extends BaseEditor { memento[WALK_THROUGH_EDITOR_VIEW_STATE_PREFERENCE_KEY] = editorViewStateMemento; } + const scrollState = this.scrollbar.getScrollState(); const editorViewState: IWalkThroughEditorViewState = { viewState: { - scrollTop: this.scrollbar.getScrollTop(), - scrollLeft: this.scrollbar.getScrollLeft() + scrollTop: scrollState.scrollTop, + scrollLeft: scrollState.scrollLeft } }; -- GitLab