feat(ngTransclude): don't overwrite the contents with an unfilled optional slot

petebacondarwin · petebacondarwin · commit 0812af49bd4f · 2015-12-04T21:42:07.000Z
Previously the contents of the `ngTransclude` element would always be emptied, even if there was no transclusion to replace it. Now, optional slots that have not been filled with content will not cause the `ngTransclude` contents to be emptied. Closes angular#13426
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -2152,7 +2152,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           transcludeFn = controllersBoundTransclude;
           transcludeFn.$$boundTransclude = boundTranscludeFn;
           // expose the slots on the `$transclude` function
-          transcludeFn.$slots = boundTranscludeFn.$$slots;
+          transcludeFn.isSlotFilled = function(slotName) {
+            return !!boundTranscludeFn.$$slots[slotName];
+          };
         }
 
         if (controllerDirectives) {
diff --git a/src/ng/directive/ngTransclude.js b/src/ng/directive/ngTransclude.js
@@ -11,8 +11,9 @@
  * You can specify that you want to insert a named transclusion slot, instead of the default slot, by providing the slot name
  * as the value of the `ng-transclude` or `ng-transclude-slot` attribute.
  *
- * Any existing content of this element will be removed before the transcluded content is inserted,
- * but only if the transcluded content is not empty.
+ * For required slots and the default transclusion, existing content will be removed before the transcluded content is inserted.
+ *
+ * For optional slots, existing content is left in place, if the slot was not filled.
  *
  * @element ANY
  *
@@ -118,11 +119,13 @@
             restrict: 'E',
             transclude: {
               'paneTitle': '?title',
-              'paneBody': 'body'
+              'paneBody': 'body',
+              'paneFooter': '?footer'
             },
             template: '<div style="border: 1px solid black;">' +
                         '<div ng-transclude="title" style="background-color: gray"></div>' +
                         '<div ng-transclude="body"></div>' +
+                        '<div ng-transclude="footer" style="background-color: gray">Default Footer</div>' +
                       '</div>'
           };
       })
@@ -171,7 +174,10 @@ var ngTranscludeDirective = ngDirective({
        startingTag($element));
     }
 
-    $transclude(ngTranscludeCloneAttachFn, null, $attrs.ngTransclude || $attrs.ngTranscludeSlot);
+    // If there is no slot name defined or the slot name is not optional
+    // then transclude the slot
+    var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
+    $transclude(ngTranscludeCloneAttachFn, null, slotName);
   }
 });
 
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -7966,7 +7966,7 @@ describe('$compile', function() {
     });
 
 
-    it('should provide the elements marked with matching transclude elements as additional transclude functions on the $slots property', function() {
+    it('should return true from `isSlotFilled(slotName) for slots that have content in the transclusion', function() {
       var capturedTranscludeFn;
       module(function() {
         directive('minionComponent', function() {
@@ -7975,7 +7975,7 @@ describe('$compile', function() {
             scope: {},
             transclude: {
               minionSlot: 'minion',
-              bossSlot: 'boss'
+              bossSlot: '?boss'
             },
             template:
               '<div class="boss" ng-transclude="bossSlot"></div>' +
@@ -7993,36 +7993,49 @@ describe('$compile', function() {
           '  <minion>stuart</minion>' +
           '  <minion>bob</minion>' +
           '  <span>dorothy</span>' +
-          '  <boss>gru</boss>' +
           '</minion-component>')($rootScope);
         $rootScope.$apply();
 
-        var minionTranscludeFn = capturedTranscludeFn.$slots['minionSlot'];
-        var minions = minionTranscludeFn();
-        expect(minions[0].outerHTML).toEqual('<minion class="ng-scope">stuart</minion>');
-        expect(minions[1].outerHTML).toEqual('<minion class="ng-scope">bob</minion>');
-
-        var scope = element.scope();
-
-        var minionScope = jqLite(minions[0]).scope();
-        expect(minionScope.$parent).toBe(scope);
-
-        var bossTranscludeFn = capturedTranscludeFn.$slots['bossSlot'];
-        var boss = bossTranscludeFn();
-        expect(boss[0].outerHTML).toEqual('<boss class="ng-scope">gru</boss>');
+        var hasMinions = capturedTranscludeFn.isSlotFilled('minionSlot');
+        var hasBosses = capturedTranscludeFn.isSlotFilled('bossSlot');
 
-        var bossScope = jqLite(boss[0]).scope();
-        expect(bossScope.$parent).toBe(scope);
-
-        expect(bossScope).not.toBe(minionScope);
+        expect(hasMinions).toBe(true);
+        expect(hasBosses).toBe(false);
+      });
+    });
 
-        dealoc(boss);
-        dealoc(minions);
+    it('should not overwrite the contents of an `ng-transclude` element, if the matching optional slot is not filled', function() {
+      module(function() {
+        directive('minionComponent', function() {
+          return {
+            restrict: 'E',
+            scope: {},
+            transclude: {
+              minionSlot: 'minion',
+              bossSlot: '?boss'
+            },
+            template:
+              '<div class="boss" ng-transclude="bossSlot">default boss content</div>' +
+              '<div class="minion" ng-transclude="minionSlot">default minion content</div>' +
+              '<div class="other" ng-transclude>default content</div>'
+          };
+        });
+      });
+      inject(function($rootScope, $compile) {
+        element = $compile(
+          '<minion-component>' +
+            '<minion>stuart</minion>' +
+            '<span>dorothy</span>' +
+            '<minion>kevin</minion>' +
+          '</minion-component>')($rootScope);
+        $rootScope.$apply();
+        expect(element.children().eq(0).text()).toEqual('default boss content');
+        expect(element.children().eq(1).text()).toEqual('stuartkevin');
+        expect(element.children().eq(2).text()).toEqual('dorothy');
       });
     });
 
-    it('should set unfilled optional transclude slots to `null` in the $transclude.$slots property', function() {
-      var capturedTranscludeFn;
+    it('should not overwrite the contents of an `ng-transclude` element, if the matching optional slot is not filled', function() {
       module(function() {
         directive('minionComponent', function() {
           return {
@@ -8032,9 +8045,10 @@ describe('$compile', function() {
               minionSlot: 'minion',
               bossSlot: '?boss'
             },
-            link: function(s, e, a, c, transcludeFn) {
-              capturedTranscludeFn = transcludeFn;
-            }
+            template:
+              '<div class="boss" ng-transclude="bossSlot">default boss content</div>' +
+              '<div class="minion" ng-transclude="minionSlot">default minion content</div>' +
+              '<div class="other" ng-transclude>default content</div>'
           };
         });
       });
@@ -8043,9 +8057,12 @@ describe('$compile', function() {
           '<minion-component>' +
             '<minion>stuart</minion>' +
             '<span>dorothy</span>' +
+            '<minion>kevin</minion>' +
           '</minion-component>')($rootScope);
-        expect(capturedTranscludeFn.$slots.minionSlot).toEqual(jasmine.any(Function));
-        expect(capturedTranscludeFn.$slots.bossSlot).toBe(null);
+        $rootScope.$apply();
+        expect(element.children().eq(0).text()).toEqual('default boss content');
+        expect(element.children().eq(1).text()).toEqual('stuartkevin');
+        expect(element.children().eq(2).text()).toEqual('dorothy');
       });
     });
   });
