feat(uiSrefActive): allow active & active-eq on same element

[joshuahiggins]

This pull request will allow the usage of both ui-sref-active and ui-sref-active-eq on the same element.

One use case includes menus with expanding states where a parent element can be both active and open, such as in this example:

<ul>
  <li ui-sref-active="open" ui-sref-active-eq="active">
    <a ui-sref="home"></a>
    <ul>
      <li ui-sref-active-eq="active">
        <a ui-sref="home.view1"></a>
      </li>
      <li ui-sref-active-eq="active">
        <a ui-sref="home.view2"></a>
      </li>
    </ul>
  </li>
  <li ui-sref-active="open" ui-sref-active-eq="active">
    <a ui-sref="account"></a>
    <ul>
      <li ui-sref-active-eq="active">
        <a ui-sref="account.view1"></a>
      </li>
      <li ui-sref-active-eq="active">
        <a ui-sref="account.view2"></a>
      </li>
      <li ui-sref-active-eq="active">
        <a ui-sref="account.view3"></a>
      </li>
    </ul>
  </li>
</ul>

When the account child views are active, the parent view should have a class of "open". However, when the parent is active, then the parent element should have both "open" and "active" classes.

This pull request exposes and squashes a race condition bug that's present today, #1997.

I apologize that this is a duplicate of a pull request I previously closed. I trashed that request due to failing tests before learning the quirks of $timeout.flush() in Karma. After digging around, I noticed that you had a comment explaining a workaround. Reopened after getting all the tests sorted out and passing.

[nateabele]

Seems legit.

apologize that this is a duplicate of a pull request I previously closed. I trashed that request due to failing tests before learning the quirks of $timeout.flush() in Karma. After digging around, I noticed that you had a comment explaining a workaround. Reopened after getting all the tests sorted out and passing.

So, all the extra calls to timeoutFlush() are because things fail otherwise?

[joshuahiggins]

So, all the extra calls to timeoutFlush() are because things fail otherwise?

The calls to $timeout.flush() are necessary as a $timeout was added to the directive to ensure that the element has finished rendering to the DOM before attempting to add a class to it. This is an issue that is currently present in the code and can be seen when using ui-sref-active in conjunction with ng-repeat, as well as other use cases. It just happens that this feature request highlighted that issue and caused the issue to happen more regularly.

That being said, running $q.flush() -- which is just $rootScope.digest() -- after $timeout.flush() should be redundant as $timeout will trigger an $apply() which triggers $rootScope.digest() anyway. By that logic, I replaced $q.flush() with timeoutFlush() in all the ui-sref-active[-eq] tests and it passed all karma:unit tests, but failed some karma:past and karma:unstable failures.

I am honestly not familiar enough with Angular pre 1.2.x versions to understand why $timeout.flush() wouldn't satisfy the digest needs of these tests. On that note, I left $q.flush() in place and just added the timeoutFlush() method after on all tests relating to ui-sref-active[-eq] and all prior versions of Angular just..... sorted themselves out. All tests pass with flying colors, despite the redundancy of the flushes. Forgive my naivety on the topic!

[nateabele]

@joshuahiggins No, it's fine, I was just checking. Thanks for the detailed explanation.

@christopherthielen Looks good to me. You? I don't think it should interfere with any future plans.

[christopherthielen]

👍

[joshuahiggins]

Any thoughts on when the next minor version release might be?