Remove the ng-binding class from text before lookup#94
Remove the ng-binding class from text before lookup#94rubenv merged 8 commits intorubenv:masterfrom circlingthesun:ng-binding-fix
Conversation
|
That sounds like a bug. Have a look at how the directive is written. It's marked as terminal so it shouldn't process the contents until we inject it again. Which version are you testing this with?
|
|
Version of Angular? Same one as in the unit test (~1.2.0), the same also happens with 1.3 beta. ng(Ref/Src/Srcset/...) has a priority of 100 which is higher than translate's 350 (see https://github.com/angular/angular.js/blob/833e60a203263a81a24d773c82c3563c3fb2c97c/src/ng/directive/attrs.js#L355). |
|
It's lower :-) So from my understanding, ng-href shouldn't even run. Need to investigate. Oh, also, I haven't gotten round to reviewing the other patches yet, sorry for that. |
|
Post link functions are run in reverse priority order so functions with lower numbers are executed first. Don't stress about the patches, I have my local copies :) |
|
+1 |
|
Surely this must be fixable by tuning the priority / doing things in compile vs link? I mean, it feels very wrong to be messing around with the HTML that comes out, that's just a recipe for future breakage. I'd love to see this fixed, but I have a feeling that there's a better way to do this. |
|
Well, to avoid the ng-binding class one would have to set the priority to less that 100 which seems to break other tests. I've not had a hard look at why other tests break though. There may be a better solution. That would be nice. |
|
So the problem is that the ng-binding attribute gets added during the compile phase of the ngBind directive. The translate directive first runs at link time at when it is too late. The element content cannot be accessed at compile time when using a transclusion. So you either need to retrieve the element html a 3rd translate directive, or get rid of the transclusion. I did the later as well as getting rid of the 2nd directive. Mmm... it seems I borked things. |
|
...and its fixed. Thats a bit more elegant right? |
There was a problem hiding this comment.
Yeah, also known as kitchen dutch ;p
There was a problem hiding this comment.
Corrected this in 8fa33ba. I'm cool with having Afrikaans in the test suite [1], but it needs to be in the right language code.
[1] I have very fond memories of South Africa.
|
Looking very good, I'll need to do some thorough testing to make sure we don't hit any corner-cases. |
|
Haven't found any issues with this patch. Let's release it. I'll bump the minor version to avoid any breakage, but I have yet to encounter some. |
|
Released to NPM as |
3 participants
When using ng-href and related directives, the 'ng-binding' class gets added to elements before the translate directive can get to it. This patch strips the ng-binding class from elements inside the translate directive to that the msgid lookup succeeds.