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

directive behavior differs from 1.2.26 to 1.3.4 #10236

Closed
frfancha opened this issue Nov 26, 2014 · 8 comments
Closed

directive behavior differs from 1.2.26 to 1.3.4 #10236

frfancha opened this issue Nov 26, 2014 · 8 comments

Comments

@frfancha
Copy link

http://plnkr.co/edit/giVsWj1HDjlVN8lEJaHR?p=preview
In 1.2.26
test-directive ng-hide="directiveScope" does the job to hide
in 1.3.4 must be
test-directive ng-hide="controllerScope"
I think 1.3.4 behavior is the correct one, but is it really so?
Is this a wanted breaking change?
Fred

@pkozlowski-opensource
Copy link
Member

Yes, I believe that this one of a breaking changes in 1.3.x, trying to find the relevant commit

@frfancha
Copy link
Author

It seems a big change for me but I don't find any mention of it in https://docs.angularjs.org/guide/migration
Perhaps I don't look correctly.

@thorn0
Copy link
Contributor

thorn0 commented Dec 1, 2014

An isolated scope mustn't be accessible from outside, so I think it's a bug fix.

@caitp
Copy link
Contributor

caitp commented Dec 4, 2014

I don't think this is going to be rolled back, I'm in agreement with @thorn0 --- but lets make sure that the change is documented

@caitp
Copy link
Contributor

caitp commented Dec 4, 2014

From a bisect, looks like 2ee29c5 is responsible.

@petebacondarwin could you document this in the changelog?

@petebacondarwin
Copy link
Contributor

@thorn0 is correct here. In 1.2.x this was a bug that the isolated scope was leaking into the attributes of the template that contained the isolated scope directive.

It was never supposed to leak - it was not deemed a breaking change but a fix.

But since some people may have been erroneously relying on this bug then we probably should add a note in the CHANGELOG and migration docs.

@matias-sandell
Copy link

I'm trying to understand if this is what forces me to wrap the directive template with an extra tag if I want to use ng-if? ng-show seems to work though. http://plnkr.co/edit/nVsNbyzE8ZigiMxyjzZG?p=preview

@petebacondarwin
Copy link
Contributor

@matiaslarsson - Possibly it is due to this bug: #9837

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

No branches or pull requests

6 participants