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

Backport $compiler fixes from 1.3 to v1.2.x#7828

Closed
caitp wants to merge 9 commits into
angular:v1.2.xfrom
caitp:COMPILATE
Closed

Backport $compiler fixes from 1.3 to v1.2.x#7828
caitp wants to merge 9 commits into
angular:v1.2.xfrom
caitp:COMPILATE

Conversation

@caitp
Copy link
Copy Markdown
Contributor

@caitp caitp commented Jun 13, 2014

This doesn't include 2cde927 which contained breaking changes,
but includes all of the transclusion fixes.

petebacondarwin and others added 8 commits June 13, 2014 09:55
If you have two directives that both expect to receive transcluded content
the outer directive works but the inner directive never receives a
transclusion function. This only failed if the first transclude directive
was not the first directive found in compilation.

Handles the regression identified in e994259

Fixes angular#7240
Closes angular#7387
The boundTransclusionFn that is passed in is really the one from the
parent node.  The change to parentBoundTranscludeFn clarifies this compared
to the childBoundTranscludeFn.
…rective

If a directive provides a template but is not explicitly requesting transclusion
then the compiler should not pass a transclusion function to the directives
within the template.
Nested isolated transclude directives.

This improves/fixes the fix in d414b78.

See the changed ng-ifunit test: The template inside ng-if should be bound to the
isolate scope of `iso` directive (resp. its child scope). Not to a child of
the root scope. This shows the issue with ng-if. It’s however problem with
other directives too.

Instead of remembering the scope, we pass around the bound parent transclusion.

Conflicts:
	test/ng/directive/ngIfSpec.js
If a "replace" directive has an async template, which contains a transclusion
directive at its root node, then outer transclusions were failing to be
passed to this directive.  An example would be uses of `ngIf` inside and
outside the template.

Collaborated with @caitp

Closes angular#7183
Closes angular#7772
@caitp caitp added cla: yes and removed cla: no labels Jun 13, 2014
@caitp
Copy link
Copy Markdown
Contributor Author

caitp commented Jun 13, 2014

@petebacondarwin so yeah, IE8 has issues with element directives, that's the main reason why tests were failing.

It might be worth changing the 1.3 tests not to use element directives, just to keep things in sync. I know that supposedly document.createElement(elementName) should work, but it didn't seem to have any effect when I tried it, so I dunno.

At any rate, this stuff is passing, last failure is just a flake on safari which passes locally.

@petebacondarwin
Copy link
Copy Markdown
Contributor

LGTM! Thanks @caitp
On 13 Jun 2014 16:06, "Caitlin Potter" notifications@github.com wrote:

@petebacondarwin https://github.com/petebacondarwin so yeah, IE8 has
issues with element directives, that's the main reason why tests were
failing.

It might be worth changing the 1.3 tests not to use element directives,
just to keep things in sync. I know that supposedly
document.createElement(elementName) should work, but it didn't seem to
have any effect when I tried it, so I dunno.

At any rate, this stuff is passing, last failure is just a flake on safari
which passes locally.


Reply to this email directly or view it on GitHub
#7828 (comment).

caitp added a commit that referenced this pull request Jun 13, 2014
@rodyhaddad
Copy link
Copy Markdown
Contributor

I believe this can be closed?

@caitp
Copy link
Copy Markdown
Contributor Author

caitp commented Jun 13, 2014

Didn't realize github was going to be an idiot =(

@caitp caitp closed this Jun 13, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants