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

fix(ngSwitch): use the correct transclusion scope #8244

Closed
wants to merge 2 commits into from

Conversation

petebacondarwin
Copy link
Contributor

If an ngSwitchWhen or ngSwitchDefault directive is on an element that also
contains a transclusion directive (such as ngRepeat) the new scope should
be the one provided by the bound transclusion function.

Previously we were incorrectly creating a simple child of the main ngSwitch
scope.

Closes #8235

@mary-poppins
Copy link

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 (#8244)

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!

'<pre ng-switch-when="item1" ng-repeat="bar in bars">' +
'foo = {{foo}}' +
'bar = {{bar}}' +
'$index = {{$index}}' +
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if we really need to reproduce their entire reproduction here --- it's probably safer to just use <span ng-switch-when="item1" ng-repeat="bar in bars">{{$index}}</span> instead --- smaller, and possibly fewer text nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree but I think it is useful to have the {{foo}} bit in there too.

Copy link
Contributor

Choose a reason for hiding this comment

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

keep {{foo}} - simplify the rest.

@IgorMinar
Copy link
Contributor

otherwise lgtm

@petebacondarwin
Copy link
Contributor Author

Actually ngSwitch is totally fubarred if we have multiple cases that each contain multi-element transclusions. See #8235 (comment)
We need to do more work here

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jul 17, 2014
@petebacondarwin
Copy link
Contributor Author

@lgalfaso - what do you think of these fixes?

@lgalfaso
Copy link
Contributor

LGTM

@petebacondarwin
Copy link
Contributor Author

Thanks to @lgalfaso for help with this!

If an ngSwitchWhen or ngSwitchDefault directive is on an element that also
contains a transclusion directive (such as ngRepeat) the new scope should
be the one provided by the bound transclusion function.

Previously we were incorrectly creating a simple child of the main ngSwitch
scope.

BREAKING CHANGE:
** Directive Priority Changed ** - this commit changes the priority
of `ngSwitchWhen` and `ngSwitchDefault` from 800 to 1200.  This makes their
priority higher than `ngRepeat`, which allows items to be repeated on
the switch case element reliably.

In general your directives should have a lower priority than these directives
if you want them to exist inside the case elements. If you relied on the
priority of these directives then you should check that your code still
operates correctly.

Closes angular#8235
thammin added a commit to thammin/angular.js that referenced this pull request Sep 3, 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.

ngSwitchWhen element eats ngRepeat's child scope
5 participants