diff --git a/addon/components/bs-dropdown/menu.hbs b/addon/components/bs-dropdown/menu.hbs index 44d4ff645..20ebb2e33 100644 --- a/addon/components/bs-dropdown/menu.hbs +++ b/addon/components/bs-dropdown/menu.hbs @@ -17,8 +17,8 @@ {{yield (hash item=(bs-default @itemComponent (component "bs-dropdown/menu/item")) - link-to=(bs-default @linkToComponent (component "bs-dropdown/menu/link-to")) - linkTo=(bs-default @linkToComponent (component "bs-dropdown/menu/link-to")) + link-to=(bs-default @linkToComponent (component "bs-link-to" class=(if (macroCondition (macroGetOwnConfig "isNotBS3")) "dropdown-item"))) + linkTo=(bs-default @linkToComponent (component "bs-link-to" class=(if (macroCondition (macroGetOwnConfig "isNotBS3")) "dropdown-item"))) divider=(bs-default @dividerComponent (component "bs-dropdown/menu/divider")) ) }} @@ -46,8 +46,8 @@ {{yield (hash item=(bs-default @itemComponent (component "bs-dropdown/menu/item")) - link-to=(bs-default @linkToComponent (component "bs-dropdown/menu/link-to")) - linkTo=(bs-default @linkToComponent (component "bs-dropdown/menu/link-to")) + link-to=(bs-default @linkToComponent (component "bs-link-to" class=(if (macroCondition (macroGetOwnConfig "isNotBS3")) "dropdown-item"))) + linkTo=(bs-default @linkToComponent (component "bs-link-to" class=(if (macroCondition (macroGetOwnConfig "isNotBS3")) "dropdown-item"))) divider=(bs-default @dividerComponent (component "bs-dropdown/menu/divider")) ) }} diff --git a/addon/components/bs-dropdown/menu/link-to.js b/addon/components/bs-dropdown/menu/link-to.js deleted file mode 100644 index c1f695fc5..000000000 --- a/addon/components/bs-dropdown/menu/link-to.js +++ /dev/null @@ -1,15 +0,0 @@ -import LinkComponent from '@ember/routing/link-component'; -import { classNames } from '@ember-decorators/component'; -import { macroCondition, getOwnConfig } from '@embroider/macros'; - -/** - - Extended `{{link-to}}` component for use within Dropdowns. - - @class DropdownMenuLinkTo - @namespace Components - @extends Ember.LinkComponent - @public - */ -@classNames(macroCondition(getOwnConfig().isNotBS3) ? 'dropdown-item' : '') -export default class DropdownMenuLinkTo extends LinkComponent {} diff --git a/addon/components/bs-link-to.hbs b/addon/components/bs-link-to.hbs new file mode 100644 index 000000000..8c79fee2a --- /dev/null +++ b/addon/components/bs-link-to.hbs @@ -0,0 +1,10 @@ + + {{yield}} + \ No newline at end of file diff --git a/addon/components/bs-link-to.js b/addon/components/bs-link-to.js new file mode 100644 index 000000000..84bb72d12 --- /dev/null +++ b/addon/components/bs-link-to.js @@ -0,0 +1,110 @@ +/* eslint-disable ember/classic-decorator-no-classic-methods */ +import Component from '@ember/component'; +import { tagName } from '@ember-decorators/component'; +import { inject as service } from '@ember/service'; +import { assert } from '@ember/debug'; +import ComponentChild from '../mixins/component-child'; +import { dependentKeyCompat } from '@ember/object/compat'; + +/** + This is largely copied from Ember.LinkComponent. It is used as extending from Ember.LinkComponent has been deprecated. + We need this to + * register ourselves to a parent component that needs to know `active` state due to Bootstrap markup requirements, see Nav/LinkTo + * continue supporting positional params until we can remove support + + @class LinkComponent + @namespace Components + @extends Component + @private +*/ +@tagName('') +class LinkComponent extends Component.extend(ComponentChild) { + @service('router') + router; + + @dependentKeyCompat + get active() { + if (!this.route) { + return false; + } + + // Ember < 3.22 does not correctly entangle autotracking with routing state changes, so we manually do that here + // See https://github.com/emberjs/ember.js/issues/19004 + // shamelessly stolen from https://github.com/rwjblue/ember-router-helpers/blob/master/addon/utils/track-active-route.js + + // ensure we recompute anytime `router.currentURL` changes + this.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) + this.router.currentRouteName; + + return this.router.isActive(this.route, ...this._models, { queryParams: this._query }); + } + + get _models() { + let { model, models } = this; + + if (model !== undefined) { + return [model]; + } else if (models !== undefined) { + assert('The `@models` argument must be an array.', Array.isArray(models)); + return models; + } else { + return []; + } + } + + get _query() { + return this.query ?? {}; + } + + // eslint-disable-next-line ember/no-component-lifecycle-hooks + didReceiveAttrs() { + super.didReceiveAttrs(...arguments); + + let { params } = this; + if (!params || params.length === 0) { + return; + } + + params = params.slice(); + + // taken from original Ember.LnkComponent + // Process the positional arguments, in order. + + // Skipping this, as we don't support this + // 1. Inline link title comes first, if present. + // if (!hasBlock) { + // this.set('linkTitle', params.shift()); + // } + + // 2. The last argument is possibly the `query` object. + let queryParams = params[params.length - 1]; + + if (queryParams && queryParams.isQueryParams) { + this.set('query', params.pop().values); + } else { + this.set('query', undefined); + } + + // 3. If there is a `route`, it is now at index 0. + if (params.length === 0) { + this.set('route', undefined); + } else { + this.set('route', params.shift()); + } + + // 4. Any remaining indices (if any) are `models`. + this.set('model', undefined); + this.set('models', params); + } +} + +LinkComponent.reopenClass({ + positionalParams: 'params', +}); + +export default LinkComponent; diff --git a/addon/components/bs-nav.hbs b/addon/components/bs-nav.hbs index 7ccd1aeee..883b50502 100644 --- a/addon/components/bs-nav.hbs +++ b/addon/components/bs-nav.hbs @@ -2,10 +2,9 @@ {{yield (hash item=(bs-default @itemComponent (component "bs-nav/item")) - link-to=(bs-default @linkToComponent (component "bs-nav/link-to")) - linkTo=(bs-default @linkToComponent (component "bs-nav/link-to")) + link-to=(bs-default @linkToComponent (component "bs-link-to" class=(if (macroCondition (macroGetOwnConfig "isNotBS3")) "nav-link"))) + linkTo=(bs-default @linkToComponent (component "bs-link-to" class=(if (macroCondition (macroGetOwnConfig "isNotBS3")) "nav-link"))) dropdown=(component (bs-default @dropdownComponent (component "bs-dropdown")) inNav=true htmlTag="li") ) }} - \ No newline at end of file diff --git a/addon/components/bs-nav/item.js b/addon/components/bs-nav/item.js index 0801d32e7..0f273f4b2 100644 --- a/addon/components/bs-nav/item.js +++ b/addon/components/bs-nav/item.js @@ -4,7 +4,7 @@ import { filter, filterBy, gt } from '@ember/object/computed'; import Component from '@ember/component'; import { action } from '@ember/object'; import { scheduleOnce } from '@ember/runloop'; -import LinkComponent from '@ember/routing/link-component'; +import LinkComponent from 'ember-bootstrap/components/bs-link-to'; import ComponentParent from 'ember-bootstrap/mixins/component-parent'; import overrideableCP from 'ember-bootstrap/utils/cp/overrideable'; import { assert } from '@ember/debug'; diff --git a/addon/components/bs-nav/link-to.js b/addon/components/bs-nav/link-to.js deleted file mode 100644 index cf87c8f5b..000000000 --- a/addon/components/bs-nav/link-to.js +++ /dev/null @@ -1,17 +0,0 @@ -import LinkComponent from '@ember/routing/link-component'; -import ComponentChild from 'ember-bootstrap/mixins/component-child'; -import { classNames } from '@ember-decorators/component'; -import { macroCondition, getOwnConfig } from '@embroider/macros'; - -/** - - Extended `{{link-to}}` component for use within Navs. - - @class NavLinkTo - @namespace Components - @extends Ember.LinkComponent - @uses Mixins.ComponentChild - @public - */ -@classNames(macroCondition(getOwnConfig().isNotBS3) ? 'nav-link' : '') -export default class NavLinkTo extends LinkComponent.extend(ComponentChild) {} diff --git a/addon/components/bs-navbar/link-to.hbs b/addon/components/bs-navbar/link-to.hbs new file mode 100644 index 000000000..03fd4b157 --- /dev/null +++ b/addon/components/bs-navbar/link-to.hbs @@ -0,0 +1,11 @@ + + {{yield}} + \ No newline at end of file diff --git a/addon/components/bs-navbar/link-to.js b/addon/components/bs-navbar/link-to.js index bdcaf558d..285258780 100644 --- a/addon/components/bs-navbar/link-to.js +++ b/addon/components/bs-navbar/link-to.js @@ -1,6 +1,5 @@ -import BsNavLinkToComponent from 'ember-bootstrap/components/bs-nav/link-to'; -import defaultValue from 'ember-bootstrap/utils/default-decorator'; - +import Component from '@glimmer/component'; +import { action } from '@ember/object'; /** * Extended `{{link-to}}` component for use within Navbars. * @@ -9,25 +8,18 @@ import defaultValue from 'ember-bootstrap/utils/default-decorator'; * @extends Components.NavLinkTo * @public */ -export default class NavbarLinkTo extends BsNavLinkToComponent { +export default class NavbarLinkTo extends Component { /** * @property collapseNavbar * @type {Boolean} * @default true * @public */ - @defaultValue - collapseNavbar = true; - - /** - * @event onCollapse - * @private - */ - onCollapse() {} - click() { - if (this.collapseNavbar) { - this.onCollapse(); + @action + onClick() { + if (this.args.collapseNavbar ?? true) { + this.args.onCollapse(); } } } diff --git a/app/components/bs-dropdown/menu/link-to.js b/app/components/bs-dropdown/menu/link-to.js deleted file mode 100644 index ca1ab9b14..000000000 --- a/app/components/bs-dropdown/menu/link-to.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from 'ember-bootstrap/components/bs-dropdown/menu/link-to'; diff --git a/app/components/bs-link-to.js b/app/components/bs-link-to.js new file mode 100644 index 000000000..079984eb6 --- /dev/null +++ b/app/components/bs-link-to.js @@ -0,0 +1 @@ +export { default } from 'ember-bootstrap/components/bs-link-to'; diff --git a/app/components/bs-nav/link-to.js b/app/components/bs-nav/link-to.js deleted file mode 100644 index 99b2678d4..000000000 --- a/app/components/bs-nav/link-to.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from 'ember-bootstrap/components/bs-nav/link-to'; diff --git a/tests/acceptance/bs-nav-link-test.js b/tests/acceptance/bs-nav-link-test.js index c2a145478..0384c774f 100644 --- a/tests/acceptance/bs-nav-link-test.js +++ b/tests/acceptance/bs-nav-link-test.js @@ -1,9 +1,6 @@ import { visit } from '@ember/test-helpers'; import { test, module } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import hasEmberVersion from '@ember/test-helpers/has-ember-version'; - -const supportsAngleBracketsLinkTo = hasEmberVersion(3, 10); module('Acceptance | bs-nav-link', function (hooks) { setupApplicationTest(hooks); @@ -19,16 +16,14 @@ module('Acceptance | bs-nav-link', function (hooks) { assert.dom(this.element.querySelectorAll('.nav li')[2]).hasClass('active'); }); - if (supportsAngleBracketsLinkTo) { - test('active @route property marks nav item as active', async function (assert) { - await visit('/acceptance/linkto/1'); - assert.dom(this.element.querySelectorAll('.nav li')[0]).hasClass('active'); - assert.dom(this.element.querySelectorAll('.nav li')[1]).hasNoClass('active'); - assert.dom(this.element.querySelectorAll('.nav li')[2]).hasClass('active'); - await visit('/acceptance/linkto/2'); - assert.dom(this.element.querySelectorAll('.nav li')[0]).hasNoClass('active'); - assert.dom(this.element.querySelectorAll('.nav li')[1]).hasClass('active'); - assert.dom(this.element.querySelectorAll('.nav li')[2]).hasClass('active'); - }); - } + test('active @route property marks nav item as active', async function (assert) { + await visit('/acceptance/linkto/1'); + assert.dom(this.element.querySelectorAll('.nav li')[0]).hasClass('active'); + assert.dom(this.element.querySelectorAll('.nav li')[1]).hasNoClass('active'); + assert.dom(this.element.querySelectorAll('.nav li')[2]).hasClass('active'); + await visit('/acceptance/linkto/2'); + assert.dom(this.element.querySelectorAll('.nav li')[0]).hasNoClass('active'); + assert.dom(this.element.querySelectorAll('.nav li')[1]).hasClass('active'); + assert.dom(this.element.querySelectorAll('.nav li')[2]).hasClass('active'); + }); }); diff --git a/tests/helpers/setup-no-deprecations.js b/tests/helpers/setup-no-deprecations.js index 32553caa7..451338ba9 100644 --- a/tests/helpers/setup-no-deprecations.js +++ b/tests/helpers/setup-no-deprecations.js @@ -9,8 +9,6 @@ const ignoredDeprecations = [ /Versions of modifier manager capabilities prior to 3\.22 have been deprecated/, /Usage of the Ember Global is deprecated./, /import .* directly from/, - // this will be fixed in a future PR... - /Using Ember.LinkComponent/, ]; export default function setupNoDeprecations({ beforeEach, afterEach }) { diff --git a/tests/integration/components/bs-dropdown/menu/link-to-test.js b/tests/integration/components/bs-dropdown/menu/link-to-test.js index 663fc18e2..07d5297e4 100644 --- a/tests/integration/components/bs-dropdown/menu/link-to-test.js +++ b/tests/integration/components/bs-dropdown/menu/link-to-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; +import { click, render } from '@ember/test-helpers'; import { testBS3, testNotBS3 } from '../../../../helpers/bootstrap'; import hbs from 'htmlbars-inline-precompile'; import setupNoDeprecations from '../../../../helpers/setup-no-deprecations'; @@ -12,32 +12,43 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) { }); testBS3('it has correct markup', async function (assert) { - // Template block usage: await render(hbs` - {{#bs-dropdown/menu/link-to "index"}} - template block text - {{/bs-dropdown/menu/link-to}} - `); - - assert.dom('*').hasText('template block text'); + + Dropdown + + template block text + + `); + await click('button'); + + assert.dom('a').hasText('template block text'); assert.dom('a.dropdown-item').doesNotExist('renders as plain element with no dropdown item class'); }); testNotBS3('it has correct markup', async function (assert) { - // Template block usage: await render(hbs` - {{#bs-dropdown/menu/link-to "index"}} - template block text - {{/bs-dropdown/menu/link-to}} - `); - - assert.dom('*').hasText('template block text'); + + Dropdown + + template block text + + `); + await click('button'); + + assert.dom('a').hasText('template block text'); assert.dom('a.dropdown-item').exists({ count: 1 }, 'renders as plain element with dropdown item class'); }); module('positional params', function () { test('simple route link', async function (assert) { - await render(hbs`{{#bs-dropdown/menu/link-to "index"}}Link{{/bs-dropdown/menu/link-to}}`); + await render(hbs` + + Dropdown + + {{#menu.link-to "index"}}Link{{/menu.link-to}} + + `); + await click('button'); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasText('Link'); @@ -45,16 +56,28 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) { }); test('link with model', async function (assert) { - await render( - hbs`{{#bs-dropdown/menu/link-to "acceptance.link" "1" (query-params foo="bar")}}Link{{/bs-dropdown/menu/link-to}}` - ); + await render(hbs` + + Dropdown + + {{#menu.link-to "acceptance.link" "1" (query-params foo="bar")}}Link{{/menu.link-to}} + + `); + await click('button'); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasAttribute('href', '/acceptance/link/1?foo=bar'); }); test('disabled link', async function (assert) { - await render(hbs`{{#bs-dropdown/menu/link-to "index" disabled=true}}Link{{/bs-dropdown/menu/link-to}}`); + await render(hbs` + + Dropdown + + {{#menu.link-to "index" disabled=true}}Link{{/menu.link-to}} + + `); + await click('button'); assert.dom('a').hasClass('disabled'); }); @@ -64,7 +87,14 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) { setupNoDeprecations(hooks); test('simple route link', async function (assert) { - await render(hbs`Link`); + await render(hbs` + + Dropdown + + Link + + `); + await click('button'); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasText('Link'); @@ -72,16 +102,28 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) { }); test('link with model', async function (assert) { - await render( - hbs`Link` - ); + await render(hbs` + + Dropdown + + Link + + `); + await click('button'); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasAttribute('href', '/acceptance/link/1?foo=bar'); }); test('disabled link', async function (assert) { - await render(hbs`Link`); + await render(hbs` + + Dropdown + + Link + + `); + await click('button'); assert.dom('a').hasClass('disabled'); }); diff --git a/tests/integration/components/bs-nav-test.js b/tests/integration/components/bs-nav-test.js index fa5815078..234a767e5 100644 --- a/tests/integration/components/bs-nav-test.js +++ b/tests/integration/components/bs-nav-test.js @@ -47,8 +47,8 @@ module('Integration | Component | bs-nav', function (hooks) { }); test('it exposes contextual components', async function (assert) { - // eslint-disable-next-line ember/no-private-routing-service - this.owner.lookup('router:main').setupRouter(); + this.owner.setupRouter(); + await render(hbs` @@ -73,6 +73,8 @@ module('Integration | Component | bs-nav', function (hooks) { }); test('it passes accessibility checks', async function (assert) { + this.owner.setupRouter(); + await render(hbs` diff --git a/tests/integration/components/bs-nav/item-test.js b/tests/integration/components/bs-nav/item-test.js index 3074c4f18..158ec18b7 100644 --- a/tests/integration/components/bs-nav/item-test.js +++ b/tests/integration/components/bs-nav/item-test.js @@ -17,9 +17,11 @@ module('Integration | Component | bs-nav/item', function (hooks) { test('it has correct markup', async function (assert) { await render(hbs` - - template block text - + + + template block text + + `); assert.dom('*').hasText('template block text', 'Shows block content'); @@ -31,36 +33,60 @@ module('Integration | Component | bs-nav/item', function (hooks) { test('it does not have aria role="presentation"', async function (assert) { // Should not have role="presentation" even so Bootstrap 3 docs have it. // This was discussed at https://github.com/kaliber5/ember-bootstrap/pull/782. - await render(hbs``); + await render(hbs` + + + template block text + + + `); assert.dom('li').doesNotHaveAttribute('role'); }); test('can be disabled', async function (assert) { - await render(hbs``); - + await render(hbs` + + + template block text + + + `); assert.dom('li').hasClass('disabled', 'has disabled class'); }); test('can be active', async function (assert) { - await render(hbs``); - + await render(hbs` + + + template block text + + + `); assert.dom('li').hasClass('active', 'has active class'); }); test('disabled link makes nav item disabled', async function (assert) { await render(hbs` - - Test - - `); + + + Link + + + `); assert.dom('li').hasClass('disabled', 'has disabled class'); }); test('clicking item calls onClick action', async function (assert) { let action = sinon.spy(); this.actions.click = action; - await render(hbs``); + await render(hbs` + + + template block text + + + `); await click('li'); assert.ok(action.calledOnce, 'action has been called'); diff --git a/tests/integration/components/bs-nav/link-to-test.js b/tests/integration/components/bs-nav/link-to-test.js index 810f0a2b8..37cf93a4e 100644 --- a/tests/integration/components/bs-nav/link-to-test.js +++ b/tests/integration/components/bs-nav/link-to-test.js @@ -13,7 +13,13 @@ module('Integration | Component | bs-nav/link-to', function (hooks) { module('positional params', function () { test('simple route link', async function (assert) { - await render(hbs`{{#bs-nav/link-to "index"}}Link{{/bs-nav/link-to}}`); + await render(hbs` + + + {{#nav.link-to "index"}}Link{{/nav.link-to}} + + + `); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasText('Link'); @@ -21,21 +27,37 @@ module('Integration | Component | bs-nav/link-to', function (hooks) { }); test('link with model', async function (assert) { - await render(hbs`{{#bs-nav/link-to "acceptance.link" "1" (query-params foo="bar")}}Link{{/bs-nav/link-to}}`); + await render(hbs` + + + {{#nav.link-to "acceptance.link" "1" (query-params foo="bar")}}Link{{/nav.link-to}} + + + `); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasAttribute('href', '/acceptance/link/1?foo=bar'); }); testNotBS3('link has nav-link class', async function (assert) { - await render(hbs`{{#bs-nav/link-to "index"}}Link{{/bs-nav/link-to}}`); - + await render(hbs` + + + {{#nav.link-to "index"}}Link{{/nav.link-to}} + + + `); assert.dom('a').hasClass('nav-link'); }); test('disabled link', async function (assert) { - await render(hbs`{{#bs-nav/link-to "index" disabled=true}}Link{{/bs-nav/link-to}}`); - + await render(hbs` + + + {{#nav.link-to "index" disabled=true}}Link{{/nav.link-to}} + + + `); assert.dom('a').hasClass('disabled'); }); }); @@ -44,7 +66,13 @@ module('Integration | Component | bs-nav/link-to', function (hooks) { setupNoDeprecations(hooks); test('simple route link', async function (assert) { - await render(hbs`Link`); + await render(hbs` + + + Link + + + `); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasText('Link'); @@ -52,23 +80,38 @@ module('Integration | Component | bs-nav/link-to', function (hooks) { }); test('link with model', async function (assert) { - await render( - hbs`Link` - ); + await render(hbs` + + + Link + + + `); assert.dom('a').exists({ count: 1 }); assert.dom('a').hasAttribute('href', '/acceptance/link/1?foo=bar'); }); testNotBS3('link has nav-link class', async function (assert) { - await render(hbs`Link`); + await render(hbs` + + + Link + + + `); assert.dom('a').hasClass('nav-link'); }); test('disabled link', async function (assert) { - await render(hbs`Link`); - + await render(hbs` + + + Link + + + `); assert.dom('a').hasClass('disabled'); }); });