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

Commit

Permalink
fix(translateDirective): fixes bug that directive writes into scope
Browse files Browse the repository at this point in the history 
Fixes #128
  • Loading branch information
@0x-r4bbit
0x-r4bbit committed Aug 20, 2013
1 parent 8a34dd7 commit 4e06468
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions src/directive/translate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ angular.module('pascalprecht.translate')

return {
restrict: 'AE',
scope: true,
link: function linkFn(scope, element, attr) {

if (attr.translateInterpolation) {
Expand All @@ -92,7 +91,7 @@ angular.module('pascalprecht.translate')
// If the attribute has no content, the element's text value (white spaces trimmed off) will be used.
attr.$observe('translate', function (translationId) {
if (angular.equals(translationId , '') || translationId === undefined) {
scope.translationId = $interpolate(element.text().replace(/^\s+|\s+$/g,''))(scope.$parent);
scope.translationId = $interpolate(element.text().replace(/^\s+|\s+$/g,''))(scope);
} else {
scope.translationId = translationId;
}
Expand Down

6 comments on commit 4e06468

@elemoine
Copy link

Choose a reason for hiding this comment

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

I'm surprised by this fix. Does this mean that the translate directive is going to write values (e.g. translationId) into the user's scope? Don't you expect clashes when multiple translate directive instances share the same scope? Sorry if these comments don't make sense.

@0x-r4bbit
Copy link
Member Author

Choose a reason for hiding this comment

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

@elemoine your comments make totally sense! Sorry man, it's very late here in germany. Shit, that's why I originally created a child scope in translate directive. We need another solution =(

@elemoine
Copy link

Choose a reason for hiding this comment

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

Having a new scope made sense indeed.

@0x-r4bbit
Copy link
Member Author

Choose a reason for hiding this comment

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

@elemoine Yea, will roll back that stuff. Any other idea how to fix the problem?

@elemoine
Copy link

Choose a reason for hiding this comment

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

I think I wouldn't not fix it :) Because there are application-level solutions to "fix" it:

  1. Use a function:

    <div ng-click="click()" translate>to translate</div>

  2. Use the "have a . in your models" technique (mentioned in the official doc):

    <div ng-click="model.isShown = !model.isShown" translate>to translate</div>

@0x-r4bbit
Copy link
Member Author

@0x-r4bbit 0x-r4bbit commented on 4e06468 Aug 21, 2013 via email

Choose a reason for hiding this comment

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

Please sign in to comment.