From 72b0680465dbb05c852c85eaf7417773b59dc259 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 19 May 2020 17:33:31 -0400 Subject: [PATCH] Ensure `is-active` updates properly from <-> to loading states. --- .eslintrc.js | 3 +- addon/helpers/is-active.js | 4 +- addon/utils/route-params.js | 4 +- addon/utils/track-active-route.js | 10 + tests/helpers/index.js | 11 + tests/integration/helpers/is-active-test.js | 97 ++++-- .../integration/helpers/route-params-test.js | 293 +++++++++++------- 7 files changed, 287 insertions(+), 135 deletions(-) create mode 100644 addon/utils/track-active-route.js create mode 100644 tests/helpers/index.js diff --git a/.eslintrc.js b/.eslintrc.js index 85a1900..d3b11bc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -40,7 +40,8 @@ module.exports = { 'addon/**', 'addon-test-support/**', 'app/**', - 'tests/dummy/app/**' + 'tests/dummy/app/**', + 'tests/helpers/**' ], parserOptions: { sourceType: 'script' diff --git a/addon/helpers/is-active.js b/addon/helpers/is-active.js index 32ed48c..58a94f8 100644 --- a/addon/helpers/is-active.js +++ b/addon/helpers/is-active.js @@ -1,13 +1,13 @@ import { inject as service } from '@ember/service'; import Helper from '@ember/component/helper'; import handleQueryParams from '../utils/handle-query-params'; +import trackActiveRoute from '../utils/track-active-route'; export default class IsActiveHelper extends Helper { @service router; compute(_params) { - // ensure router.currentURL is auto-tracked - this.router.currentURL; + trackActiveRoute(this.router); let params = handleQueryParams(_params); diff --git a/addon/utils/route-params.js b/addon/utils/route-params.js index b2c323c..e9579b1 100644 --- a/addon/utils/route-params.js +++ b/addon/utils/route-params.js @@ -1,4 +1,5 @@ import handleQueryParams from './handle-query-params'; +import trackActiveRoute from './track-active-route'; export default class RouteParams { constructor(router, params) { @@ -10,8 +11,7 @@ export default class RouteParams { } get isActive() { - // ensure router.currentURL is auto-tracked - this._router.currentURL; + trackActiveRoute(this._router); return this._router.isActive(...this._params); } diff --git a/addon/utils/track-active-route.js b/addon/utils/track-active-route.js new file mode 100644 index 0000000..5ed628c --- /dev/null +++ b/addon/utils/track-active-route.js @@ -0,0 +1,10 @@ +export default function trackActiveRoute(router) { + // ensure we recompute anytime `router.currentURL` changes + router.currentURL; + + // ensure we recompute whenever the `router.currentRouteName` changes + // this is slightly overlapping with router.currentURL but there are + // cases where route.currentURL doesn't change but the + // router.currentRouteName has (e.g. loading and error states) + router.currentRouteName; +} diff --git a/tests/helpers/index.js b/tests/helpers/index.js new file mode 100644 index 0000000..c8eab1c --- /dev/null +++ b/tests/helpers/index.js @@ -0,0 +1,11 @@ +export function defer() { + let deferred = { resolve: undefined, reject: undefined }; + + deferred.promise = new Promise((resolve, reject) => { + deferred.resolve = resolve; + deferred.reject = reject; + }); + + return deferred; +} + diff --git a/tests/integration/helpers/is-active-test.js b/tests/integration/helpers/is-active-test.js index 37906cd..da49c98 100644 --- a/tests/integration/helpers/is-active-test.js +++ b/tests/integration/helpers/is-active-test.js @@ -1,9 +1,12 @@ import { computed } from '@ember/object'; import Service from '@ember/service'; import { module, test } from 'qunit'; -import { setupRenderingTest } from 'ember-qunit'; -import { render, settled } from '@ember/test-helpers'; +import { setupRenderingTest, setupApplicationTest } from 'ember-qunit'; +import { render, settled, visit, waitFor } from '@ember/test-helpers'; +import EmberRouter from '@ember/routing/router'; +import Route from '@ember/routing/route'; import hbs from 'htmlbars-inline-precompile'; +import { defer } from '../../helpers'; const RouterServiceMock = Service.extend({ currentRouteName: computed('currentURL', function() { @@ -15,32 +18,90 @@ const RouterServiceMock = Service.extend({ } }); -module('helper:is-active', function(hooks) { - setupRenderingTest(hooks); +module('helper:is-active', function() { + module('rendering', function(hooks) { + setupRenderingTest(hooks); - hooks.beforeEach(function() { - this.owner.register('service:router', RouterServiceMock); + hooks.beforeEach(function() { + this.owner.register('service:router', RouterServiceMock); + }); + + test('it renders and rerenders when currentURL changes', async function(assert) { + const router = this.owner.lookup('service:router'); + + router.set('currentURL', '/foo'); + this.set('targetRoute', 'bar'); + await render(hbs`{{is-active targetRoute}}`); + + assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when curren route is different from target route'); + + router.set('currentURL', '/bar'); + + await settled(); + + assert.strictEqual(this.element.textContent, 'true', 'is-active is true now when URL has changed'); + + router.set('currentURL', '/foo'); + + await settled(); + + assert.strictEqual(this.element.textContent, 'false', 'is-active is false when curren route has changed'); + }); }); - test('it renders and rerenders when currentURL changes', async function(assert) { - const router = this.owner.lookup('service:router'); + module('routing', function(hooks) { + setupApplicationTest(hooks); + + hooks.beforeEach(function() { + class Router extends EmberRouter {} + Router.map(function() { + this.route('foo'); + this.route('bar'); + }); + + this.owner.register('router:main', Router); + this.owner.register('template:application', hbs`{{is-active 'foo'}}{{outlet}}`); + }); + + test('it updates when currentURL changes', async function(assert) { + await visit('/'); + + assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when curren route is different from target route'); + + await visit('/foo'); + + assert.strictEqual(this.element.textContent, 'true', 'is-active is true now when URL has changed'); + + await visit('/'); + + assert.strictEqual(this.element.textContent, 'false', 'is-active is false when curren route has changed'); + }); + + test('it renders and rerenders when the URL changes into and out of loading substate', async function(assert) { + let slowModelDeferred = defer(); + + this.owner.register('route:foo', class extends Route { + model() { + return slowModelDeferred.promise; + } + }); + this.owner.register('template:foo-loading', hbs``); - router.set('currentURL', '/foo'); - this.set('targetRoute', 'bar'); - await render(hbs`{{is-active targetRoute}}`); + await visit('/'); - assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when curren route is different from target route'); + assert.strictEqual(this.element.textContent, 'false', 'precond - is-active is not true when on index route'); - router.set('currentURL', '/bar'); + let visitPromise = visit('/foo'); - await settled(); + await waitFor('.loading-spinner'); - assert.strictEqual(this.element.textContent, 'true', 'is-active is true now when URL has changed'); + assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when on the loading substate'); - router.set('currentURL', '/foo'); + slowModelDeferred.resolve(); - await settled(); + await visitPromise; - assert.strictEqual(this.element.textContent, 'false', 'is-active is false when curren route has changed'); + assert.strictEqual(this.element.textContent, 'true', 'is-active is true now that model hook has fully resolved'); + }); }); }); diff --git a/tests/integration/helpers/route-params-test.js b/tests/integration/helpers/route-params-test.js index 17b0599..4b51970 100644 --- a/tests/integration/helpers/route-params-test.js +++ b/tests/integration/helpers/route-params-test.js @@ -1,202 +1,271 @@ import { module, test } from 'qunit'; -import { setupRenderingTest } from 'ember-qunit'; -import { click, render, settled } from '@ember/test-helpers'; +import { setupRenderingTest, setupApplicationTest } from 'ember-qunit'; +import { click, render, visit, waitFor, settled } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; import { setRouteParamsClass } from 'ember-router-helpers/helpers/route-params'; import RouteParams from 'ember-router-helpers/utils/route-params'; +import { defer } from '../../helpers'; +import EmberRouter from '@ember/routing/router'; +import Route from '@ember/routing/route'; -module('helper:route-params', function(hooks) { - setupRenderingTest(hooks); +module('helper:route-params', function() { + module('rendering', function(hooks) { + setupRenderingTest(hooks); - hooks.beforeEach(function() { - this.owner.lookup('router:main').setupRouter(); - }); + hooks.beforeEach(function() { + this.owner.lookup('router:main').setupRouter(); + }); - hooks.afterEach(function() { - setRouteParamsClass(RouteParams); - }); + hooks.afterEach(function() { + setRouteParamsClass(RouteParams); + }); - test('route-params yields correct URL value', async function(assert) { - await render(hbs` + test('route-params yields correct URL value', async function(assert) { + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{routeParams.url}} {{/with}}`); - assert.strictEqual(this.element.textContent.trim(), '/child'); - }); + assert.strictEqual(this.element.textContent.trim(), '/child'); + }); - test('route-params yields correct URL value with query-params helper', async function(assert) { - await render(hbs` + test('route-params yields correct URL value with query-params helper', async function(assert) { + await render(hbs` {{#with (route-params 'parent.child' (query-params foo="bar")) as |routeParams|}} {{routeParams.url}} {{/with}}`); - assert.strictEqual(this.element.textContent.trim(), '/child?foo=bar'); - }); + assert.strictEqual(this.element.textContent.trim(), '/child?foo=bar'); + }); - test('route-params yields correct URL value with query params from context options', async function(assert) { - this.set('queryParams', { queryParams: { foo: 'bar' }}); + test('route-params yields correct URL value with query params from context options', async function(assert) { + this.set('queryParams', { queryParams: { foo: 'bar' }}); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child' queryParams) as |routeParams|}} {{routeParams.url}} {{/with}}`); - assert.strictEqual(this.element.textContent.trim(), '/child?foo=bar'); - }); + assert.strictEqual(this.element.textContent.trim(), '/child?foo=bar'); + }); - test('route-params only calls urlFor once per render', async function(assert) { - assert.expect(1); + test('route-params only calls urlFor once per render', async function(assert) { + assert.expect(1); - let router = this.owner.lookup('service:router'); - router.urlFor = () => { - assert.ok(true, 'urlFor called'); - return '/foo'; - } + let router = this.owner.lookup('service:router'); + router.urlFor = () => { + assert.ok(true, 'urlFor called'); + return '/foo'; + } - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{routeParams.url}} {{/with}}`); - }); + }); - test('route-params only calls transitionTo once per render', async function(assert) { - assert.expect(1); + test('route-params only calls transitionTo once per render', async function(assert) { + assert.expect(1); - class MockedRouteParams extends RouteParams { - get transitionTo() { - assert.ok(true, 'transitionTo called'); - return super.transitionTo; + class MockedRouteParams extends RouteParams { + get transitionTo() { + assert.ok(true, 'transitionTo called'); + return super.transitionTo; + } } - } - setRouteParamsClass(MockedRouteParams); + setRouteParamsClass(MockedRouteParams); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{routeParams.transitionTo}} {{/with}}`); - }); + }); - test('route-params actions invoke transitionTo', async function(assert) { - assert.expect(1); + test('route-params actions invoke transitionTo', async function(assert) { + assert.expect(1); - class MockedRouteParams extends RouteParams { - get transitionTo() { - return () => { - assert.ok(true, 'transtionTo invoked'); - }; + class MockedRouteParams extends RouteParams { + get transitionTo() { + return () => { + assert.ok(true, 'transtionTo invoked'); + }; + } } - } - setRouteParamsClass(MockedRouteParams); + setRouteParamsClass(MockedRouteParams); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{/with}}`); - await click('button'); - }); + await click('button'); + }); - test('calls preventDefault on event (e.g. `onclick={{routeParams.transitionTo}}`)', async function(assert) { - assert.expect(2); + test('calls preventDefault on event (e.g. `onclick={{routeParams.transitionTo}}`)', async function(assert) { + assert.expect(2); - let fakeRouter = { - transitionTo: () => { - assert.ok(true, 'transtionTo invoked'); - } - }; + let fakeRouter = { + transitionTo: () => { + assert.ok(true, 'transtionTo invoked'); + } + }; - class MockedRouteParams extends RouteParams { - constructor(_router, params) { - super(fakeRouter, params); + class MockedRouteParams extends RouteParams { + constructor(_router, params) { + super(fakeRouter, params); + } } - } - setRouteParamsClass(MockedRouteParams); + setRouteParamsClass(MockedRouteParams); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{/with}}`); - let element = this.element; - element.addEventListener('click', (event) => { - assert.ok(event.defaultPrevented, 'default should have been prevented on click event'); - }); + let element = this.element; + element.addEventListener('click', (event) => { + assert.ok(event.defaultPrevented, 'default should have been prevented on click event'); + }); - await click('a'); - }); + await click('a'); + }); - test('route-params only calls replaceWith once per render', async function(assert) { - assert.expect(1); + test('route-params only calls replaceWith once per render', async function(assert) { + assert.expect(1); - class MockedRouteParams extends RouteParams { - get replaceWith() { - assert.ok(true, 'replaceWith called'); - return super.replaceWith; + class MockedRouteParams extends RouteParams { + get replaceWith() { + assert.ok(true, 'replaceWith called'); + return super.replaceWith; + } } - } - setRouteParamsClass(MockedRouteParams); + setRouteParamsClass(MockedRouteParams); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{routeParams.replaceWith}} {{/with}}`); - }); + }); - test('route-params actions invoke replaceWith', async function(assert) { - assert.expect(1); + test('route-params actions invoke replaceWith', async function(assert) { + assert.expect(1); - class MockedRouteParams extends RouteParams { - get replaceWith() { - return () => { - assert.ok(true, 'replaceWith invoked'); - }; + class MockedRouteParams extends RouteParams { + get replaceWith() { + return () => { + assert.ok(true, 'replaceWith invoked'); + }; + } } - } - setRouteParamsClass(MockedRouteParams); + setRouteParamsClass(MockedRouteParams); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} {{/with}}`); - await click('button'); - }); + await click('button'); + }); - test('route-params yields correct isActive value', async function(assert) { - assert.expect(6); + test('route-params yields correct isActive value', async function(assert) { + assert.expect(6); - let currentURL = '/current-url'; - let router = this.owner.lookup('service:router'); - router.isActive = () => { - assert.ok(true, 'isActive called'); - return router.get('currentURL') === currentURL; - } + let currentURL = '/current-url'; + let router = this.owner.lookup('service:router'); + router.isActive = () => { + assert.ok(true, 'isActive called'); + return router.get('currentURL') === currentURL; + } - router.set('_router.currentURL', currentURL); + router.set('_router.currentURL', currentURL); - await render(hbs` + await render(hbs` {{#with (route-params 'parent.child') as |routeParams|}} Blah {{/with}}`); - assert.dom('a').hasClass('active'); + assert.dom('a').hasClass('active'); + + router.set('_router.currentURL', '/different-url'); + + await settled(); + + assert.dom('a').doesNotHaveClass('active'); + router.set('_router.currentURL', currentURL); + + await settled(); + + assert.dom('a').hasClass('active'); + }); + }); + + module('routing', function(hooks) { + setupApplicationTest(hooks); + + hooks.beforeEach(function() { + class Router extends EmberRouter {} + Router.map(function() { + this.route('foo'); + this.route('bar'); + }); + + this.owner.register('router:main', Router); + this.owner.register( + 'template:application', + hbs` + {{#let (route-params 'foo') as |routeParams|}} + Blah + {{/let}} + {{outlet}} + ` + ); + }); + + test('isActive updates when currentURL changes', async function(assert) { + await visit('/'); - router.set('_router.currentURL', '/different-url'); + assert.dom('a').hasClass('inactive'); - await settled(); + await visit('/foo'); - assert.dom('a').doesNotHaveClass('active'); - router.set('_router.currentURL', currentURL); + assert.dom('a').hasClass('active'); - await settled(); + await visit('/'); - assert.dom('a').hasClass('active'); + assert.dom('a').hasClass('inactive'); + }); + + test('it renders and rerenders when the URL changes into and out of loading substate', async function(assert) { + let slowModelDeferred = defer(); + + this.owner.register('route:foo', class extends Route { + model() { + return slowModelDeferred.promise; + } + }); + this.owner.register('template:foo-loading', hbs``); + + await visit('/'); + + assert.dom('a').hasClass('inactive'); + + let visitPromise = visit('/foo'); + + await waitFor('.loading-spinner'); + + assert.dom('a').hasClass('inactive'); + + slowModelDeferred.resolve(); + + await visitPromise; + + assert.dom('a').hasClass('active'); + }); }); });