From 677304e8516a747deeb9ac83858aa92dc2cb2f1a Mon Sep 17 00:00:00 2001 From: Sander Knauff Date: Mon, 18 Dec 2023 23:20:25 +0100 Subject: [PATCH] Remove relationship between bs-nav-item / bs-link-to (#2040) The active and disabled parameters were used in the styling of Bootstrap 3. Since this parent/child component relationship is hindering the switch to Glimmer components and we do not support Bootstrap 3 anymore, we assume it can safely be removed. For more information, see the discussion in #2012 --- addon/components/bs-link-to.js | 3 +- addon/components/bs-nav/item.hbs | 2 +- addon/components/bs-nav/item.js | 86 +------------------ tests/acceptance/bs-nav-link-test.js | 30 ------- tests/fastboot/tab-test.js | 2 +- .../components/bs-nav/item-test.js | 27 ------ tests/integration/components/bs-tab-test.js | 4 +- 7 files changed, 6 insertions(+), 148 deletions(-) diff --git a/addon/components/bs-link-to.js b/addon/components/bs-link-to.js index 1c2421f8b..f0dc8f455 100644 --- a/addon/components/bs-link-to.js +++ b/addon/components/bs-link-to.js @@ -3,7 +3,6 @@ import Component from '@ember/component'; import { tagName } from '@ember-decorators/component'; import { inject as service } from '@ember/service'; import { assert, deprecate } from '@ember/debug'; -import ComponentChild from '../mixins/component-child'; import { dependentKeyCompat } from '@ember/object/compat'; /** @@ -18,7 +17,7 @@ import { dependentKeyCompat } from '@ember/object/compat'; @private */ @tagName('') -class LinkComponent extends Component.extend(ComponentChild) { +class LinkComponent extends Component { @service('router') router; diff --git a/addon/components/bs-nav/item.hbs b/addon/components/bs-nav/item.hbs index d261cf15f..590a51d5b 100644 --- a/addon/components/bs-nav/item.hbs +++ b/addon/components/bs-nav/item.hbs @@ -1,4 +1,4 @@ {{!-- template-lint-disable no-invalid-interactive --}} -
  • +
  • {{yield}}
    • \ No newline at end of file diff --git a/addon/components/bs-nav/item.js b/addon/components/bs-nav/item.js index 393c4fe36..f77aa32d4 100644 --- a/addon/components/bs-nav/item.js +++ b/addon/components/bs-nav/item.js @@ -1,17 +1,10 @@ import { tagName } from '@ember-decorators/component'; -import { observes } from '@ember-decorators/object'; -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-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'; import deprecateSubclassing from 'ember-bootstrap/utils/deprecate-subclassing'; /** - Component for each item within a [Components.Nav](Components.Nav.html) component. Have a look there for examples. @class NavItem @@ -22,63 +15,7 @@ import deprecateSubclassing from 'ember-bootstrap/utils/deprecate-subclassing'; */ @tagName('') @deprecateSubclassing -export default class NavItem extends Component.extend(ComponentParent) { - /** - * Render the nav item as disabled (see [Bootstrap docs](http://getbootstrap.com/components/#nav-disabled-links)). - * By default it will look at any nested `link-to` components and make itself disabled if there is a disabled link. - * See the [link-to API](http://emberjs.com/api/classes/Ember.Templates.helpers.html#toc_disabling-the-code-link-to-code-component) - * - * @property disabled - * @type boolean - * @public - */ - @overrideableCP('_disabled', function () { - return this._disabled; - }) - disabled; - - _disabled = false; - - /** - * Render the nav item as active. - * By default it will look at any nested `link-to` components and make itself active if there is an active link - * (i.e. the link points to the current route). - * See the [link-to API](http://emberjs.com/api/classes/Ember.Templates.helpers.html#toc_handling-current-route) - * - * @property active - * @type boolean - * @public - */ - @overrideableCP('_active', function () { - return this._active; - }) - active; - - _active = false; - - /** - * Collection of all `Ember.LinkComponent`s that are children - * - * @property childLinks - * @private - */ - @filter('children', function (view) { - return view instanceof LinkComponent; - }) - childLinks; - - @filterBy('childLinks', 'active') - activeChildLinks; - - @gt('activeChildLinks.length', 0) - hasActiveChildLinks; - - @filterBy('childLinks', 'disabled') - disabledChildLinks; - - @gt('disabledChildLinks.length', 0) - hasDisabledChildLinks; - +export default class NavItem extends Component { /** * Called when clicking the nav item * @@ -99,26 +36,5 @@ export default class NavItem extends Component.extend(ComponentParent) { 'You cannot pass both `@model` and `@models` to a nav item component!', !model || !models, ); - - this.activeChildLinks; - this.disabledChildLinks; - } - - @observes('activeChildLinks.[]') - _observeActive() { - scheduleOnce('afterRender', this, this._updateActive); - } - - _updateActive() { - this.set('_active', this.hasActiveChildLinks); - } - - @observes('disabledChildLinks.[]') - _observeDisabled() { - scheduleOnce('afterRender', this, this._updateDisabled); - } - - _updateDisabled() { - this.set('_disabled', this.hasDisabledChildLinks); } } diff --git a/tests/acceptance/bs-nav-link-test.js b/tests/acceptance/bs-nav-link-test.js index bf120dbcd..384152e53 100644 --- a/tests/acceptance/bs-nav-link-test.js +++ b/tests/acceptance/bs-nav-link-test.js @@ -5,36 +5,6 @@ import { setupApplicationTest } from 'ember-qunit'; module('Acceptance | bs-nav-link', function (hooks) { setupApplicationTest(hooks); - test('active link-to component marks nav item as active', async function (assert) { - await visit('/acceptance/link/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/link/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'); - }); - test('href exists in anchor inside nav item', async function (assert) { await visit('/acceptance/link/1'); assert diff --git a/tests/fastboot/tab-test.js b/tests/fastboot/tab-test.js index 392f1cd22..bd4b0dd3a 100644 --- a/tests/fastboot/tab-test.js +++ b/tests/fastboot/tab-test.js @@ -9,7 +9,7 @@ module('FastBoot | tab', function (hooks) { assert.dom('ul.nav.nav-tabs').exists(); assert.dom('ul.nav li').exists({ count: 2 }); - assert.dom('ul.nav li.active').exists({ count: 1 }); + assert.dom('ul.nav li a.active').exists({ count: 1 }); assert.dom('.tab-pane').exists({ count: 2 }); assert.dom('.tab-pane.active').exists({ count: 1 }); }); diff --git a/tests/integration/components/bs-nav/item-test.js b/tests/integration/components/bs-nav/item-test.js index 179f32277..71c2c329b 100644 --- a/tests/integration/components/bs-nav/item-test.js +++ b/tests/integration/components/bs-nav/item-test.js @@ -47,33 +47,6 @@ module('Integration | Component | bs-nav/item', function (hooks) { assert.dom('li').doesNotHaveAttribute('role'); }); - test('can be disabled', async function (assert) { - await render(hbs` - - template block text - -`); - assert.dom('li').hasClass('disabled', 'has disabled class'); - }); - - test('can be active', async function (assert) { - 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` - - 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; diff --git a/tests/integration/components/bs-tab-test.js b/tests/integration/components/bs-tab-test.js index ce9c3ec9e..9298469c8 100644 --- a/tests/integration/components/bs-tab-test.js +++ b/tests/integration/components/bs-tab-test.js @@ -17,10 +17,10 @@ module('Integration | Component | bs-tab', function (hooks) { }); function assertActiveTab(assert, tabIndex, active = true) { - if (this.element.querySelectorAll('ul.nav.nav-tabs li').length > 0) { + if (this.element.querySelectorAll('ul.nav.nav-tabs li a').length > 0) { assert.equal( this.element - .querySelector(`ul.nav.nav-tabs li:nth-child(${tabIndex + 1})`) + .querySelector(`ul.nav.nav-tabs li:nth-child(${tabIndex + 1}) a`) .classList.contains('active'), active, active ? 'tab is active' : 'tab is inactive',