From 400a42f96a55496985d8517f7daa2372c2689932 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Mon, 9 Sep 2019 21:03:24 -0700 Subject: [PATCH 01/24] Recursively check document height before scrollTo --- addon/index.js | 34 +++++++++++++++++++++++++++++---- addon/services/router-scroll.js | 14 ++++++++++++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/addon/index.js b/addon/index.js index a44a5ce7..6c517e6a 100644 --- a/addon/index.js +++ b/addon/index.js @@ -6,6 +6,34 @@ import { scheduleOnce } from '@ember/runloop'; import { setupRouter, reset, whenRouteIdle } from 'ember-app-scheduler'; import { gte } from 'ember-compatibility-helpers'; +const TRY_TO_SCROLL_INTERVAL_MS = 50; +let timeoutHandle = null; +let scrollBarWidth = 0; //null; + +function tryScrollRecursively(fn, scrollHash) { + clearTimeout(timeoutHandle); + + const body = document.body; + const html = document.documentElement; + + const documentWidth = Math.max(body.scrollWidth, body.offsetWidth, + html.clientWidth, html.scrollWidth, html.offsetWidth); + const documentHeight = Math.max(body.scrollHeight, body.offsetHeight, + html.clientHeight, html.scrollHeight, html.offsetHeight); + + if ( + (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x + && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y) + || Date.now() > scrollHash.lastTry + ) { + fn.call(null, scrollHash.x, scrollHash.y); + } else { + timeoutHandle = setTimeout(() => { + tryScrollRecursively(fn, scrollHash) + }, TRY_TO_SCROLL_INTERVAL_MS); + } +} + let RouterScrollMixin = Mixin.create({ service: inject('router-scroll'), @@ -74,10 +102,8 @@ let RouterScrollMixin = Mixin.create({ const scrollElement = get(this, 'service.scrollElement'); const targetElement = get(this, 'service.targetElement'); - if (targetElement) { - window.scrollTo(scrollPosition.x, scrollPosition.y); - } else if ('window' === scrollElement) { - window.scrollTo(scrollPosition.x, scrollPosition.y); + if (targetElement || 'window' === scrollElement) { + tryScrollRecursively(window.scrollTo, scrollPosition); } else if ('#' === scrollElement.charAt(0)) { const element = document.getElementById(scrollElement.substring(1)); diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 7880cbb3..d0739360 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -10,6 +10,8 @@ const RouterScroll = Service.extend({ return fastboot ? fastboot.get('isFastBoot') : false; }), + SCROLL_RESTORATION: 2000, + key: null, scrollElement: 'window', targetElement: null, @@ -47,7 +49,11 @@ const RouterScroll = Service.extend({ // if we are looking to where to transition to next, we need to set the default to the position // of the targetElement on screen - set(scrollMap, 'default', { x, y }); + set(scrollMap, 'default', { + x, + y, + lastTry: Date.now() + this.SCROLL_RESTORATION + }); } } else if ('window' === scrollElement) { x = window.scrollX; @@ -63,7 +69,11 @@ const RouterScroll = Service.extend({ // only a `key` present after first load if (key && 'number' === typeOf(x) && 'number' === typeOf(y)) { - set(scrollMap, key, { x, y }); + set(scrollMap, key, { + x, + y, + lastTry: Date.now() + this.SCROLL_RESTORATION + }); } }, From 19daa545e1c59bbd208c859b313ac97afd36db4a Mon Sep 17 00:00:00 2001 From: snewcomer Date: Mon, 9 Sep 2019 21:12:43 -0700 Subject: [PATCH 02/24] default case --- addon/services/router-scroll.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index d0739360..532fcca5 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -22,7 +22,7 @@ const RouterScroll = Service.extend({ init(...args) { this._super(...args); this._loadConfig(); - set(this, 'scrollMap', { default: { x: 0, y: 0 }}); + set(this, 'scrollMap', { default: { x: 0, y: 0, lastTry: Date.now() }}); }, unsetFirstLoad() { From 63c3a89ca6976cb7242388968fca632ce94ea0cc Mon Sep 17 00:00:00 2001 From: snewcomer Date: Mon, 9 Sep 2019 21:29:15 -0700 Subject: [PATCH 03/24] further improve tests --- addon/index.js | 4 ++-- tests/unit/services/router-scroll-test.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/addon/index.js b/addon/index.js index 6c517e6a..af9d4580 100644 --- a/addon/index.js +++ b/addon/index.js @@ -8,7 +8,7 @@ import { gte } from 'ember-compatibility-helpers'; const TRY_TO_SCROLL_INTERVAL_MS = 50; let timeoutHandle = null; -let scrollBarWidth = 0; //null; +let scrollBarWidth = 1; //null; function tryScrollRecursively(fn, scrollHash) { clearTimeout(timeoutHandle); @@ -24,7 +24,7 @@ function tryScrollRecursively(fn, scrollHash) { if ( (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y) - || Date.now() > scrollHash.lastTry + || Date.now() > (scrollHash.lastTry || 0) ) { fn.call(null, scrollHash.x, scrollHash.y); } else { diff --git a/tests/unit/services/router-scroll-test.js b/tests/unit/services/router-scroll-test.js index 9f59a7f2..719a67e4 100644 --- a/tests/unit/services/router-scroll-test.js +++ b/tests/unit/services/router-scroll-test.js @@ -4,6 +4,7 @@ import { setupTest } from 'ember-qunit'; const defaultScrollMap = { default: { + lastTry: Date.now(), x: 0, y: 0 } From 60290535857a6b7cb195d0fd2302985c31260435 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Mon, 9 Sep 2019 23:00:31 -0700 Subject: [PATCH 04/24] fix tests with mock date util --- .eslintrc.js | 5 +- tests/helpers/mock-date.js | 49 +++++++++++++++++ tests/unit/mixins/router-scroll-test.js | 66 +++++++++++------------ tests/unit/services/router-scroll-test.js | 36 ++++++++----- 4 files changed, 106 insertions(+), 50 deletions(-) create mode 100644 tests/helpers/mock-date.js diff --git a/.eslintrc.js b/.eslintrc.js index ec0142c1..84aefb36 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -14,8 +14,6 @@ module.exports = { env: { browser: true }, - rules: { - }, overrides: [ // node files { @@ -33,7 +31,8 @@ module.exports = { 'addon/**', 'addon-test-support/**', 'app/**', - 'tests/dummy/app/**' + 'tests/dummy/app/**', + 'tests/helpers/**' ], parserOptions: { sourceType: 'script', diff --git a/tests/helpers/mock-date.js b/tests/helpers/mock-date.js new file mode 100644 index 00000000..5bd56224 --- /dev/null +++ b/tests/helpers/mock-date.js @@ -0,0 +1,49 @@ +/* eslint-disable */ + +let _Date = Date; +let now = null; + +function MockDate() { + let date; + + if (now !== null) { + date = new _Date(now); + } else { + date = new _Date(); + } + + return date; +} + +MockDate.now = function() { + return new MockDate().valueOf(); +}; + +MockDate.UTC = _Date.UTC; +MockDate.parse = function(dateString) { + return _Date.parse(dateString); +}; +MockDate.toString = function() { + return _Date.toString(); +}; +MockDate.prototype = _Date.prototype; + + +function set(date) { + const dateObj = new Date(date); + if (isNaN(dateObj.getTime())) { + throw new TypeError('mockdate: The time set is an invalid date: ' + date) + } + + Date = MockDate; + now = dateObj.valueOf(); +} + +function reset() { + Date = _Date; +} + +export default { + set: set, + reset: reset +}; diff --git a/tests/unit/mixins/router-scroll-test.js b/tests/unit/mixins/router-scroll-test.js index 54cb7ccf..76c79bde 100644 --- a/tests/unit/mixins/router-scroll-test.js +++ b/tests/unit/mixins/router-scroll-test.js @@ -1,9 +1,10 @@ -import { run, next } from '@ember/runloop'; +import { run } from '@ember/runloop'; import EmberObject from '@ember/object'; import Evented from '@ember/object/evented'; import RouterScroll from 'ember-router-scroll'; import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; +import { settled } from '@ember/test-helpers'; import { gte } from 'ember-compatibility-helpers'; let scrollTo, subject; @@ -44,7 +45,7 @@ module('mixin:router-scroll', function(hooks) { return gte('3.6.0-beta.1') ? transition : [transition]; } - test('when the application is FastBooted', function(assert) { + test('when the application is FastBooted', async function(assert) { assert.expect(1); const done = assert.async(); @@ -58,17 +59,15 @@ module('mixin:router-scroll', function(hooks) { subject = this.owner.factoryFor('router:main').create(); - run(() => { - if(gte('3.6.0-beta.1')) { - subject.trigger('routeDidChange'); - } else { - subject.didTransition(); - } - next(() => { - assert.ok(true, 'it should not call updateScrollPosition.'); - done(); - }); - }); + if(gte('3.6.0-beta.1')) { + subject.trigger('routeDidChange'); + } else { + subject.didTransition(); + } + + assert.ok(true, 'it should not call updateScrollPosition.'); + await settled(); + done(); }); test('when the application is not FastBooted', function(assert) { @@ -257,13 +256,15 @@ module('mixin:router-scroll', function(hooks) { assert.expect(1); const done = assert.async(); - window.scrollTo = (x, y) => - assert.ok(x === 1 && y === 2, 'Scroll to called with correct offsets'); + window.scrollTo = (x, y) => { + assert.ok(x === 1 && y === 20000, 'Scroll to called with correct offsets'); + done(); + } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ get position() { - return { x: 1, y: 2 }; + return { x: 1, y: 20000 }; }, scrollElement: 'window' })); @@ -271,14 +272,11 @@ module('mixin:router-scroll', function(hooks) { subject = this.owner.factoryFor('router:main').create(); - run(() => { - if(gte('3.6.0-beta.1')) { - subject.trigger('routeDidChange', getTransitionsMock('Hello/#')); - } else { - subject.didTransition(getTransitionsMock('Hello/#')); - } - done(); - }); + if(gte('3.6.0-beta.1')) { + subject.trigger('routeDidChange', getTransitionsMock('Hello/#')); + } else { + subject.didTransition(getTransitionsMock('Hello/#')); + } }); test('Update Scroll Position: URL has nonexistent element after anchor', function(assert) { @@ -288,13 +286,15 @@ module('mixin:router-scroll', function(hooks) { const elem = document.createElement('div'); elem.id = 'World'; document.body.insertBefore(elem, null); - window.scrollTo = (x, y) => - assert.ok(x === 1 && y === 2, 'Scroll to called with correct offsets'); + window.scrollTo = (x, y) => { + assert.ok(x === 1 && y === 20000, 'Scroll to called with correct offsets'); + done(); + } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ get position() { - return { x: 1, y: 2 }; + return { x: 1, y: 20000 }; }, scrollElement: 'window' })); @@ -308,7 +308,6 @@ module('mixin:router-scroll', function(hooks) { } else { subject.didTransition(getTransitionsMock('Hello/#Bar')); } - done(); }); }); @@ -316,13 +315,15 @@ module('mixin:router-scroll', function(hooks) { assert.expect(1); const done = assert.async(); - window.scrollTo = (x, y) => - assert.ok(x === 1 && y === 2, 'Scroll to was called with correct offsets'); + window.scrollTo = (x, y) => { + assert.ok(x === 1 && y === 200000, 'Scroll to was called with correct offsets'); + done(); + } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ get position() { - return { x: 1, y: 2 }; + return { x: 1, y: 200000 }; }, scrollElement: 'window' })); @@ -336,9 +337,6 @@ module('mixin:router-scroll', function(hooks) { } else { subject.didTransition(getTransitionsMock('Hello/World')); } - next(() => { - done(); - }); }); }); }); diff --git a/tests/unit/services/router-scroll-test.js b/tests/unit/services/router-scroll-test.js index 719a67e4..6d38ef1c 100644 --- a/tests/unit/services/router-scroll-test.js +++ b/tests/unit/services/router-scroll-test.js @@ -1,7 +1,11 @@ import EmberObject, { set, get } from '@ember/object'; import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; +import MockDate from '../../helpers/mock-date'; +MockDate.set('2000-01-01'); + +// Date set by `mock-date` const defaultScrollMap = { default: { lastTry: Date.now(), @@ -13,6 +17,10 @@ const defaultScrollMap = { module('service:router-scroll', function(hooks) { setupTest(hooks); + hooks.after(function() { + MockDate.reset(); + }) + test('it inits `scrollMap` and `key`', function(assert) { const service = this.owner.lookup('service:router-scroll'); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap); @@ -35,17 +43,17 @@ module('service:router-scroll', function(hooks) { test('updating will set `scrollMap` to the current scroll position', function(assert) { const service = this.owner.factoryFor('service:router-scroll').create({ isFirstLoad: false }); - const expected = { x: window.scrollX, y: window.scrollY }; + const expected = { x: window.scrollX, y: window.scrollY, lastTry: Date.now() + service.SCROLL_RESTORATION }; set(service, 'key', '123'); service.update(); - assert.deepEqual(get(service, 'scrollMap'), { 123: expected, default: { x: 0, y: 0 } }); + assert.deepEqual(get(service, 'scrollMap'), { 123: expected, default: { x: 0, y: 0, lastTry: Date.now() } }); }); test('updating will not set `scrollMap` if scrollElement is defined and element is not found', function(assert) { const service = this.owner.factoryFor('service:router-scroll').create({ scrollElement: '#other-elem' }); service.update(); - const expected = { x: 0, y: 0 }; + const expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap); }); @@ -54,7 +62,7 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ targetElement: '#other-elem' }); service.update(); - const expected = { x: 0, y: 0 }; + const expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap); }); @@ -68,7 +76,7 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ scrollElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0 }; + let expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap, 'does not set scrollMap b/c in fastboot'); @@ -83,7 +91,7 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ targetElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0 }; + let expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap, 'does not set scrollMap b/c in fastboot'); @@ -97,10 +105,12 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ isFirstLoad: false, scrollElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0 }; + let expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); - assert.deepEqual(get(service, 'scrollMap'), { '123': expected, default: { x: 0, y: 0 } }, 'sets scrollMap'); + assert.deepEqual(get(service, 'scrollMap'), { + '123': Object.assign({}, expected, { lastTry: Date.now() + service.SCROLL_RESTORATION }), + default: { x: 0, y: 0, lastTry: Date.now() } }, 'sets scrollMap'); }); test('updating will set default `scrollMap` if targetElement is defined', function(assert) { @@ -113,17 +123,17 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ isFirstLoad: false, targetElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0 }; + let expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); - assert.deepEqual(get(service, 'scrollMap'), { '123': { x: 0, y: 100 }, default: { x: 0, y: 100 } }, 'sets scrollMap'); + assert.deepEqual(get(service, 'scrollMap'), { '123': { x: 0, y: 100, lastTry: Date.now() + service.SCROLL_RESTORATION }, default: { x: 0, y: 100, lastTry: Date.now() + service.SCROLL_RESTORATION } }, 'sets scrollMap'); }); test('computing the position for an existing state uuid return the coords', function(assert) { const service = this.owner.lookup('service:router-scroll'); window.history.replaceState({ uuid: '123' }, null); - const expected = { x: 1, y: 1 }; + const expected = { x: 1, y: 1, lastTry: Date.now() }; set(service, 'scrollMap.123', expected); assert.deepEqual(get(service, 'position'), expected); }); @@ -133,7 +143,7 @@ module('service:router-scroll', function(hooks) { const state = window.history.state; window.history.replaceState({ uuid: '123' }, null); - const expected = { x: 0, y: 0 }; + const expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected); window.history.replaceState(state, null); }); @@ -141,7 +151,7 @@ module('service:router-scroll', function(hooks) { test('computing the position for a non-existant state returns default', function(assert) { const service = this.owner.lookup('service:router-scroll'); - const expected = { x: 0, y: 0 }; + const expected = { x: 0, y: 0, lastTry: Date.now() }; assert.deepEqual(get(service, 'position'), expected); }); }); From 8592b66dc641f33b0dedb77c671955112493271d Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 08:05:24 -0700 Subject: [PATCH 05/24] need run --- tests/unit/mixins/router-scroll-test.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/unit/mixins/router-scroll-test.js b/tests/unit/mixins/router-scroll-test.js index 76c79bde..8d4c8d4c 100644 --- a/tests/unit/mixins/router-scroll-test.js +++ b/tests/unit/mixins/router-scroll-test.js @@ -272,11 +272,13 @@ module('mixin:router-scroll', function(hooks) { subject = this.owner.factoryFor('router:main').create(); - if(gte('3.6.0-beta.1')) { - subject.trigger('routeDidChange', getTransitionsMock('Hello/#')); - } else { - subject.didTransition(getTransitionsMock('Hello/#')); - } + run(() => { + if(gte('3.6.0-beta.1')) { + subject.trigger('routeDidChange', getTransitionsMock('Hello/#')); + } else { + subject.didTransition(getTransitionsMock('Hello/#')); + } + }) }); test('Update Scroll Position: URL has nonexistent element after anchor', function(assert) { From 563f04126bd81cd27028b0b825f2209399f96e3d Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 08:16:39 -0700 Subject: [PATCH 06/24] put back rules --- .eslintrc.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 84aefb36..0afe5adc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -14,6 +14,8 @@ module.exports = { env: { browser: true }, + rules: { + }, overrides: [ // node files { From 35c9aeb9cb33cc12b32b16e4037c047e5e2200f3 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 08:18:45 -0700 Subject: [PATCH 07/24] updatemsg --- tests/helpers/mock-date.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/helpers/mock-date.js b/tests/helpers/mock-date.js index 5bd56224..da3ab242 100644 --- a/tests/helpers/mock-date.js +++ b/tests/helpers/mock-date.js @@ -32,7 +32,7 @@ MockDate.prototype = _Date.prototype; function set(date) { const dateObj = new Date(date); if (isNaN(dateObj.getTime())) { - throw new TypeError('mockdate: The time set is an invalid date: ' + date) + throw new TypeError('invalid date -' + date) } Date = MockDate; From c6d1a03ec763517086155ca0a25821f376b4ce44 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 11:35:14 -0700 Subject: [PATCH 08/24] update lastTry before updating scroll position --- addon/index.js | 2 +- addon/services/router-scroll.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/addon/index.js b/addon/index.js index af9d4580..beb8243d 100644 --- a/addon/index.js +++ b/addon/index.js @@ -6,7 +6,7 @@ import { scheduleOnce } from '@ember/runloop'; import { setupRouter, reset, whenRouteIdle } from 'ember-app-scheduler'; import { gte } from 'ember-compatibility-helpers'; -const TRY_TO_SCROLL_INTERVAL_MS = 50; +const TRY_TO_SCROLL_INTERVAL_MS = 30; let timeoutHandle = null; let scrollBarWidth = 1; //null; diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 532fcca5..2b490fba 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -111,6 +111,11 @@ Object.defineProperty(RouterScroll.prototype, 'position', { set(this, 'key', stateUuid); const key = getWithDefault(this, 'key', '-1'); + if (scrollMap[key]) { + // update lastTry before finalizing scroll position before updating page + set(scrollMap[key], 'lastTry', Date.now() + this.SCROLL_RESTORATION); + } + return getWithDefault(scrollMap, key, scrollMap.default); } }); From 3a8b4d9c0b2696a1d60b7c34c1396012985ca21a Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:01:15 -0700 Subject: [PATCH 09/24] Use rAF instead of setTimeout for better read/write --- addon/index.js | 62 +++++++++++++++++++--------------- addon/utils/scrollbar-width.js | 25 ++++++++++++++ 2 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 addon/utils/scrollbar-width.js diff --git a/addon/index.js b/addon/index.js index beb8243d..51e8927f 100644 --- a/addon/index.js +++ b/addon/index.js @@ -3,35 +3,31 @@ import { get, getWithDefault, computed } from '@ember/object'; import { inject } from '@ember/service'; import { getOwner } from '@ember/application'; import { scheduleOnce } from '@ember/runloop'; -import { setupRouter, reset, whenRouteIdle } from 'ember-app-scheduler'; +import { setupRouter, reset, whenRouteIdle, whenRoutePainted } from 'ember-app-scheduler'; import { gte } from 'ember-compatibility-helpers'; +import { getScrollBarWidth } from './utils/scrollbar-width'; -const TRY_TO_SCROLL_INTERVAL_MS = 30; -let timeoutHandle = null; -let scrollBarWidth = 1; //null; +let scrollBarWidth = getScrollBarWidth(); +const body = document.body; +const html = document.documentElement; function tryScrollRecursively(fn, scrollHash) { - clearTimeout(timeoutHandle); - - const body = document.body; - const html = document.documentElement; - - const documentWidth = Math.max(body.scrollWidth, body.offsetWidth, - html.clientWidth, html.scrollWidth, html.offsetWidth); - const documentHeight = Math.max(body.scrollHeight, body.offsetHeight, - html.clientHeight, html.scrollHeight, html.offsetHeight); - - if ( - (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x - && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y) - || Date.now() > (scrollHash.lastTry || 0) - ) { - fn.call(null, scrollHash.x, scrollHash.y); - } else { - timeoutHandle = setTimeout(() => { + window.requestAnimationFrame(() => { + const documentWidth = Math.max(body.scrollWidth, body.offsetWidth, + html.clientWidth, html.scrollWidth, html.offsetWidth); + const documentHeight = Math.max(body.scrollHeight, body.offsetHeight, + html.clientHeight, html.scrollHeight, html.offsetHeight); + + if ( + (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x + && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y) + || Date.now() > (scrollHash.lastTry || 0) + ) { + fn.call(null, scrollHash.x, scrollHash.y); + } else { tryScrollRecursively(fn, scrollHash) - }, TRY_TO_SCROLL_INTERVAL_MS); - } + } + }) } let RouterScrollMixin = Mixin.create({ @@ -68,8 +64,11 @@ let RouterScrollMixin = Mixin.create({ * Updates the scroll position * @param {transition|transition[]} transition If before Ember 3.6, this will be an array of transitions, otherwise * it will be a single transition + * @method updateScrollPosition + * @param {Object} transition + * @param {Boolean} recursiveCheck - if check until window height is same or lger than than y`` */ - updateScrollPosition(transition) { + updateScrollPosition(transition, recursiveCheck) { const url = get(this, 'currentURL'); const hashElement = url ? document.getElementById(url.split('#').pop()) : null; @@ -103,7 +102,11 @@ let RouterScrollMixin = Mixin.create({ const targetElement = get(this, 'service.targetElement'); if (targetElement || 'window' === scrollElement) { - tryScrollRecursively(window.scrollTo, scrollPosition); + if (recursiveCheck) { + tryScrollRecursively(window.scrollTo, scrollPosition); + } else { + window.scrollTo(scrollPosition.x, scrollPosition.y); + } } else if ('#' === scrollElement.charAt(0)) { const element = document.getElementById(scrollElement.substring(1)); @@ -129,9 +132,14 @@ let RouterScrollMixin = Mixin.create({ } const delayScrollTop = get(this, 'service.delayScrollTop'); + const afterPaint = get(this, 'service.afterPaint'); if (!delayScrollTop) { - scheduleOnce('render', this, () => this.updateScrollPosition(transition)); + scheduleOnce('render', this, () => this.updateScrollPosition(transition, true)); + } else if (afterPaint) { + whenRoutePainted().then(() => { + this.updateScrollPosition(transition); + }); } else { // as described in ember-app-scheduler, this addon can be used to delay rendering until after First Meaningful Paint. // If you loading your routes progressively, this may be a good option to delay scrollTop until the remaining DOM elements are painted. diff --git a/addon/utils/scrollbar-width.js b/addon/utils/scrollbar-width.js new file mode 100644 index 00000000..478fb143 --- /dev/null +++ b/addon/utils/scrollbar-width.js @@ -0,0 +1,25 @@ +// https://stackoverflow.com/questions/13382516/getting-scroll-bar-width-using-javascript +export function getScrollBarWidth() { + let outer = document.createElement('div'); + outer.style.visibility = 'hidden'; + outer.style.width = '100px'; + outer.style.msOverflowStyle = 'scrollbar'; + + document.body.appendChild(outer); + + let widthNoScroll = outer.offsetWidth; + // force scrollbars + outer.style.overflow = 'scroll'; + + // add innerdiv + let inner = document.createElement('div'); + inner.style.width = '100%'; + outer.appendChild(inner); + + let widthWithScroll = inner.offsetWidth; + + // remove divs + outer.parentNode.removeChild(outer); + + return widthNoScroll - widthWithScroll; +} From e3fcd90153f246b555243f068112f08d0410e0e2 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:18:45 -0700 Subject: [PATCH 10/24] Lets try with rAF with afterPaint config as well --- README.md | 22 +++++++++++++-- addon/index.js | 14 ++++++---- addon/services/router-scroll.js | 15 ++-------- tests/unit/mixins/router-scroll-test.js | 34 ++++++++++++++++++++--- tests/unit/services/router-scroll-test.js | 29 ++++++++++--------- 5 files changed, 75 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index fb0420d2..8a34cf3c 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,8 @@ needs:[ 'service:router-scroll', 'service:scheduler' ], ### Options +#### Target Elements + If you need to scroll to the top of an area that generates a vertical scroll bar, you can specify the id of an element of the scrollable area. Default is `window` for using the scroll position of the whole viewport. You can pass an options object in your application's `config/environment.js` file. @@ -93,8 +95,22 @@ ENV['routerScroll'] = { }; ``` -Moreover, if your route breaks up render into multiple phases, you may need to delay scrollTop functionality until after -the First Meaningful Paint using `delayScrollTop: true` in your config. `delayScrollTop` defaults to `false`. +#### Scroll Timing + +You may want the default out of the box behaviour. We schedule an action after Ember's `render`. + +However, you have other options. You may need to delay scroll functionality until after +the First Meaningful Paint using `afterPaint: true` in your config. `afterPaint` defaults to `false`. + +Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. + +```javascript +ENV['routerScroll'] = { + afterPaint: true +}; +``` + +Also, if you need to perform the logic when the route is idle or if your route breaks up render into multiple phases, add `delayScrollTop: true` in your config. `delayScrollTop` defaults to `false`. This will be renamed to `afterIdle` in a major release. ```javascript ENV['routerScroll'] = { @@ -102,6 +118,8 @@ ENV['routerScroll'] = { }; ``` +I would suggest trying all of them out and seeing which works best for your app! + ## A working example diff --git a/addon/index.js b/addon/index.js index 51e8927f..b30d7a48 100644 --- a/addon/index.js +++ b/addon/index.js @@ -10,6 +10,7 @@ import { getScrollBarWidth } from './utils/scrollbar-width'; let scrollBarWidth = getScrollBarWidth(); const body = document.body; const html = document.documentElement; +let ATTEMPTS = 0; function tryScrollRecursively(fn, scrollHash) { window.requestAnimationFrame(() => { @@ -18,13 +19,13 @@ function tryScrollRecursively(fn, scrollHash) { const documentHeight = Math.max(body.scrollHeight, body.offsetHeight, html.clientHeight, html.scrollHeight, html.offsetHeight); - if ( - (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x - && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y) - || Date.now() > (scrollHash.lastTry || 0) - ) { + if (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x + && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y + || ATTEMPTS >= 60) { + ATTEMPTS = 0; fn.call(null, scrollHash.x, scrollHash.y); } else { + ATTEMPTS++; tryScrollRecursively(fn, scrollHash) } }) @@ -133,8 +134,9 @@ let RouterScrollMixin = Mixin.create({ const delayScrollTop = get(this, 'service.delayScrollTop'); const afterPaint = get(this, 'service.afterPaint'); + const afterIdle = get(this, 'service.afterIdle'); - if (!delayScrollTop) { + if (!delayScrollTop && !afterPaint && !afterIdle) { scheduleOnce('render', this, () => this.updateScrollPosition(transition, true)); } else if (afterPaint) { whenRoutePainted().then(() => { diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 2b490fba..6c818e52 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -10,8 +10,6 @@ const RouterScroll = Service.extend({ return fastboot ? fastboot.get('isFastBoot') : false; }), - SCROLL_RESTORATION: 2000, - key: null, scrollElement: 'window', targetElement: null, @@ -22,7 +20,7 @@ const RouterScroll = Service.extend({ init(...args) { this._super(...args); this._loadConfig(); - set(this, 'scrollMap', { default: { x: 0, y: 0, lastTry: Date.now() }}); + set(this, 'scrollMap', { default: { x: 0, y: 0 } }); }, unsetFirstLoad() { @@ -51,8 +49,7 @@ const RouterScroll = Service.extend({ // of the targetElement on screen set(scrollMap, 'default', { x, - y, - lastTry: Date.now() + this.SCROLL_RESTORATION + y }); } } else if ('window' === scrollElement) { @@ -71,8 +68,7 @@ const RouterScroll = Service.extend({ if (key && 'number' === typeOf(x) && 'number' === typeOf(y)) { set(scrollMap, key, { x, - y, - lastTry: Date.now() + this.SCROLL_RESTORATION + y }); } }, @@ -111,11 +107,6 @@ Object.defineProperty(RouterScroll.prototype, 'position', { set(this, 'key', stateUuid); const key = getWithDefault(this, 'key', '-1'); - if (scrollMap[key]) { - // update lastTry before finalizing scroll position before updating page - set(scrollMap[key], 'lastTry', Date.now() + this.SCROLL_RESTORATION); - } - return getWithDefault(scrollMap, key, scrollMap.default); } }); diff --git a/tests/unit/mixins/router-scroll-test.js b/tests/unit/mixins/router-scroll-test.js index 8d4c8d4c..ec8be302 100644 --- a/tests/unit/mixins/router-scroll-test.js +++ b/tests/unit/mixins/router-scroll-test.js @@ -167,6 +167,30 @@ module('mixin:router-scroll', function(hooks) { }); }); + test('when the application is not FastBooted with afterPaint', function(assert) { + assert.expect(1); + const done = assert.async(); + + this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); + this.owner.register('service:router-scroll', EmberObject.extend({ afterPaint: true })); + this.owner.register('router:main', EmberObject.extend(Evented, RouterScroll, { + updateScrollPosition() { + assert.ok(true, 'it should call updateScrollPosition.'); + done(); + } + })); + + subject = this.owner.factoryFor('router:main').create(); + + run(() => { + if(gte('3.6.0-beta.1')) { + subject.trigger('routeDidChange'); + } else { + subject.didTransition(); + } + }); + }); + test('Update Scroll Position: Can preserve position using routerService', function(assert) { assert.expect(0); const done = assert.async(); @@ -201,8 +225,10 @@ module('mixin:router-scroll', function(hooks) { const elem = document.createElement('div'); elem.id = 'World'; document.body.insertBefore(elem, null); - window.scrollTo = (x, y) => + window.scrollTo = (x, y) => { assert.ok(x === elem.offsetLeft && y === elem.offsetTop, 'Scroll to called with correct offsets'); + done() + } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ @@ -219,7 +245,6 @@ module('mixin:router-scroll', function(hooks) { } else { subject.didTransition(getTransitionsMock('Hello/#World', false)); } - done(); }); }); @@ -230,8 +255,10 @@ module('mixin:router-scroll', function(hooks) { const elem = document.createElement('div'); elem.id = 'World'; document.body.insertBefore(elem, null); - window.scrollTo = (x, y) => + window.scrollTo = (x, y) => { assert.ok(x === elem.offsetLeft && y === elem.offsetTop, 'Scroll to called with correct offsets'); + done(); + } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ @@ -248,7 +275,6 @@ module('mixin:router-scroll', function(hooks) { } else { subject.didTransition(getTransitionsMock('Hello/#World', false)); } - done(); }); }); diff --git a/tests/unit/services/router-scroll-test.js b/tests/unit/services/router-scroll-test.js index 6d38ef1c..0d15df5b 100644 --- a/tests/unit/services/router-scroll-test.js +++ b/tests/unit/services/router-scroll-test.js @@ -8,7 +8,6 @@ MockDate.set('2000-01-01'); // Date set by `mock-date` const defaultScrollMap = { default: { - lastTry: Date.now(), x: 0, y: 0 } @@ -43,17 +42,17 @@ module('service:router-scroll', function(hooks) { test('updating will set `scrollMap` to the current scroll position', function(assert) { const service = this.owner.factoryFor('service:router-scroll').create({ isFirstLoad: false }); - const expected = { x: window.scrollX, y: window.scrollY, lastTry: Date.now() + service.SCROLL_RESTORATION }; + const expected = { x: window.scrollX, y: window.scrollY }; set(service, 'key', '123'); service.update(); - assert.deepEqual(get(service, 'scrollMap'), { 123: expected, default: { x: 0, y: 0, lastTry: Date.now() } }); + assert.deepEqual(get(service, 'scrollMap'), { 123: expected, default: { x: 0, y: 0 } }); }); test('updating will not set `scrollMap` if scrollElement is defined and element is not found', function(assert) { const service = this.owner.factoryFor('service:router-scroll').create({ scrollElement: '#other-elem' }); service.update(); - const expected = { x: 0, y: 0, lastTry: Date.now() }; + const expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap); }); @@ -62,7 +61,7 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ targetElement: '#other-elem' }); service.update(); - const expected = { x: 0, y: 0, lastTry: Date.now() }; + const expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap); }); @@ -76,7 +75,7 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ scrollElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0, lastTry: Date.now() }; + let expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap, 'does not set scrollMap b/c in fastboot'); @@ -91,7 +90,7 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ targetElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0, lastTry: Date.now() }; + let expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap, 'does not set scrollMap b/c in fastboot'); @@ -105,12 +104,12 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ isFirstLoad: false, scrollElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0, lastTry: Date.now() }; + let expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); assert.deepEqual(get(service, 'scrollMap'), { - '123': Object.assign({}, expected, { lastTry: Date.now() + service.SCROLL_RESTORATION }), - default: { x: 0, y: 0, lastTry: Date.now() } }, 'sets scrollMap'); + '123': expected, + default: { x: 0, y: 0 } }, 'sets scrollMap'); }); test('updating will set default `scrollMap` if targetElement is defined', function(assert) { @@ -123,17 +122,17 @@ module('service:router-scroll', function(hooks) { const service = this.owner.factoryFor('service:router-scroll').create({ isFirstLoad: false, targetElement: '#other-elem' }); window.history.replaceState({ uuid: '123' }, null); - let expected = { x: 0, y: 0, lastTry: Date.now() }; + let expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected, 'position is defaulted'); service.update(); - assert.deepEqual(get(service, 'scrollMap'), { '123': { x: 0, y: 100, lastTry: Date.now() + service.SCROLL_RESTORATION }, default: { x: 0, y: 100, lastTry: Date.now() + service.SCROLL_RESTORATION } }, 'sets scrollMap'); + assert.deepEqual(get(service, 'scrollMap'), { '123': { x: 0, y: 100 }, default: { x: 0, y: 100 } }, 'sets scrollMap'); }); test('computing the position for an existing state uuid return the coords', function(assert) { const service = this.owner.lookup('service:router-scroll'); window.history.replaceState({ uuid: '123' }, null); - const expected = { x: 1, y: 1, lastTry: Date.now() }; + const expected = { x: 1, y: 1 }; set(service, 'scrollMap.123', expected); assert.deepEqual(get(service, 'position'), expected); }); @@ -143,7 +142,7 @@ module('service:router-scroll', function(hooks) { const state = window.history.state; window.history.replaceState({ uuid: '123' }, null); - const expected = { x: 0, y: 0, lastTry: Date.now() }; + const expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected); window.history.replaceState(state, null); }); @@ -151,7 +150,7 @@ module('service:router-scroll', function(hooks) { test('computing the position for a non-existant state returns default', function(assert) { const service = this.owner.lookup('service:router-scroll'); - const expected = { x: 0, y: 0, lastTry: Date.now() }; + const expected = { x: 0, y: 0 }; assert.deepEqual(get(service, 'position'), expected); }); }); From 8c7af084d2065cc373eb3af0de7b550927bf74b9 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:27:01 -0700 Subject: [PATCH 11/24] add more notes --- addon/index.js | 2 ++ addon/services/router-scroll.js | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/addon/index.js b/addon/index.js index b30d7a48..f8bc6e21 100644 --- a/addon/index.js +++ b/addon/index.js @@ -104,8 +104,10 @@ let RouterScrollMixin = Mixin.create({ if (targetElement || 'window' === scrollElement) { if (recursiveCheck) { + // our own implementation tryScrollRecursively(window.scrollTo, scrollPosition); } else { + // using ember-app-scheduler window.scrollTo(scrollPosition.x, scrollPosition.y); } } else if ('#' === scrollElement.charAt(0)) { diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 6c818e52..ec59e118 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -13,9 +13,12 @@ const RouterScroll = Service.extend({ key: null, scrollElement: 'window', targetElement: null, - delayScrollTop: false, isFirstLoad: true, preserveScrollPosition: false, + delayScrollTop: false, + // ember-app-scheduler properties + afterPaint: false, + afterIdle: false, init(...args) { this._super(...args); @@ -90,10 +93,16 @@ const RouterScroll = Service.extend({ set(this, 'targetElement', targetElement); } - const delayScrollTop = config.routerScroll.delayScrollTop; + const { afterPaint, afterIdle, delayScrollTop } = config.routerScroll; if (delayScrollTop === true) { set(this, 'delayScrollTop', true); } + if (afterPaint === true) { + set(this, 'afterPaint', true); + } + if (afterIdle === true) { + set(this, 'afterIdle', true); + } } } }); From 867a8d54f3aecf96e7a9984e1c19e30fde166b61 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:32:32 -0700 Subject: [PATCH 12/24] add another readme note --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8a34cf3c..47d5989f 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ You may want the default out of the box behaviour. We schedule an action after However, you have other options. You may need to delay scroll functionality until after the First Meaningful Paint using `afterPaint: true` in your config. `afterPaint` defaults to `false`. -Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. +Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. This all happens after `routeDidChange`. ```javascript ENV['routerScroll'] = { From 43448be60ac46a9f966c28197bec05d7714a74b9 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:34:17 -0700 Subject: [PATCH 13/24] grammar --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 47d5989f..c7e2756e 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ ENV['routerScroll'] = { #### Scroll Timing -You may want the default out of the box behaviour. We schedule an action after Ember's `render`. +You may want the default out of the box behaviour. We schedule scroll after Ember's `render`. However, you have other options. You may need to delay scroll functionality until after the First Meaningful Paint using `afterPaint: true` in your config. `afterPaint` defaults to `false`. From 511dde685419814059d438025abec57f2441ffaa Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:36:00 -0700 Subject: [PATCH 14/24] remove mock-date --- tests/helpers/mock-date.js | 49 ----------------------- tests/unit/services/router-scroll-test.js | 8 ---- 2 files changed, 57 deletions(-) delete mode 100644 tests/helpers/mock-date.js diff --git a/tests/helpers/mock-date.js b/tests/helpers/mock-date.js deleted file mode 100644 index da3ab242..00000000 --- a/tests/helpers/mock-date.js +++ /dev/null @@ -1,49 +0,0 @@ -/* eslint-disable */ - -let _Date = Date; -let now = null; - -function MockDate() { - let date; - - if (now !== null) { - date = new _Date(now); - } else { - date = new _Date(); - } - - return date; -} - -MockDate.now = function() { - return new MockDate().valueOf(); -}; - -MockDate.UTC = _Date.UTC; -MockDate.parse = function(dateString) { - return _Date.parse(dateString); -}; -MockDate.toString = function() { - return _Date.toString(); -}; -MockDate.prototype = _Date.prototype; - - -function set(date) { - const dateObj = new Date(date); - if (isNaN(dateObj.getTime())) { - throw new TypeError('invalid date -' + date) - } - - Date = MockDate; - now = dateObj.valueOf(); -} - -function reset() { - Date = _Date; -} - -export default { - set: set, - reset: reset -}; diff --git a/tests/unit/services/router-scroll-test.js b/tests/unit/services/router-scroll-test.js index 0d15df5b..77d130c3 100644 --- a/tests/unit/services/router-scroll-test.js +++ b/tests/unit/services/router-scroll-test.js @@ -1,11 +1,7 @@ import EmberObject, { set, get } from '@ember/object'; import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; -import MockDate from '../../helpers/mock-date'; -MockDate.set('2000-01-01'); - -// Date set by `mock-date` const defaultScrollMap = { default: { x: 0, @@ -16,10 +12,6 @@ const defaultScrollMap = { module('service:router-scroll', function(hooks) { setupTest(hooks); - hooks.after(function() { - MockDate.reset(); - }) - test('it inits `scrollMap` and `key`', function(assert) { const service = this.owner.lookup('service:router-scroll'); assert.deepEqual(get(service, 'scrollMap'), defaultScrollMap); From f51dbcc373da5a8ed13de220a37677366d9dcf10 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:38:00 -0700 Subject: [PATCH 15/24] nit --- addon/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addon/index.js b/addon/index.js index f8bc6e21..4d7d265a 100644 --- a/addon/index.js +++ b/addon/index.js @@ -67,7 +67,7 @@ let RouterScrollMixin = Mixin.create({ * it will be a single transition * @method updateScrollPosition * @param {Object} transition - * @param {Boolean} recursiveCheck - if check until window height is same or lger than than y`` + * @param {Boolean} recursiveCheck - if check until document height is same or lger than than y */ updateScrollPosition(transition, recursiveCheck) { const url = get(this, 'currentURL'); From 394c811bb3c1b7a5a5649a4d202b9b51af1464e3 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:57:10 -0700 Subject: [PATCH 16/24] update comments --- addon/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/addon/index.js b/addon/index.js index 4d7d265a..823bb6a5 100644 --- a/addon/index.js +++ b/addon/index.js @@ -141,12 +141,13 @@ let RouterScrollMixin = Mixin.create({ if (!delayScrollTop && !afterPaint && !afterIdle) { scheduleOnce('render', this, () => this.updateScrollPosition(transition, true)); } else if (afterPaint) { + // as described in ember-app-scheduler, this addon can be used to delay rendering until after First Meaningful Paint. + // If you loading your routes progressively, this may be a good option to delay scrollTop until the remaining DOM elements are painted. whenRoutePainted().then(() => { this.updateScrollPosition(transition); }); } else { - // as described in ember-app-scheduler, this addon can be used to delay rendering until after First Meaningful Paint. - // If you loading your routes progressively, this may be a good option to delay scrollTop until the remaining DOM elements are painted. + // as described in ember-app-scheduler, this addon can be used to delay rendering until after the route is idle whenRouteIdle().then(() => { this.updateScrollPosition(transition); }); From 06af4f03130cb8f2522e259514f1f53403e14ff6 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 21:59:19 -0700 Subject: [PATCH 17/24] add moar comments --- README.md | 2 +- addon/index.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c7e2756e..371d2d7a 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ ENV['routerScroll'] = { #### Scroll Timing -You may want the default out of the box behaviour. We schedule scroll after Ember's `render`. +You may want the default out of the box behaviour. We schedule scroll after Ember's `render`. This occurs on the tightest schedule between route transition start and end However, you have other options. You may need to delay scroll functionality until after the First Meaningful Paint using `afterPaint: true` in your config. `afterPaint` defaults to `false`. diff --git a/addon/index.js b/addon/index.js index 823bb6a5..b857cb34 100644 --- a/addon/index.js +++ b/addon/index.js @@ -139,6 +139,7 @@ let RouterScrollMixin = Mixin.create({ const afterIdle = get(this, 'service.afterIdle'); if (!delayScrollTop && !afterPaint && !afterIdle) { + // out of the 3 options, this happens on the tightest schedule scheduleOnce('render', this, () => this.updateScrollPosition(transition, true)); } else if (afterPaint) { // as described in ember-app-scheduler, this addon can be used to delay rendering until after First Meaningful Paint. From 4fffc4aaca2c7992121a48a2f7ccbd8fcac4bfd6 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 22:02:49 -0700 Subject: [PATCH 18/24] go back to tests the way they were before --- tests/unit/mixins/router-scroll-test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/mixins/router-scroll-test.js b/tests/unit/mixins/router-scroll-test.js index ec8be302..78315164 100644 --- a/tests/unit/mixins/router-scroll-test.js +++ b/tests/unit/mixins/router-scroll-test.js @@ -283,14 +283,14 @@ module('mixin:router-scroll', function(hooks) { const done = assert.async(); window.scrollTo = (x, y) => { - assert.ok(x === 1 && y === 20000, 'Scroll to called with correct offsets'); + assert.ok(x === 1 && y === 2, 'Scroll to called with correct offsets'); done(); } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ get position() { - return { x: 1, y: 20000 }; + return { x: 1, y: 2 }; }, scrollElement: 'window' })); @@ -315,14 +315,14 @@ module('mixin:router-scroll', function(hooks) { elem.id = 'World'; document.body.insertBefore(elem, null); window.scrollTo = (x, y) => { - assert.ok(x === 1 && y === 20000, 'Scroll to called with correct offsets'); + assert.ok(x === 1 && y === 2, 'Scroll to called with correct offsets'); done(); } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ get position() { - return { x: 1, y: 20000 }; + return { x: 1, y: 2 }; }, scrollElement: 'window' })); @@ -344,14 +344,14 @@ module('mixin:router-scroll', function(hooks) { const done = assert.async(); window.scrollTo = (x, y) => { - assert.ok(x === 1 && y === 200000, 'Scroll to was called with correct offsets'); + assert.ok(x === 1 && y === 20, 'Scroll to was called with correct offsets'); done(); } this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); this.owner.register('service:router-scroll', EmberObject.extend({ get position() { - return { x: 1, y: 200000 }; + return { x: 1, y: 20 }; }, scrollElement: 'window' })); From c0b0c77bbd804552f485fe41ec3d07d47788312e Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 22:04:23 -0700 Subject: [PATCH 19/24] add doc block --- addon/index.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/addon/index.js b/addon/index.js index b857cb34..31ef5e6e 100644 --- a/addon/index.js +++ b/addon/index.js @@ -12,6 +12,16 @@ const body = document.body; const html = document.documentElement; let ATTEMPTS = 0; +/** + * By default, we start checking to see if the document height is >= the last known `y` position + * we want to scroll to. This is important for content heavy pages that might try to scrollTo + * before the content has painted + * + * @method tryScrollRecursively + * @param {Function} fn + * @param {Object} srollHash + * @void + */ function tryScrollRecursively(fn, scrollHash) { window.requestAnimationFrame(() => { const documentWidth = Math.max(body.scrollWidth, body.offsetWidth, From 7aa6f0a52539731e414f083903bf4830bbb03147 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Tue, 10 Sep 2019 22:06:07 -0700 Subject: [PATCH 20/24] spellings --- addon/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addon/index.js b/addon/index.js index 31ef5e6e..585b1c25 100644 --- a/addon/index.js +++ b/addon/index.js @@ -19,7 +19,7 @@ let ATTEMPTS = 0; * * @method tryScrollRecursively * @param {Function} fn - * @param {Object} srollHash + * @param {Object} scrollHash * @void */ function tryScrollRecursively(fn, scrollHash) { From 37725004fd1a59064c2492916c78cf6e115b4015 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 11 Sep 2019 09:08:09 -0700 Subject: [PATCH 21/24] change config names --- README.md | 6 +++--- addon/index.js | 8 ++++---- addon/services/router-scroll.js | 14 +++++++------- tests/unit/mixins/router-scroll-test.js | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 371d2d7a..2d97e889 100644 --- a/README.md +++ b/README.md @@ -100,17 +100,17 @@ ENV['routerScroll'] = { You may want the default out of the box behaviour. We schedule scroll after Ember's `render`. This occurs on the tightest schedule between route transition start and end However, you have other options. You may need to delay scroll functionality until after -the First Meaningful Paint using `afterPaint: true` in your config. `afterPaint` defaults to `false`. +the First Meaningful Paint using `whenPainted: true` in your config. `whenPainted` defaults to `false`. Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. This all happens after `routeDidChange`. ```javascript ENV['routerScroll'] = { - afterPaint: true + whenPainted: true }; ``` -Also, if you need to perform the logic when the route is idle or if your route breaks up render into multiple phases, add `delayScrollTop: true` in your config. `delayScrollTop` defaults to `false`. This will be renamed to `afterIdle` in a major release. +Also, if you need to perform the logic when the route is idle or if your route breaks up render into multiple phases, add `delayScrollTop: true` in your config. `delayScrollTop` defaults to `false`. This will be renamed to `whenIdle` in a major release. ```javascript ENV['routerScroll'] = { diff --git a/addon/index.js b/addon/index.js index 585b1c25..2444250b 100644 --- a/addon/index.js +++ b/addon/index.js @@ -145,13 +145,13 @@ let RouterScrollMixin = Mixin.create({ } const delayScrollTop = get(this, 'service.delayScrollTop'); - const afterPaint = get(this, 'service.afterPaint'); - const afterIdle = get(this, 'service.afterIdle'); + const whenPainted = get(this, 'service.whenPainted'); + const whenIdle = get(this, 'service.whenIdle'); - if (!delayScrollTop && !afterPaint && !afterIdle) { + if (!delayScrollTop && !whenPainted && !whenIdle) { // out of the 3 options, this happens on the tightest schedule scheduleOnce('render', this, () => this.updateScrollPosition(transition, true)); - } else if (afterPaint) { + } else if (whenPainted) { // as described in ember-app-scheduler, this addon can be used to delay rendering until after First Meaningful Paint. // If you loading your routes progressively, this may be a good option to delay scrollTop until the remaining DOM elements are painted. whenRoutePainted().then(() => { diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index ec59e118..68c9bc1e 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -17,8 +17,8 @@ const RouterScroll = Service.extend({ preserveScrollPosition: false, delayScrollTop: false, // ember-app-scheduler properties - afterPaint: false, - afterIdle: false, + whenPainted: false, + whenIdle: false, init(...args) { this._super(...args); @@ -93,15 +93,15 @@ const RouterScroll = Service.extend({ set(this, 'targetElement', targetElement); } - const { afterPaint, afterIdle, delayScrollTop } = config.routerScroll; + const { whenPainted, whenIdle, delayScrollTop } = config.routerScroll; if (delayScrollTop === true) { set(this, 'delayScrollTop', true); } - if (afterPaint === true) { - set(this, 'afterPaint', true); + if (whenPainted === true) { + set(this, 'whenPainted', true); } - if (afterIdle === true) { - set(this, 'afterIdle', true); + if (whenIdle === true) { + set(this, 'whenIdle', true); } } } diff --git a/tests/unit/mixins/router-scroll-test.js b/tests/unit/mixins/router-scroll-test.js index 78315164..40c4c154 100644 --- a/tests/unit/mixins/router-scroll-test.js +++ b/tests/unit/mixins/router-scroll-test.js @@ -167,12 +167,12 @@ module('mixin:router-scroll', function(hooks) { }); }); - test('when the application is not FastBooted with afterPaint', function(assert) { + test('when the application is not FastBooted with whenPainted', function(assert) { assert.expect(1); const done = assert.async(); this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); - this.owner.register('service:router-scroll', EmberObject.extend({ afterPaint: true })); + this.owner.register('service:router-scroll', EmberObject.extend({ whenPainted: true })); this.owner.register('router:main', EmberObject.extend(Evented, RouterScroll, { updateScrollPosition() { assert.ok(true, 'it should call updateScrollPosition.'); From 8119f12514010b6622aeadf638c6719e5d8d663d Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 11 Sep 2019 09:47:54 -0700 Subject: [PATCH 22/24] cancel animationFrame is need be --- README.md | 2 +- addon/index.js | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2d97e889..20b0277a 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ You may want the default out of the box behaviour. We schedule scroll after Emb However, you have other options. You may need to delay scroll functionality until after the First Meaningful Paint using `whenPainted: true` in your config. `whenPainted` defaults to `false`. -Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. This all happens after `routeDidChange`. +Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. We include the `setupRouter` and `reset`. This all happens after `routeDidChange`. ```javascript ENV['routerScroll'] = { diff --git a/addon/index.js b/addon/index.js index 2444250b..5522da57 100644 --- a/addon/index.js +++ b/addon/index.js @@ -11,6 +11,7 @@ let scrollBarWidth = getScrollBarWidth(); const body = document.body; const html = document.documentElement; let ATTEMPTS = 0; +let requestId; /** * By default, we start checking to see if the document height is >= the last known `y` position @@ -23,7 +24,7 @@ let ATTEMPTS = 0; * @void */ function tryScrollRecursively(fn, scrollHash) { - window.requestAnimationFrame(() => { + requestId = window.requestAnimationFrame(() => { const documentWidth = Math.max(body.scrollWidth, body.offsetWidth, html.clientWidth, html.scrollWidth, html.offsetWidth); const documentHeight = Math.max(body.scrollHeight, body.offsetHeight, @@ -68,6 +69,10 @@ let RouterScrollMixin = Mixin.create({ destroy() { reset(); + if (requestId) { + window.cancelAnimationFrame(requestId); + } + this._super(...arguments); }, From 19442a2f3eb00174d2e35142043fdaa9bccadbb7 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 11 Sep 2019 13:17:16 -0700 Subject: [PATCH 23/24] update based on feedback --- README.md | 8 ++++---- addon/index.js | 16 ++++++++-------- addon/services/router-scroll.js | 20 ++++++++++++-------- tests/unit/mixins/router-scroll-test.js | 4 ++-- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 20b0277a..c824626c 100644 --- a/README.md +++ b/README.md @@ -97,20 +97,20 @@ ENV['routerScroll'] = { #### Scroll Timing -You may want the default out of the box behaviour. We schedule scroll after Ember's `render`. This occurs on the tightest schedule between route transition start and end +You may want the default "out of the box" behaviour. We schedule scroll after Ember's `render`. This occurs on the tightest schedule between route transition start and end However, you have other options. You may need to delay scroll functionality until after -the First Meaningful Paint using `whenPainted: true` in your config. `whenPainted` defaults to `false`. +the First Meaningful Paint using `scrollWhenPainted: true` in your config. `scrollWhenPainted` defaults to `false`. Then next two config properties uses [`ember-app-scheduler`](https://github.com/ember-app-scheduler/ember-app-scheduler), so be sure to follow the instructions in the README. We include the `setupRouter` and `reset`. This all happens after `routeDidChange`. ```javascript ENV['routerScroll'] = { - whenPainted: true + scrollWhenPainted: true }; ``` -Also, if you need to perform the logic when the route is idle or if your route breaks up render into multiple phases, add `delayScrollTop: true` in your config. `delayScrollTop` defaults to `false`. This will be renamed to `whenIdle` in a major release. +Also, if you need to perform the logic when the route is idle or if your route breaks up render into multiple phases, add `delayScrollTop: true` in your config. `delayScrollTop` defaults to `false`. This will be renamed to `scrollWhenIdle` in a major release. ```javascript ENV['routerScroll'] = { diff --git a/addon/index.js b/addon/index.js index 5522da57..c23c824f 100644 --- a/addon/index.js +++ b/addon/index.js @@ -11,6 +11,7 @@ let scrollBarWidth = getScrollBarWidth(); const body = document.body; const html = document.documentElement; let ATTEMPTS = 0; +const MAX_ATTEMPTS = 100; // rAF runs every 16ms ideally, so 60x a second let requestId; /** @@ -32,7 +33,7 @@ function tryScrollRecursively(fn, scrollHash) { if (documentWidth + scrollBarWidth - window.innerWidth >= scrollHash.x && documentHeight + scrollBarWidth - window.innerHeight >= scrollHash.y - || ATTEMPTS >= 60) { + || ATTEMPTS >= MAX_ATTEMPTS) { ATTEMPTS = 0; fn.call(null, scrollHash.x, scrollHash.y); } else { @@ -78,11 +79,10 @@ let RouterScrollMixin = Mixin.create({ /** * Updates the scroll position - * @param {transition|transition[]} transition If before Ember 3.6, this will be an array of transitions, otherwise * it will be a single transition * @method updateScrollPosition - * @param {Object} transition - * @param {Boolean} recursiveCheck - if check until document height is same or lger than than y + * @param {transition|transition[]} transition If before Ember 3.6, this will be an array of transitions, otherwise + * @param {Boolean} recursiveCheck - if "true", check until document height is >= y. `y` is the last coordinate the target page was on */ updateScrollPosition(transition, recursiveCheck) { const url = get(this, 'currentURL'); @@ -150,13 +150,13 @@ let RouterScrollMixin = Mixin.create({ } const delayScrollTop = get(this, 'service.delayScrollTop'); - const whenPainted = get(this, 'service.whenPainted'); - const whenIdle = get(this, 'service.whenIdle'); + const scrollWhenPainted = get(this, 'service.scrollWhenPainted'); + const scrollWhenIdle = get(this, 'service.scrollWhenIdle'); - if (!delayScrollTop && !whenPainted && !whenIdle) { + if (!delayScrollTop && !scrollWhenPainted && !scrollWhenIdle) { // out of the 3 options, this happens on the tightest schedule scheduleOnce('render', this, () => this.updateScrollPosition(transition, true)); - } else if (whenPainted) { + } else if (scrollWhenPainted) { // as described in ember-app-scheduler, this addon can be used to delay rendering until after First Meaningful Paint. // If you loading your routes progressively, this may be a good option to delay scrollTop until the remaining DOM elements are painted. whenRoutePainted().then(() => { diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 68c9bc1e..69866539 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -17,13 +17,17 @@ const RouterScroll = Service.extend({ preserveScrollPosition: false, delayScrollTop: false, // ember-app-scheduler properties - whenPainted: false, - whenIdle: false, + scrollWhenPainted: false, + scrollWhenIdle: false, init(...args) { this._super(...args); this._loadConfig(); - set(this, 'scrollMap', { default: { x: 0, y: 0 } }); + set(this, 'scrollMap', { + default: { + x: 0, y: 0 + } + }); }, unsetFirstLoad() { @@ -93,15 +97,15 @@ const RouterScroll = Service.extend({ set(this, 'targetElement', targetElement); } - const { whenPainted, whenIdle, delayScrollTop } = config.routerScroll; + const { scrollWhenPainted, scrollWhenIdle, delayScrollTop } = config.routerScroll; if (delayScrollTop === true) { set(this, 'delayScrollTop', true); } - if (whenPainted === true) { - set(this, 'whenPainted', true); + if (scrollWhenPainted === true) { + set(this, 'scrollWhenPainted', true); } - if (whenIdle === true) { - set(this, 'whenIdle', true); + if (scrollWhenIdle === true) { + set(this, 'scrollWhenIdle', true); } } } diff --git a/tests/unit/mixins/router-scroll-test.js b/tests/unit/mixins/router-scroll-test.js index 40c4c154..9416eb3e 100644 --- a/tests/unit/mixins/router-scroll-test.js +++ b/tests/unit/mixins/router-scroll-test.js @@ -167,12 +167,12 @@ module('mixin:router-scroll', function(hooks) { }); }); - test('when the application is not FastBooted with whenPainted', function(assert) { + test('when the application is not FastBooted with scrollWhenPainted', function(assert) { assert.expect(1); const done = assert.async(); this.owner.register('service:fastboot', EmberObject.extend({ isFastBoot: false })); - this.owner.register('service:router-scroll', EmberObject.extend({ whenPainted: true })); + this.owner.register('service:router-scroll', EmberObject.extend({ scrollWhenPainted: true })); this.owner.register('router:main', EmberObject.extend(Evented, RouterScroll, { updateScrollPosition() { assert.ok(true, 'it should call updateScrollPosition.'); From 5f152a37736c9f04b1d87b4271ffc705aa08294e Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 11 Sep 2019 13:32:20 -0700 Subject: [PATCH 24/24] use default false --- addon/services/router-scroll.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 69866539..8b6a6b4a 100644 --- a/addon/services/router-scroll.js +++ b/addon/services/router-scroll.js @@ -97,16 +97,14 @@ const RouterScroll = Service.extend({ set(this, 'targetElement', targetElement); } - const { scrollWhenPainted, scrollWhenIdle, delayScrollTop } = config.routerScroll; - if (delayScrollTop === true) { - set(this, 'delayScrollTop', true); - } - if (scrollWhenPainted === true) { - set(this, 'scrollWhenPainted', true); - } - if (scrollWhenIdle === true) { - set(this, 'scrollWhenIdle', true); - } + const { + scrollWhenPainted = false, + scrollWhenIdle = false, + delayScrollTop = false + } = config.routerScroll; + set(this, 'delayScrollTop', delayScrollTop); + set(this, 'scrollWhenPainted', scrollWhenPainted); + set(this, 'scrollWhenIdle', scrollWhenIdle); } } });