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

fix(ngMessage): make ngMessage work with ngBind #13074

Merged
merged 1 commit into from
Nov 4, 2015
Merged

fix(ngMessage): make ngMessage work with ngBind #13074

merged 1 commit into from
Nov 4, 2015
+42 −0

Conversation

@Narretz
Copy link
Contributor

@Narretz Narretz commented Oct 12, 2015

Fixes #8089

(I'm not actually sure why this works tbh)

@Narretz Narretz added this to the 1.5.x - migration-facilitation milestone Oct 12, 2015
@Narretz Narretz force-pushed the fix-ngmessages-8089 branch from 54d6039 to dd308f5 Compare October 12, 2015 15:22
@gkalpak
Copy link
Member

gkalpak commented Oct 13, 2015

LGTM

(I'm not actually sure why this works tbh)

LOL ! It works because ngMessage has transclude: 'element', which means that the comment which replaces it will be (sort of) compiled with maxPriority: directive.priority.
Previously, both ngMessage and ngBind had the default priority (0), so ngBind's compile function was applied to the comment node.

With priority: 1 on ngMessage, ngBind won't be applied to the comment node (as it has a lower priority), but to the transcluded element (when/if it gets inserted into the DOM).

I'm not sure if this is a little BC, since all directives with priority 0 (or undefined => default), will not be applied during the initial compilation phase.
Directives that should be applied, usually have (or should have) a higher priority anyway, but you never know.

@Narretz Narretz force-pushed the fix-ngmessages-8089 branch from dd308f5 to ffff155 Compare November 4, 2015 16:10
@Narretz
fix(ngMessage): make ngMessage compatible with ngBind
4971ef1 
Fixes angular#8089
Closes angular#13074

BREAKING CHANGE:

ngMessage is now compiled with a priority of 1, which means directives
on the same element as ngMessage with a priority lower than 1 will
be applied when ngMessage calls the $transclude function.
Previously, they were applied during the initial compile phase and were
passed the comment element created by the transclusion of ngMessage.
To restore this behavior, custom directives need to have
their priority increased to at least "1".
@Narretz Narretz force-pushed the fix-ngmessages-8089 branch from ffff155 to 4971ef1 Compare November 4, 2015 16:14
@Narretz Narretz merged commit 4971ef1 into angular:master Nov 4, 2015
@Narretz
Copy link
Contributor Author

Narretz commented Nov 4, 2015

Thanks @gkalpak ! I've merged it only into 1.5 and added a BC notice.

@gkalpak
Copy link
Member

gkalpak commented Nov 4, 2015

👍

gkalpak pushed a commit to gkalpak/angular.js that referenced this pull request Nov 23, 2015
@Narretz @gkalpak
fix(ngMessage): make ngMessage compatible with ngBind
cb32fce 
Fixes angular#8089
Closes angular#13074

BREAKING CHANGE:

ngMessage is now compiled with a priority of 1, which means directives
on the same element as ngMessage with a priority lower than 1 will
be applied when ngMessage calls the $transclude function.
Previously, they were applied during the initial compile phase and were
passed the comment element created by the transclusion of ngMessage.
To restore this behavior, custom directives need to have
their priority increased to at least "1".
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 frequency: low severity: inconvenient type: bug

Projects

None yet

Milestone

1.5.x

Development

Successfully merging this pull request may close these issues.

3 participants

@Narretz @gkalpak @googlebot