From 4f32e3eef152bcaab7f7ab151fc824e71a591473 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Thu, 17 Jul 2014 20:07:46 +0100 Subject: [PATCH 1/2] fix(ngSwitch): use the correct transclusion scope 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 #8235 --- src/ng/directive/ngSwitch.js | 9 ++++----- test/ng/directive/ngSwitchSpec.js | 24 +++++++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js index 9abe61ca860f..05bfdd7a34da 100644 --- a/src/ng/directive/ngSwitch.js +++ b/src/ng/directive/ngSwitch.js @@ -166,9 +166,8 @@ 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; selectedElements.push(caseElement); @@ -183,7 +182,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) { @@ -194,7 +193,7 @@ var ngSwitchWhenDirective = ngDirective({ var ngSwitchDefaultDirective = ngDirective({ transclude: 'element', - priority: 800, + priority: 1200, require: '^ngSwitch', multiElement: true, link: function(scope, element, attr, ctrl, $transclude) { diff --git a/test/ng/directive/ngSwitchSpec.js b/test/ng/directive/ngSwitchSpec.js index 48e27ae9f50a..1e7bf8fa4b0d 100644 --- a/test/ng/directive/ngSwitchSpec.js +++ b/test/ng/directive/ngSwitchSpec.js @@ -241,7 +241,7 @@ describe('ngSwitch', function() { $rootScope.url = 'x'; $rootScope.$apply(); expect(getChildScope()).toBeUndefined(); - expect(child1.$destroy).toHaveBeenCalledOnce(); + expect(child1.$destroy).toHaveBeenCalled(); $rootScope.url = 'a'; $rootScope.$apply(); @@ -251,6 +251,28 @@ describe('ngSwitch', function() { })); + it("should use the correct transclusion scope if there is a transclude directive on the ng-swith-when/ng-switch-default element", inject(function($rootScope, $compile) { + element = $compile( + '
' + + '
' +
+              'foo = {{foo}}' +
+              'bar = {{bar}}' +
+              '$index = {{$index}}' +
+          '
' + + '
' + )($rootScope); + $rootScope.$apply('foo="item1";bars=["one", "two"]'); + expect(element.text()).toEqual( + 'foo = item1' + + 'bar = one' + + '$index = 0' + + 'foo = item1' + + 'bar = two' + + '$index = 1' + ); + })); + + it('should not leak jq data when compiled but not attached to parent when parent is destroyed', inject(function($rootScope, $compile) { element = $compile( From 7843b71877cd841c214544b86d1c55084b3d788d Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Thu, 17 Jul 2014 22:10:50 +0100 Subject: [PATCH 2/2] fix(ngSwitch): interoperate with multi-element transclude directives Closes #8235 Closes #8244 --- src/ng/directive/ngSwitch.js | 6 ++++-- test/ng/directive/ngSwitchSpec.js | 35 +++++++++++++++++-------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js index 05bfdd7a34da..1eb1d32a4e21 100644 --- a/src/ng/directive/ngSwitch.js +++ b/src/ng/directive/ngSwitch.js @@ -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() { @@ -169,8 +169,10 @@ var ngSwitchDirective = ['$animate', function($animate) { 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); }); }); diff --git a/test/ng/directive/ngSwitchSpec.js b/test/ng/directive/ngSwitchSpec.js index 1e7bf8fa4b0d..5e48d68f19f3 100644 --- a/test/ng/directive/ngSwitchSpec.js +++ b/test/ng/directive/ngSwitchSpec.js @@ -251,25 +251,28 @@ describe('ngSwitch', function() { })); - it("should use the correct transclusion scope if there is a transclude directive on the ng-swith-when/ng-switch-default element", inject(function($rootScope, $compile) { + it("should interoperate with other transclusion directives like ngRepeat", inject(function($rootScope, $compile) { element = $compile( - '
' + - '
' +
-              'foo = {{foo}}' +
-              'bar = {{bar}}' +
-              '$index = {{$index}}' +
-          '
' + + '
' + + '
{{value}}:{{foo}}|
' + + '
{{value}}:{{bar}}|
' + '
' )($rootScope); - $rootScope.$apply('foo="item1";bars=["one", "two"]'); - expect(element.text()).toEqual( - 'foo = item1' + - 'bar = one' + - '$index = 0' + - 'foo = item1' + - 'bar = two' + - '$index = 1' - ); + $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|'); + }));