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

Transclude issue#7530

Closed
petebacondarwin wants to merge 1 commit into
angular:masterfrom
petebacondarwin:transclude-issue
Closed

Transclude issue#7530
petebacondarwin wants to merge 1 commit into
angular:masterfrom
petebacondarwin:transclude-issue

Conversation

@petebacondarwin
Copy link
Copy Markdown
Contributor

No description provided.

@mary-poppins
Copy link
Copy Markdown

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7530)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@petebacondarwin
Copy link
Copy Markdown
Contributor Author

@IgorMinar - seems to solve the problem http://plnkr.co/edit/AqCHhmudH8XxDwP4oGPM?p=preview

@vojtajina
Copy link
Copy Markdown
Contributor

LGTM, I think this is a correct fix.

@vojtajina
Copy link
Copy Markdown
Contributor

These two commits should be squashed I think as it's fixing a bug introduced in the first one (during refactoring/cleaning up from the original e10bd88).

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
Comment thread test/ng/compileSpec.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why do we need $digest twice?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@vojtajina This is not needed now I've rewritten the ngif fix. I'll close this and send in a series of prs for all this today

@vojtajina
Copy link
Copy Markdown
Contributor

@petebacondarwin I'm running google tests to make sure this is fine, will let you know when you can submit this.

@btford btford modified the milestones: 1.3.0-beta.11, 1.3.0-beta.10 May 23, 2014
@vojtajina vojtajina mentioned this pull request May 29, 2014
@vojtajina
Copy link
Copy Markdown
Contributor

Closed by #7610

@vojtajina vojtajina closed this May 30, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants