Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngClassOdd/Even doesn't work when reversing orderBy filter #1563

Closed

Description

@rgaskill

When reversing the order of the orderByFilter the ngClassOdd and ngClassEven don't update. The unit test thet demonstrates the issue is here:

http://plnkr.co/edit/kXJcDC

Activity

  1. jtymes commented on Nov 13, 2012

    @jtymes
    Contributor

    Just want to note for others that this is related to #1519

    Something is clearly broken with these directives.

  2. pkozlowski-opensource commented on Nov 27, 2012

    @pkozlowski-opensource
    Member

    @rgaskill It was fixed in the latest version of AngularJS (1.0.3 / 1.1.1) through this PR:
    #1602

    Working plunk here: http://plnkr.co/edit/R1Zpzt?p=preview

  3. pkozlowski-opensource commented on Nov 27, 2012

    @pkozlowski-opensource
    Member

    BTW: nice way of exposing a bug by attaching test!

  4. rgaskill commented on Nov 27, 2012

    @rgaskill
    Author

    Thanks Shouldn't the test get pulled into the project?

  5. pkozlowski-opensource commented on Nov 27, 2012

    @pkozlowski-opensource
    Member

    @rgaskill I think that @IgorMinar added tests for this but if you find anything not covered with tests please open a separate PR with tests only. Thnx once again for the bug report and the PR, it helped to see the real problem and brainstorm different solutions.

  6. rgaskill commented on Nov 27, 2012

    @rgaskill
    Author

    @pkozlowski-opensource I have created PR: #1613. It appears to me the problem was fixed once before #1076 but then it broke again. The tests originally added with #1076 didn't catch the break because the sort test didn't use the orderBy filter. This new test does.

    Thank you for all the work is being done to keep this project moving forward!

  7. IgorMinar commented on Nov 28, 2012

    @IgorMinar
    Contributor

    Isn't this fixed in master?

  8. pkozlowski-opensource commented on Nov 28, 2012

    @pkozlowski-opensource
    Member

    @IgorMinar It is fixed in master and works OK. It is just that @rgaskill opened #1613 with an additional test. I was pinging you to see if this test is needed or maybe the functionality is already covered by other tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions