From 1332a763f0e339f4ab000c5ba325611aa6706a47 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Fri, 18 Nov 2022 18:00:40 -0700 Subject: [PATCH 1/2] Replace scroll-tracker mixin with class decorator NOTE: there may be an issue with this on FastBoot (there is a report to that effect in the output in the console). It is not clear *why*, though, since the behavior in this decorated class *should* be the same as in the mixin. Possibly the difference between the route events and the router events is coming into play? --- app/mixins/scroll-tracker.js | 42 -------------- app/routes/project-version/classes/class.js | 5 +- app/routes/project-version/modules/module.js | 5 +- .../project-version/namespaces/namespace.js | 5 +- app/routes/project.js | 5 +- app/services/scroll-position-reset.js | 6 ++ app/utils/with-scroll-reset.js | 58 +++++++++++++++++++ tests/unit/mixins/scroll-tracker-test.js | 12 ---- 8 files changed, 76 insertions(+), 62 deletions(-) delete mode 100644 app/mixins/scroll-tracker.js create mode 100644 app/utils/with-scroll-reset.js delete mode 100644 tests/unit/mixins/scroll-tracker-test.js diff --git a/app/mixins/scroll-tracker.js b/app/mixins/scroll-tracker.js deleted file mode 100644 index 51a2e0d8..00000000 --- a/app/mixins/scroll-tracker.js +++ /dev/null @@ -1,42 +0,0 @@ -import Mixin from '@ember/object/mixin'; -import { inject as service } from '@ember/service'; -import config from 'ember-api-docs/config/environment'; -import getOffset from 'ember-api-docs/utils/get-offset'; - -export default Mixin.create({ - scrollPositionReset: service(), - - actions: { - willTransition(transition) { - this.scrollPositionReset.scheduleReset(transition); - }, - - didTransition() { - this._super(); - if ( - typeof FastBoot === 'undefined' && - window.location.search === '?anchor=' - ) { - let elem = document.querySelector('#methods'); - - if (elem && elem.offsetHeight) { - const offsetToScroll = getOffset( - elem, - config.APP.scrollContainerSelector - ); - const scrollContainer = document.querySelector( - config.APP.scrollContainerSelector - ); - if (scrollContainer.scrollTo) { - scrollContainer.scrollTo(0, offsetToScroll - 10); - } else { - // fallback for IE11 - scrollContainer.scrollTop = offsetToScroll - 10; - } - return; - } - } - this.scrollPositionReset.doReset(); - }, - }, -}); diff --git a/app/routes/project-version/classes/class.js b/app/routes/project-version/classes/class.js index e435e828..e50923fb 100644 --- a/app/routes/project-version/classes/class.js +++ b/app/routes/project-version/classes/class.js @@ -2,12 +2,13 @@ import { inject as service } from '@ember/service'; import { resolve, all } from 'rsvp'; import Route from '@ember/routing/route'; import { set } from '@ember/object'; -import ScrollTracker from 'ember-api-docs/mixins/scroll-tracker'; import { pluralize } from 'ember-inflector'; import getFullVersion from 'ember-api-docs/utils/get-full-version'; import createExcerpt from 'ember-api-docs/utils/create-excerpt'; +import { withScrollReset } from '../../../utils/with-scroll-reset'; -export default class ClassRoute extends Route.extend(ScrollTracker) { +@withScrollReset +export default class ClassRoute extends Route { /** @type {import('@ember/routing/router-service').default} */ @service router; diff --git a/app/routes/project-version/modules/module.js b/app/routes/project-version/modules/module.js index f57093e5..32953d93 100644 --- a/app/routes/project-version/modules/module.js +++ b/app/routes/project-version/modules/module.js @@ -1,8 +1,9 @@ import ClassRoute from '../classes/class'; -import ScrollTracker from 'ember-api-docs/mixins/scroll-tracker'; import getFullVersion from 'ember-api-docs/utils/get-full-version'; +import { withScrollReset } from '../../../utils/with-scroll-reset'; -export default class ModuleRoute extends ClassRoute.extend(ScrollTracker) { +@withScrollReset +export default class ModuleRoute extends ClassRoute { async model(params) { const { project, project_version: compactVersion } = this.paramsFor('project-version'); diff --git a/app/routes/project-version/namespaces/namespace.js b/app/routes/project-version/namespaces/namespace.js index dc8e31d0..ab122b1e 100644 --- a/app/routes/project-version/namespaces/namespace.js +++ b/app/routes/project-version/namespaces/namespace.js @@ -1,8 +1,9 @@ import ClassRoute from '../classes/class'; -import ScrollTracker from 'ember-api-docs/mixins/scroll-tracker'; import getFullVersion from 'ember-api-docs/utils/get-full-version'; +import { withScrollReset } from '../utils/with-scroll-reset'; -export default class NamespaceRoute extends ClassRoute.extend(ScrollTracker) { +@withScrollReset +export default class NamespaceRoute extends ClassRoute { templateName = 'project-version/classes/class'; async model(params) { diff --git a/app/routes/project.js b/app/routes/project.js index bcc5f143..592346a7 100644 --- a/app/routes/project.js +++ b/app/routes/project.js @@ -1,8 +1,9 @@ import Route from '@ember/routing/route'; -import ScrollTracker from 'ember-api-docs/mixins/scroll-tracker'; import { inject as service } from '@ember/service'; +import { withScrollReset } from '../utils/with-scroll-reset'; -export default class ProjectRoute extends Route.extend(ScrollTracker) { +@withScrollReset +export default class ProjectRoute extends Route { /** @type {import('@ember/routing/router-service').default} */ @service router; diff --git a/app/services/scroll-position-reset.js b/app/services/scroll-position-reset.js index de468b1b..26deacab 100644 --- a/app/services/scroll-position-reset.js +++ b/app/services/scroll-position-reset.js @@ -34,6 +34,12 @@ export default class ScrollPositionResetService extends Service { //TODO: Use routeInfo for reliable behavior const dynamicSlugLocation = 3; + // These will be unset when first entering the app, in which case we know we + // are not changing tabs. + if (!transition.from || !transition.to) { + return false; + } + let fromRoutePathParts = transition.from.name.split('.'); let toRoutePathParts = transition.to.name.split('.'); diff --git a/app/utils/with-scroll-reset.js b/app/utils/with-scroll-reset.js new file mode 100644 index 00000000..8cf8403d --- /dev/null +++ b/app/utils/with-scroll-reset.js @@ -0,0 +1,58 @@ +import { inject as service } from '@ember/service'; +import config from 'ember-api-docs/config/environment'; +import getOffset from 'ember-api-docs/utils/get-offset'; + +/** + * Add scroll reset behavior to a `Route`. + * + * @param {import('@ember/routing/route').default} SomeRoute A Route class to + * decorate with the scroll position handling. + * @returns the decorated class + */ +export function withScrollReset(SomeRoute) { + class WithScrollReset extends SomeRoute { + /** @type {import('@ember/routing/router-service').default} */ + @service + router; + + /** @type {import('../services/scroll-position-reset').default} */ + @service + scrollPositionReset; + + constructor() { + super(...arguments); + this.router.on('routeWillChange', (transition) => + this.scrollPositionReset.scheduleReset(transition) + ); + + this.router.on('routeDidChange', () => { + if ( + typeof FastBoot === 'undefined' && + window.location.search === '?anchor=' + ) { + let elem = document.querySelector('#methods'); + + if (elem && elem.offsetHeight) { + const offsetToScroll = getOffset( + elem, + config.APP.scrollContainerSelector + ); + const scrollContainer = document.querySelector( + config.APP.scrollContainerSelector + ); + if (scrollContainer.scrollTo) { + scrollContainer.scrollTo(0, offsetToScroll - 10); + } else { + // fallback for IE11 + scrollContainer.scrollTop = offsetToScroll - 10; + } + return; + } + } + this.scrollPositionReset.doReset(); + }); + } + } + + return WithScrollReset; +} diff --git a/tests/unit/mixins/scroll-tracker-test.js b/tests/unit/mixins/scroll-tracker-test.js deleted file mode 100644 index 8459719e..00000000 --- a/tests/unit/mixins/scroll-tracker-test.js +++ /dev/null @@ -1,12 +0,0 @@ -import EmberObject from '@ember/object'; -import ScrollTrackerMixin from 'ember-api-docs/mixins/scroll-tracker'; -import { module, test } from 'qunit'; - -module('Unit | Mixin | scroll tracker', function () { - // Replace this with your real tests. - test('it works', function (assert) { - let ScrollTrackerObject = EmberObject.extend(ScrollTrackerMixin); - let subject = ScrollTrackerObject.create(); - assert.ok(subject); - }); -}); From 4fce8b39132c81c0e0e30f5b6c4e62459685f7de Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Sun, 20 Nov 2022 07:39:18 -0700 Subject: [PATCH 2/2] Avoid a potential FastBoot issue in scroll-position-reset --- app/services/scroll-position-reset.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/services/scroll-position-reset.js b/app/services/scroll-position-reset.js index 26deacab..6f2a5824 100644 --- a/app/services/scroll-position-reset.js +++ b/app/services/scroll-position-reset.js @@ -63,6 +63,12 @@ export default class ScrollPositionResetService extends Service { } doReset() { + // No-op if we get called in FastBoot, since doing the `document` operations + // is by definition unsafe in that mode! + if (typeof FastBoot !== 'undefined') { + return; + } + if (this._shouldResetScroll) { const selector = document.querySelector(scrollContainerSelector); if (selector.scrollTo) {