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

fix(tooltip): Use only once, don't use an empty transclusion fn #2825

Closed
wants to merge 1 commit into from
Closed

fix(tooltip): Use only once, don't use an empty transclusion fn #2825

wants to merge 1 commit into from

Conversation

Igosuki
Copy link
Contributor

@Igosuki Igosuki commented Oct 13, 2014

The additional $digest and empty transclusion seem unnecessary in 1.2 and are breaking changes in 1.3 (ng-class breaks).

@pkozlowski-opensource
Copy link
Member

Thnx @Igosuki !

ulle pushed a commit to ulle/bootstrap that referenced this pull request Oct 22, 2014
@chrisirhc
Copy link
Contributor

@Igosuki , could you post a Plunker demonstrating this breakage? I don't see it and now this change actually causes symptoms demonstrated in #2935 . This change is actually incorrect as it never binds the template to a new scope after the first time it's linked, since no new clones are created.

I've checked the documentation and I don't see this breakage that you mention.
After reverting change, I also got the test case mentioned in #2935 working:
http://plnkr.co/edit/l49BH8DMwHqKuPjgZ5qn?p=preview

@chrisirhc chrisirhc reopened this Nov 8, 2014
@chrisirhc
Copy link
Contributor

I'll be reverting this change on master. If you have a test case for this change, please do post it so I can look into it.

@Igosuki
Copy link
Contributor Author

Igosuki commented Nov 10, 2014

Will do ! Just have to extract the test case from my app.

chrisirhc added a commit that referenced this pull request Nov 11, 2014
Thanks to @red-0ne for the test case.

This commit reverts a1b1ec4
(#2825) .

Fixes #2935
@chrisirhc
Copy link
Contributor

@Igosuki , does #2951 demonstrate the issue you were having?

OronNadiv pushed a commit to lanetix/bootstrap that referenced this pull request Nov 18, 2014
OronNadiv pushed a commit to lanetix/bootstrap that referenced this pull request Nov 18, 2014
Thanks to @red-0ne for the test case.

This commit reverts a1b1ec4
(angular-ui#2825) .

Fixes angular-ui#2935
@Igosuki
Copy link
Contributor Author

Igosuki commented Jan 5, 2015

Hey @chrisirhc sorry I didn't get back to you on this. #2951 does demonstrate the exact problem.

And I don't see any issues upgrading to 0.12 with your merges.

@wesleycho
Copy link
Contributor

Thanks for the find, I am closing this PR as it appears this is no longer an issue with the changes that have been made to the tooltip.

@wesleycho wesleycho closed this Mar 16, 2015
@karianna karianna added this to the 0.13.0 milestone Mar 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants