Skip to content

Commit

Permalink
Remove relationship between bs-nav-item / bs-link-to (ember-bootstrap…
Browse files Browse the repository at this point in the history
…#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 ember-bootstrap#2012
  • Loading branch information
SanderKnauff authored Dec 18, 2023
1 parent 6888aa0 commit 677304e
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 148 deletions.
3 changes: 1 addition & 2 deletions addon/components/bs-link-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion addon/components/bs-nav/item.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{!-- template-lint-disable no-invalid-interactive --}}
<li class="nav-item {{if this.disabled "disabled"}} {{if this.active "active"}}" ...attributes {{on "click" this.handleClick}}>
<li class="nav-item" ...attributes {{on "click" this.handleClick}}>
{{yield}}
</li>
86 changes: 1 addition & 85 deletions addon/components/bs-nav/item.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
*
Expand All @@ -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);
}
}
30 changes: 0 additions & 30 deletions tests/acceptance/bs-nav-link-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/fastboot/tab-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
Expand Down
27 changes: 0 additions & 27 deletions tests/integration/components/bs-nav/item-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`<BsNav as |nav|>
<nav.item @disabled={{true}}>
template block text
</nav.item>
</BsNav>`);
assert.dom('li').hasClass('disabled', 'has disabled class');
});

test('can be active', async function (assert) {
await render(hbs`<BsNav as |nav|>
<nav.item @active={{true}}>
template block text
</nav.item>
</BsNav>`);
assert.dom('li').hasClass('active', 'has active class');
});

test('disabled link makes nav item disabled', async function (assert) {
await render(hbs`<BsNav as |nav|>
<nav.item>
<nav.link-to @route='application' @disabled={{true}}>Link</nav.link-to>
</nav.item>
</BsNav>`);
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;
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/components/bs-tab-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 677304e

Please sign in to comment.