Skip to content

Commit

Permalink
Returns false instead of null to avoid null.toString().
Browse files Browse the repository at this point in the history
  • Loading branch information
Gaku Ueda committed Dec 6, 2013
1 parent ea03a14 commit 764b914
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/viewDirective.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

$ViewDirective.$inject = ['$state', '$compile', '$controller', '$injector', '$anchorScroll'];
function $ViewDirective( $state, $compile, $controller, $injector, $anchorScroll) {
var $animator = $injector.has('$animator') ? $injector.get('$animator') : null;
var $animator = $injector.has('$animator') ? $injector.get('$animator') : false;

This comment has been minimized.

Sign in to view

Copy link
@caitp

caitp Dec 7, 2013

Contributor

should probably retag this release... apparently people (or at least one person on IRC) is running into this bug, and it basically breaks any app that isn't including $animator. Kind of a bummer.

Maybe a regression test is in order here, too
var viewIsUpdating = false;

var directive = {
Expand Down

7 comments on commit 764b914

@nateabele
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caitp Yeah, that's my plan. I need to figure out how to automate running the test suite against multiple versions of Angular, which I haven't had the time to research yet.

@nateabele
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ProLoser @PascalPrecht Hey guys, any advice or guidance on setting up Karma to run tests against multiple versions of Angular?

@caitp
Copy link
Contributor

@caitp caitp commented on 764b914 Dec 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nateabele --- I'll see how difficult it is to write a Karma plugin which will install different versions of bower components before running a suite --- If a plugin like that exists, then it should be trivial to just have multiple configs (as in Angular core) to run variations on the test suites

...

We may need to write this as a really ugly grunt task for now, and that kind of sucks... The plugin is writeable, but the problem is that I don't think I can really get around the asynchronous nature of the bower API to make it work, and I can't find any way to make karma plugins play nicely with async plugins. I've opened an issue about it

@caitp
Copy link
Contributor

@caitp caitp commented on 764b914 Dec 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess the quickest thing to do would be to just check different versions of angular + angular-animate into the tree, and run the tests with different versions of them similar to AngularJS's jqlite vs jquery tests.

It kind of sucks having to check them into the tree, but I'm not sure how long it will be before asynchronous plugins are a thing

@g00fy-
Copy link

@g00fy- g00fy- commented on 764b914 Dec 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK 1.1.X releases were never production ready, so I personally don't see any reason in supporting outdated development versions.

@caitp
Copy link
Contributor

@caitp caitp commented on 764b914 Dec 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true @g00fy-, they were never marked as stable. However the 1.2 animation code is not in master yet, and people are apparently using the unstable stuff.

Specifying a hard deadline for the removal of the 1.1.5 stuff is probably adequate, but I think it would still be good to make sure this stuff works well with 1.0.8 for a while, even without ngAnimate.

@incrop
Copy link

@incrop incrop commented on 764b914 Dec 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@g00fy- Some people may use ui-route without using ngAnimate module at all.

Please sign in to comment.