From 7477ed0bf62069eeab37f85517b33c260e31e2bd Mon Sep 17 00:00:00 2001 From: abe33 Date: Mon, 14 Mar 2016 23:34:03 +0100 Subject: [PATCH] Implement asynchronous animated scrolling When both `scrollAnimation` and `independentMinimapScroll` settings are enabled, the animation of the minimap no longer follow the animation of the editor, preventing the minimap from jumping to the starting editor scroll before moving towards the end scrolling position. --- lib/minimap-element.js | 42 +++++++++++++++------- lib/minimap.js | 11 ++++-- spec/minimap-element-spec.js | 70 ++++++++++++++++++++++++++++++------ 3 files changed, 97 insertions(+), 26 deletions(-) diff --git a/lib/minimap-element.js b/lib/minimap-element.js index 1112764a..6883e06c 100644 --- a/lib/minimap-element.js +++ b/lib/minimap-element.js @@ -980,19 +980,34 @@ export default class MinimapElement { * @access private */ canvasLeftMousePressed (y) { - let deltaY = y - this.getBoundingClientRect().top - let row = Math.floor(deltaY / this.minimap.getLineHeight()) + this.minimap.getFirstVisibleScreenRow() + const deltaY = y - this.getBoundingClientRect().top + const row = Math.floor(deltaY / this.minimap.getLineHeight()) + this.minimap.getFirstVisibleScreenRow() - let textEditor = this.minimap.getTextEditor() + const textEditor = this.minimap.getTextEditor() - let scrollTop = row * textEditor.getLineHeightInPixels() - this.minimap.getTextEditorHeight() / 2 + const scrollTop = row * textEditor.getLineHeightInPixels() - this.minimap.getTextEditorHeight() / 2 if (atom.config.get('minimap.scrollAnimation')) { + const duration = atom.config.get('minimap.scrollAnimationDuration') + const independentScroll = this.minimap.scrollIndependentlyOnMouseWheel() + let from = this.minimap.getTextEditorScrollTop() let to = scrollTop - let step = (now) => this.minimap.setTextEditorScrollTop(now) - let duration = atom.config.get('minimap.scrollAnimationDuration') - this.animate({from: from, to: to, duration: duration, step: step}) + let step + + if (independentScroll) { + const minimapFrom = this.minimap.getScrollTop() + const minimapTo = Math.min(1, scrollTop / (this.minimap.getTextEditorMaxScrollTop() || 1)) * this.minimap.getMaxScrollTop() + + step = (now, t) => { + this.minimap.setTextEditorScrollTop(now, true) + this.minimap.setScrollTop(minimapFrom + (minimapTo - minimapFrom) * t) + } + this.animate({from: from, to: to, duration: duration, step: step}) + } else { + step = (now) => this.minimap.setTextEditorScrollTop(now) + this.animate({from: from, to: to, duration: duration, step: step}) + } } else { this.minimap.setTextEditorScrollTop(scrollTop) } @@ -1255,25 +1270,26 @@ export default class MinimapElement { * @access private */ animate ({from, to, duration, step}) { + const start = this.getTime() let progress - let start = this.getTime() - let swing = function (progress) { + const swing = function (progress) { return 0.5 - Math.cos(progress * Math.PI) / 2 } - let update = () => { + const update = () => { if (!this.minimap) { return } - let passed = this.getTime() - start + const passed = this.getTime() - start if (duration === 0) { progress = 1 } else { progress = passed / duration } if (progress > 1) { progress = 1 } - let delta = swing(progress) - step(from + (to - from) * delta) + const delta = swing(progress) + const value = from + (to - from) * delta + step(value, delta) if (progress < 1) { requestAnimationFrame(update) } } diff --git a/lib/minimap.js b/lib/minimap.js index b5a3e217..eb986d38 100644 --- a/lib/minimap.js +++ b/lib/minimap.js @@ -205,10 +205,14 @@ export default class Minimap { })) subs.add(this.adapter.onDidChangeScrollTop(() => { - if (!this.standAlone) { + if (!this.standAlone && !this.ignoreTextEditorScroll) { this.updateScrollTop() this.emitter.emit('did-change-scroll-top', this) } + + if (this.ignoreTextEditorScroll) { + this.ignoreTextEditorScroll = false + } })) subs.add(this.adapter.onDidChangeScrollLeft(() => { if (!this.standAlone) { @@ -478,7 +482,10 @@ export default class Minimap { * * @param {number} scrollTop the new scroll top value */ - setTextEditorScrollTop (scrollTop) { this.adapter.setScrollTop(scrollTop) } + setTextEditorScrollTop (scrollTop, ignoreTextEditorScroll = false) { + this.ignoreTextEditorScroll = ignoreTextEditorScroll + this.adapter.setScrollTop(scrollTop) + } /** * Returns the `TextEditor` scroll left value. diff --git a/spec/minimap-element-spec.js b/spec/minimap-element-spec.js index f86e568f..0e5e502b 100644 --- a/spec/minimap-element-spec.js +++ b/spec/minimap-element-spec.js @@ -660,6 +660,8 @@ describe('MinimapElement', () => { }) describe('pressing the mouse on the minimap canvas (without scroll animation)', () => { + let canvas + beforeEach(() => { let t = 0 spyOn(minimapElement, 'getTime').andCallFake(() => { @@ -672,15 +674,29 @@ describe('MinimapElement', () => { atom.config.set('minimap.scrollAnimation', false) canvas = minimapElement.getFrontCanvas() - mousedown(canvas) }) it('scrolls the editor to the line below the mouse', () => { - expect(editorElement.getScrollTop()).toBeGreaterThan(380) + mousedown(canvas) + expect(editorElement.getScrollTop()).toBeCloseTo(480) + }) + + describe('when independentMinimapScroll setting is enabled', () => { + beforeEach(() => { + minimap.setScrollTop(1000) + atom.config.set('minimap.independentMinimapScroll', true) + }) + + it('scrolls the editor to the line below the mouse', () => { + mousedown(canvas) + expect(editorElement.getScrollTop()).toBeCloseTo(480) + }) }) }) describe('pressing the mouse on the minimap canvas (with scroll animation)', () => { + let canvas + beforeEach(() => { let t = 0 spyOn(minimapElement, 'getTime').andCallFake(() => { @@ -694,27 +710,59 @@ describe('MinimapElement', () => { atom.config.set('minimap.scrollAnimationDuration', 300) canvas = minimapElement.getFrontCanvas() - mousedown(canvas) - - waitsFor(() => { return nextAnimationFrame !== noAnimationFrame }) }) it('scrolls the editor gradually to the line below the mouse', () => { + mousedown(canvas) + waitsFor(() => { return nextAnimationFrame !== noAnimationFrame }) // wait until all animations run out waitsFor(() => { - // Should be 400 on stable and 480 on beta. - // I'm still looking for a reason. nextAnimationFrame !== noAnimationFrame && nextAnimationFrame() - return editorElement.getScrollTop() >= 380 + return editorElement.getScrollTop() >= 480 }) }) it('stops the animation if the text editor is destroyed', () => { - editor.destroy() + mousedown(canvas) + waitsFor(() => { return nextAnimationFrame !== noAnimationFrame }) - nextAnimationFrame !== noAnimationFrame && nextAnimationFrame() + runs(() => { + editor.destroy() - expect(nextAnimationFrame === noAnimationFrame) + nextAnimationFrame !== noAnimationFrame && nextAnimationFrame() + + expect(nextAnimationFrame === noAnimationFrame) + }) + }) + + describe('when independentMinimapScroll setting is enabled', () => { + beforeEach(() => { + minimap.setScrollTop(1000) + atom.config.set('minimap.independentMinimapScroll', true) + }) + + it('scrolls the editor gradually to the line below the mouse', () => { + mousedown(canvas) + waitsFor(() => { return nextAnimationFrame !== noAnimationFrame }) + // wait until all animations run out + waitsFor(() => { + nextAnimationFrame !== noAnimationFrame && nextAnimationFrame() + return editorElement.getScrollTop() >= 480 + }) + }) + + it('stops the animation if the text editor is destroyed', () => { + mousedown(canvas) + waitsFor(() => { return nextAnimationFrame !== noAnimationFrame }) + + runs(() => { + editor.destroy() + + nextAnimationFrame !== noAnimationFrame && nextAnimationFrame() + + expect(nextAnimationFrame === noAnimationFrame) + }) + }) }) })