Merge pull request #15265 from bekzod/compute-active

[BUGFIX] fixed issue when passing `false` to `activeClass` would break things
emberjs · Sep 14, 2017 · b27344a · b27344a
2 parents 88a9a97 + fe751ed
commit b27344a
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 7 deletions.
diff --git a/packages/ember-glimmer/lib/components/link-to.js b/packages/ember-glimmer/lib/components/link-to.js
@@ -545,7 +545,7 @@ const LinkComponent = EmberComponent.extend({
     }
   }),
 
-  _computeActive(routerState) {
+  _isActive(routerState) {
     if (get(this, 'loading')) { return false; }
 
     let routing = get(this, '_routing');
@@ -563,7 +563,7 @@ const LinkComponent = EmberComponent.extend({
 
     for (let i = 0; i < currentWhen.length; i++) {
       if (routing.isActiveForRoute(models, resolvedQueryParams, currentWhen[i], routerState, isCurrentWhenSpecified)) {
-        return get(this, 'activeClass');
+        return true;
       }
     }
 
@@ -584,31 +584,37 @@ const LinkComponent = EmberComponent.extend({
     @property active
     @private
   */
-  active: computed('attrs.params', '_routing.currentState', function computeLinkToComponentActive() {
+  active: computed('attrs.params', '_active', function computeLinkToComponentActiveClass() {
     let currentState = get(this, '_routing.currentState');
     if (!currentState) { return false; }
 
-    return this._computeActive(currentState);
+    return this.get('_active') ? get(this, 'activeClass') : false;
+  }),
+
+  _active: computed('_routing.currentState', function computeLinkToComponentActive() {
+    let currentState = get(this, '_routing.currentState');
+    if (!currentState) { return false; }
+    return this._isActive(currentState);
   }),
 
   willBeActive: computed('_routing.targetState', function computeLinkToComponentWillBeActive() {
     let routing = get(this, '_routing');
     let targetState = get(routing, 'targetState');
     if (get(routing, 'currentState') === targetState) { return; }
 
-    return !!this._computeActive(targetState);
+    return this._isActive(targetState);
   }),
 
   transitioningIn: computed('active', 'willBeActive', function computeLinkToComponentTransitioningIn() {
-    if (get(this, 'willBeActive') === true && !get(this, 'active')) {
+    if (get(this, 'willBeActive') === true && !get(this, '_active')) {
       return 'ember-transitioning-in';
     } else {
       return false;
     }
   }),
 
   transitioningOut: computed('active', 'willBeActive', function computeLinkToComponentTransitioningOut() {
-    if (get(this, 'willBeActive') === false && get(this, 'active')) {
+    if (get(this, 'willBeActive') === false && get(this, '_active')) {
       return 'ember-transitioning-out';
     } else {
       return false;

diff --git a/packages/ember/tests/helpers/link_to_test/link_to_transitioning_classes_test.js b/packages/ember/tests/helpers/link_to_test/link_to_transitioning_classes_test.js
@@ -20,11 +20,13 @@ moduleFor('The {{link-to}} helper: .transitioning-in .transitioning-out CSS clas
 
     this.aboutDefer = RSVP.defer();
     this.otherDefer = RSVP.defer();
+    this.newsDefer = RSVP.defer();
     let _this = this;
 
     this.router.map(function() {
       this.route('about');
       this.route('other');
+      this.route('news');
     });
 
     this.add('route:about', Route.extend({
@@ -39,11 +41,18 @@ moduleFor('The {{link-to}} helper: .transitioning-in .transitioning-out CSS clas
       }
     }));
 
+    this.add('route:news', Route.extend({
+      model() {
+        return _this.newsDefer.promise;
+      }
+    }));
+
     this.addTemplate('application',`
       {{outlet}}
       {{link-to 'Index' 'index' id='index-link'}}
       {{link-to 'About' 'about' id='about-link'}}
       {{link-to 'Other' 'other' id='other-link'}}
+      {{link-to 'News' 'news' activeClass=false id='news-link'}}
     `);
 
     this.visit('/');
@@ -53,6 +62,7 @@ moduleFor('The {{link-to}} helper: .transitioning-in .transitioning-out CSS clas
     super.teardown();
     this.aboutDefer = null;
     this.otherDefer = null;
+    this.newsDefer = null;
   }
 
   ['@test while a transition is underway'](assert) {
@@ -88,6 +98,41 @@ moduleFor('The {{link-to}} helper: .transitioning-in .transitioning-out CSS clas
     assertHasNoClass(assert, $about, 'ember-transitioning-out');
     assertHasNoClass(assert, $other, 'ember-transitioning-out');
   }
+
+  ['@test while a transition is underway with activeClass is false'](assert) {
+    let $index = this.$('#index-link');
+    let $news = this.$('#news-link');
+    let $other = this.$('#other-link');
+
+    $news.click();
+
+    assertHasClass(assert, $index, 'active');
+    assertHasNoClass(assert, $news, 'active');
+    assertHasNoClass(assert, $other, 'active');
+
+    assertHasNoClass(assert, $index, 'ember-transitioning-in');
+    assertHasClass(assert, $news, 'ember-transitioning-in');
+    assertHasNoClass(assert, $other, 'ember-transitioning-in');
+
+    assertHasClass(assert, $index, 'ember-transitioning-out');
+    assertHasNoClass(assert, $news, 'ember-transitioning-out');
+    assertHasNoClass(assert, $other, 'ember-transitioning-out');
+
+    this.runTask(() => this.newsDefer.resolve());
+
+    assertHasNoClass(assert, $index, 'active');
+    assertHasNoClass(assert, $news, 'active');
+    assertHasNoClass(assert, $other, 'active');
+
+    assertHasNoClass(assert, $index, 'ember-transitioning-in');
+    assertHasNoClass(assert, $news, 'ember-transitioning-in');
+    assertHasNoClass(assert, $other, 'ember-transitioning-in');
+
+    assertHasNoClass(assert, $index, 'ember-transitioning-out');
+    assertHasNoClass(assert, $news, 'ember-transitioning-out');
+    assertHasNoClass(assert, $other, 'ember-transitioning-out');
+  }
+
 });
 
 moduleFor(`The {{link-to}} helper: .transitioning-in .transitioning-out CSS classes - nested link-to's`, class extends ApplicationTestCase {