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

Allow interpolation inside ng-message (fix for #11616) #12001

Closed
wants to merge 1 commit into from
Closed

Allow interpolation inside ng-message (fix for #11616) #12001

wants to merge 1 commit into from

Conversation

sravenhorst
Copy link

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@sravenhorst
Copy link
Author

I have just signed the "first contribution" report.

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Jun 1, 2015
@Narretz
Copy link
Contributor

Narretz commented Jun 2, 2015

Thanks for the PR. As you wrote in the issue, couldn't you also simply move the watch logic back to the link function?

@Narretz
Copy link
Contributor

Narretz commented Jun 2, 2015

Also, we need a test for this.

@Narretz Narretz added this to the 1.4.x milestone Jun 2, 2015
Narretz
Narretz reviewed Jun 3, 2015
View reviewed changes
src/ngMessages/messages.js
forAttr: '@for'
},
scope: true,
controllerAs: 'ngMessagesController',
Copy link
Contributor

Choose a reason for hiding this comment

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

This and scope: true are changing the semantics of the directive. Not sure if we want this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with @Narretz we do not want to add this new scope

@Narretz Narretz changed the title This would fix #11616 Allow interpolation inside ng-message (fix for #11616) Jun 3, 2015
@tkrotoff tkrotoff mentioned this pull request Oct 7, 2015
Closed
Narretz added a commit to Narretz/angular.js that referenced this pull request Nov 6, 2015
@Narretz
docs(changelog, migration): add BC notice for ngMessages evaluation
d4e3ac0 
Introduced by

Closes angular#11616
Closes angular#12001
@Narretz Narretz closed this in aff74ec Nov 6, 2015
Narretz added a commit that referenced this pull request Nov 6, 2015
@Narretz
docs(changelog, migration): add BC notice for ngMessages evaluation
ab9b021 
Introduced by

Closes #11616
Closes #12001
gkalpak pushed a commit to gkalpak/angular.js that referenced this pull request Nov 23, 2015
@Narretz @gkalpak
docs(changelog, migration): add BC notice for ngMessages evaluation
9d1b201 
Introduced by

Closes angular#11616
Closes angular#12001
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes component: ngMessages needs: test severity: regression
Projects
None yet
Milestone
1.4.x
Development

Successfully merging this pull request may close these issues.
4 participants
@sravenhorst @googlebot @Narretz @lgalfaso