Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for https://github.com/pineconellc/angular-foundation/issues/61 #125

Merged
merged 6 commits into from
Oct 24, 2014

Conversation

a-lucas
Copy link
Contributor

@a-lucas a-lucas commented Oct 9, 2014

The issue #61 is hard to replicate.

To replicate it, you need to have a very large page, with lots of data and controllers.
(I had this sisue with one page and 25 controllers who each inherits the same main profile controller )

On the directive init, attrs.active is true but unparsed.
The expression is complex and has to wait all the other controllers to initialize to get evaluated correctly.

getActive = $parse(attrs.active); 

takes time to parse, but at the same time, the $watch is triggered.

This means the 'setActive' function is undefined in the $watch for this specific case where the $parse takes too long to eval.

@jbrowning
Copy link
Member

Thanks @a-lucas.

  1. Please fix spacing
  2. Angular provides angular.isFunction which would be useful here

@jbrowning
Copy link
Member

Please convert your tabs to spaces as requested by Semaphore. 😉

Merge remote-tracking branch 'upstream/master'
@a-lucas
Copy link
Contributor Author

a-lucas commented Oct 24, 2014

@jbrowning all done, it took me a while because I was learning the whole phantomjs / karma / jasmine unit testing procedure in the last 2 weeks, and I took this complex project as a learning base.
Unit testing is awesome!

@jbrowning
Copy link
Member

👍 Thanks for your contribution @a-lucas.

jbrowning added a commit that referenced this pull request Oct 24, 2014
@jbrowning jbrowning merged commit f8ef93c into yalabot:master Oct 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants