From 37b5d2eb4dbbf5a23baf70c0ee1994c77e6dc05b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 21 Apr 2017 16:54:59 +0200 Subject: [PATCH] Restore scrollbar positions correctly on reload --- spec/text-editor-component-spec.js | 7 ++++++- src/text-editor-component.js | 32 ++++++++++++++---------------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 0588aafa0..e5ec8d98a 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -212,10 +212,13 @@ describe('TextEditorComponent', () => { }) - it('updates the bottom/right of dummy scrollbars and client height/width measurements when scrollbar styles change', async () => { + it('updates the bottom/right of dummy scrollbars and client height/width measurements without forgetting the previous scroll top/left when scrollbar styles change', async () => { const {component, element, editor} = buildComponent({height: 100, width: 100}) expect(getHorizontalScrollbarHeight(component)).toBeGreaterThan(10) expect(getVerticalScrollbarWidth(component)).toBeGreaterThan(10) + setScrollTop(component, 20) + setScrollLeft(component, 10) + await component.getNextUpdatePromise() const style = document.createElement('style') style.textContent = '::-webkit-scrollbar { height: 10px; width: 10px; }' @@ -228,6 +231,8 @@ describe('TextEditorComponent', () => { expect(getVerticalScrollbarWidth(component)).toBe(10) expect(component.refs.horizontalScrollbar.element.style.right).toBe('10px') expect(component.refs.verticalScrollbar.element.style.bottom).toBe('10px') + expect(component.refs.horizontalScrollbar.element.scrollLeft).toBe(10) + expect(component.refs.verticalScrollbar.element.scrollTop).toBe(20) expect(component.getScrollContainerClientHeight()).toBe(100 - 10) expect(component.getScrollContainerClientWidth()).toBe(100 - component.getGutterContainerWidth() - 10) }) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index d13259b96..bde57efaf 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -339,6 +339,14 @@ class TextEditorComponent { this.scrollTopPending = false this.scrollLeftPending = false if (this.remeasureScrollbars) { + // Flush stored scroll positions to the vertical and the horizontal + // scrollbars. This is because they have just been destroyed and recreated + // as a result of their remeasurement, but we could not assign the scroll + // top while they were initialized because they were not attached to the + // DOM yet. + this.refs.verticalScrollbar.flushScrollPosition() + this.refs.horizontalScrollbar.flushScrollPosition() + this.measureScrollbarDimensions() this.remeasureScrollbars = false etch.updateSync(this) @@ -2585,31 +2593,21 @@ class DummyScrollbarComponent { constructor (props) { this.props = props etch.initialize(this) - if (this.props.orientation === 'horizontal') { - this.element.scrollLeft = this.props.scrollLeft - } else { - this.element.scrollTop = this.props.scrollTop - } } update (newProps) { const oldProps = this.props this.props = newProps etch.updateSync(this) - if (this.props.orientation === 'horizontal') { - if (newProps.scrollLeft !== oldProps.scrollLeft) { - this.element.scrollLeft = this.props.scrollLeft - } - } else { - if (newProps.scrollTop !== oldProps.scrollTop) { - this.element.scrollTop = this.props.scrollTop - } - } + + const shouldFlushScrollPosition = ( + newProps.scrollTop !== oldProps.scrollTop || + newProps.scrollLeft !== oldProps.scrollLeft + ) + if (shouldFlushScrollPosition) this.flushScrollPosition() } - // Scroll position must be updated after the inner element is updated to - // ensure the element has an adequate scrollHeight/scrollWidth - updateScrollPosition () { + flushScrollPosition () { if (this.props.orientation === 'horizontal') { this.element.scrollLeft = this.props.scrollLeft } else {