Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

#516 Tooltip $apply already in progress #517

Closed
wants to merge 1 commit into from

Conversation

blabno
Copy link

@blabno blabno commented Jun 12, 2013

No description provided.

scope.$apply( show );
if (!scope.$$phase) {
scope.$apply(show);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably use $timeout instead of this :-)

$timeout(show)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ajoslin Agreed.

@pkozlowski-opensource
Copy link
Member

@ajoslin @joshdmiller scope.$$phase is a pure hack. In fact it is going to be removed from AngularJS so there will be no check and no "apply already in progress" errors. In short - in the very near future this problem will naturally go away.

But we are here and now and still scope.$$phase is a pure hack :-) Honestly I'm not sure how we are getting into this scenario, description in #516 is not super-clear for me.

While I agree that $timeout is better it is still not ideal as we are allowing browser to go into the rendering mode and just after switching back to JS to finish model calculations and go to painting once again.

I would propose to close this PR (as it is not in the state to be merged) and continue the discussion in #516

@pkozlowski-opensource
Copy link
Member

@blabno based on the discussion above it can't be merged as-is. Going to close for now, let's move the discussion to #516

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants