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
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f32e3e
fix(ngSwitch): use the correct transclusion scope
petebacondarwin Jul 17, 2014
7843b71
fix(ngSwitch): interoperate with multi-element transclude directives
petebacondarwin Jul 17, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/ng/directive/ngSwitch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ var ngSwitchDirective = ['$animate', function($animate) {
previousElements.length = 0;

for (i = 0, ii = selectedScopes.length; i < ii; ++i) {
var selected = selectedElements[i];
var selected = getBlockElements(selectedElements[i].clone);
selectedScopes[i].$destroy();
previousElements[i] = selected;
$animate.leave(selected, function() {
Expand All @@ -166,12 +166,13 @@ var ngSwitchDirective = ['$animate', function($animate) {
if ((selectedTranscludes = ngSwitchController.cases['!' + value] || ngSwitchController.cases['?'])) {
scope.$eval(attr.change);
forEach(selectedTranscludes, function(selectedTransclude) {
var selectedScope = scope.$new();
selectedScopes.push(selectedScope);
selectedTransclude.transclude(selectedScope, function(caseElement) {
selectedTransclude.transclude(function(caseElement, selectedScope) {
selectedScopes.push(selectedScope);
var anchor = selectedTransclude.element;
caseElement[caseElement.length++] = document.createComment(' end ngSwitchWhen: ');
var block = { clone: caseElement };

selectedElements.push(caseElement);
selectedElements.push(block);
$animate.enter(caseElement, anchor.parent(), anchor);
});
});
Expand All @@ -183,7 +184,7 @@ var ngSwitchDirective = ['$animate', function($animate) {

var ngSwitchWhenDirective = ngDirective({
transclude: 'element',
priority: 800,
priority: 1200,
require: '^ngSwitch',
multiElement: true,
link: function(scope, element, attrs, ctrl, $transclude) {
Expand All @@ -194,7 +195,7 @@ var ngSwitchWhenDirective = ngDirective({

var ngSwitchDefaultDirective = ngDirective({
transclude: 'element',
priority: 800,
priority: 1200,
require: '^ngSwitch',
multiElement: true,
link: function(scope, element, attr, ctrl, $transclude) {
Expand Down
27 changes: 26 additions & 1 deletion test/ng/directive/ngSwitchSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe('ngSwitch', function() {
$rootScope.url = 'x';
$rootScope.$apply();
expect(getChildScope()).toBeUndefined();
expect(child1.$destroy).toHaveBeenCalledOnce();
expect(child1.$destroy).toHaveBeenCalled();
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should assert a specific callCount here

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 think putting a specific number makes the test brittle. What we care about is that destroy has been called.

Copy link
Contributor

Choose a reason for hiding this comment

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

you mean because the test will fail if the behaviour changes? But that's good, otherwise we might not have a clear understanding that behaviour has changed, and the behaviour should always be what we expect it to be.


$rootScope.url = 'a';
$rootScope.$apply();
Expand All @@ -251,6 +251,31 @@ describe('ngSwitch', function() {
}));


it("should interoperate with other transclusion directives like ngRepeat", inject(function($rootScope, $compile) {
element = $compile(
'<div ng-switch="value">' +
'<div ng-switch-when="foo" ng-repeat="foo in foos">{{value}}:{{foo}}|</div>' +
'<div ng-switch-default ng-repeat="bar in bars">{{value}}:{{bar}}|</div>' +
'</div>'
)($rootScope);
$rootScope.$apply('value="foo";foos=["one", "two"]');
expect(element.text()).toEqual('foo:one|foo:two|');

$rootScope.$apply('value="foo";foos=["one"]');
expect(element.text()).toEqual('foo:one|');

$rootScope.$apply('value="foo";foos=["one","two","three"]');
expect(element.text()).toEqual('foo:one|foo:two|foo:three|');

$rootScope.$apply('value="bar";bars=["up", "down"]');
expect(element.text()).toEqual('bar:up|bar:down|');

$rootScope.$apply('value="bar";bars=["up", "down", "forwards", "backwards"]');
expect(element.text()).toEqual('bar:up|bar:down|bar:forwards|bar:backwards|');

}));


it('should not leak jq data when compiled but not attached to parent when parent is destroyed',
inject(function($rootScope, $compile) {
element = $compile(
Expand Down