fix(interimElement): support scope.$destroy events

when navigation (eg $location) changes occur, the `scope.$destroy` event is triggered; which should remove/close/cleanup the following components: menu, select, dialog, toast, and bottomSheet. Fixes #3741. Fixes #4405. Fixes #4504. Fixes #4151. Closes #4659.
angular · Sep 16, 2015 · 77a34bd · 77a34bd
1 parent 3d0b418
commit 77a34bd
Show file tree

Hide file tree

Showing 14 changed files with 424 additions and 250 deletions.
diff --git a/src/components/bottomSheet/bottom-sheet.js b/src/components/bottomSheet/bottom-sheet.js
@@ -12,9 +12,17 @@ angular
   .directive('mdBottomSheet', MdBottomSheetDirective)
   .provider('$mdBottomSheet', MdBottomSheetProvider);
 
-function MdBottomSheetDirective() {
+/* @ngInject */
+function MdBottomSheetDirective($mdBottomSheet) {
   return {
-    restrict: 'E'
+    restrict: 'E',
+    link : function postLink(scope, element, attr) {
+      // When navigation force destroys an interimElement, then
+      // listen and $destroy() that interim instance...
+      scope.$on('$destroy', function() {
+        $mdBottomSheet.destroy();
+      });
+    }
   };
 }
 

diff --git a/src/components/bottomSheet/bottom-sheet.spec.js b/src/components/bottomSheet/bottom-sheet.spec.js
@@ -2,7 +2,7 @@ describe('$mdBottomSheet service', function() {
   beforeEach(module('material.components.bottomSheet'));
 
   describe('#build()', function() {
-    it('should escapeToClose == true', inject(function($mdBottomSheet, $rootScope, $rootElement, $timeout, $animate, $mdConstant) {
+    it('should close when `escapeToClose == true`', inject(function($mdBottomSheet, $rootScope, $rootElement, $timeout, $animate, $mdConstant) {
       var parent = angular.element('<div>');
       $mdBottomSheet.show({
         template: '<md-bottom-sheet>',
@@ -24,7 +24,7 @@ describe('$mdBottomSheet service', function() {
       expect(parent.find('md-bottom-sheet').length).toBe(0);
     }));
 
-    it('should escapeToClose == false', inject(function($mdBottomSheet, $rootScope, $rootElement, $timeout, $animate, $mdConstant) {
+    it('should not close when `escapeToClose == false`', inject(function($mdBottomSheet, $rootScope, $rootElement, $timeout, $animate, $mdConstant) {
       var parent = angular.element('<div>');
       $mdBottomSheet.show({
         template: '<md-bottom-sheet>',
@@ -42,6 +42,23 @@ describe('$mdBottomSheet service', function() {
       expect(parent.find('md-bottom-sheet').length).toBe(1);
     }));
 
+    it('should close when navigation fires `scope.$destroy()`', inject(function($mdBottomSheet, $rootScope, $rootElement, $timeout, $animate, $mdConstant) {
+          var parent = angular.element('<div>');
+          $mdBottomSheet.show({
+            template: '<md-bottom-sheet>',
+            parent: parent,
+            escapeToClose: false
+          });
+
+          $rootScope.$apply();
+          $animate.triggerCallbacks();
+
+          expect(parent.find('md-bottom-sheet').length).toBe(1);
+
+          $rootScope.$destroy();
+          expect(parent.find('md-bottom-sheet').length).toBe(0);
+        }));
+
     it('should focus child with md-autofocus', inject(function($rootScope, $animate, $document, $mdBottomSheet) {
       jasmine.mockElementFocus(this);
       var parent = angular.element('<div>');

diff --git a/src/components/bottomSheet/demoBasicUsage/script.js b/src/components/bottomSheet/demoBasicUsage/script.js
@@ -22,7 +22,7 @@ angular.module('bottomSheetDemo1', ['ngMaterial'])
       controller: 'ListBottomSheetCtrl',
       targetEvent: $event
     }).then(function(clickedItem) {
-      $scope.alert = clickedItem.name + ' clicked!';
+      $scope.alert = clickedItem['name'] + ' clicked!';
     });
   };
 
@@ -35,7 +35,7 @@ angular.module('bottomSheetDemo1', ['ngMaterial'])
     }).then(function(clickedItem) {
       $mdToast.show(
             $mdToast.simple()
-              .content(clickedItem.name + ' clicked!')
+              .content(clickedItem['name'] + ' clicked!')
               .position('top right')
               .hideDelay(1500)
           );

diff --git a/src/components/dialog/dialog.js b/src/components/dialog/dialog.js
@@ -10,7 +10,7 @@ angular
   .directive('mdDialog', MdDialogDirective)
   .provider('$mdDialog', MdDialogProvider);
 
-function MdDialogDirective($$rAF, $mdTheming) {
+function MdDialogDirective($$rAF, $mdTheming, $mdDialog) {
   return {
     restrict: 'E',
     link: function(scope, element, attr) {
@@ -25,9 +25,19 @@ function MdDialogDirective($$rAF, $mdTheming) {
           //-- delayed image loading may impact scroll height, check after images are loaded
           angular.element(images).on('load', addOverflowClass);
         }
+
+        scope.$on('$destroy', function() {
+          $mdDialog.destroy();
+        });
+
+        /**
+         *
+         */
         function addOverflowClass() {
           element.toggleClass('md-content-overflow', content.scrollHeight > content.clientHeight);
         }
+
+
       });
     }
   };
@@ -530,16 +540,30 @@ function MdDialogProvider($$interimElementProvider) {
     function onRemove(scope, element, options) {
       options.deactivateListeners();
       options.unlockScreenReader();
+      options.hideBackdrop(options.$destroy);
 
-      options.hideBackdrop();
+      // For navigation $destroy events, do a quick, non-animated removal,
+      // but for normal closes (from clicks, etc) animate the removal
 
-      return dialogPopOut(element, options)
-        .finally(function() {
-          angular.element($document[0].body).removeClass('md-dialog-is-showing');
-          element.remove();
+      return !!options.$destroy ? detachAndClean() : animateRemoval().then( detachAndClean );
 
-          options.origin.focus();
-        });
+      /**
+       * For normal closes, animate the removal.
+       * For forced closes (like $destroy events), skip the animations
+       */
+      function animateRemoval() {
+        return dialogPopOut(element, options);
+      }
+
+      /**
+       * Detach the element
+       */
+      function detachAndClean() {
+        angular.element($document[0].body).removeClass('md-dialog-is-showing');
+        element.remove();
+
+        if (!options.$destroy) options.origin.focus();
+      }
     }
 
     /**
@@ -661,10 +685,12 @@ function MdDialogProvider($$interimElementProvider) {
       /**
        * Hide modal backdrop element...
        */
-      options.hideBackdrop = function hideBackdrop() {
+      options.hideBackdrop = function hideBackdrop($destroy) {
         if (options.backdrop) {
-          $animate.leave(options.backdrop);
+          if ( !!$destroy ) options.backdrop.remove();
+          else              $animate.leave(options.backdrop);
         }
+
         if (options.disableParentScroll) {
           options.restoreScroll();
           delete options.restoreScroll;
@@ -755,7 +781,6 @@ function MdDialogProvider($$interimElementProvider) {
 
       var isFixed = $window.getComputedStyle($document[0].body).position == 'fixed';
       var backdrop = options.backdrop ? $window.getComputedStyle(options.backdrop[0]) : null;
-
       var height = backdrop ? Math.min($document[0].body.clientHeight, Math.ceil(Math.abs(parseInt(backdrop.height, 10)))) : 0;
 
       container.css({

diff --git a/src/components/dialog/dialog.spec.js b/src/components/dialog/dialog.spec.js
@@ -158,6 +158,28 @@ describe('$mdDialog', function() {
       expect(container.length).toBe(0);
     }));
 
+    it('should remove `md-dialog-container` on scope.$destroy()', inject(function($mdDialog, $rootScope, $timeout) {
+      var container, parent = angular.element('<div>');
+
+      $mdDialog.show(
+        $mdDialog.alert({
+          template: '' +
+            '<md-dialog>' +
+            '  <md-dialog-content tabIndex="0">' +
+            '    <p>Muppets are the best</p>' +
+            '  </md-dialog-content>' +
+            '</md-dialog>',
+          parent: parent
+        })
+      );
+
+      runAnimation(parent.find('md-dialog'));
+        $rootScope.$destroy();
+      container = angular.element(parent[0].querySelector('.md-dialog-container'));
+
+      expect(container.length).toBe(0);
+    }));
+
   });
 
   describe('#confirm()', function() {

diff --git a/src/components/menu/js/menuController.js b/src/components/menu/js/menuController.js
@@ -100,7 +100,7 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout) {
       preserveElement: self.isInMenuBar || self.nestedMenus.length > 0,
       parent: self.isInMenuBar ? $element : 'body'
     });
-  }
+  };
 
   // Expose a open function to the child scope for html to use
   $scope.$mdOpenMenu = this.open;
@@ -133,19 +133,24 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout) {
     this.containerProxy && this.containerProxy(ev);
   };
 
+  this.destroy = function() {
+    return $mdMenu.destroy();
+  };
+
   // Use the $mdMenu interim element service to close the menu contents
   this.close = function closeMenu(skipFocus, closeOpts) {
     if ( !self.isOpen ) return;
     self.isOpen = false;
 
     $scope.$emit('$mdMenuClose', $element);
     $mdMenu.hide(null, closeOpts);
+
     if (!skipFocus) {
       var el = self.restoreFocusTo || $element.find('button')[0];
       if (el instanceof angular.element) el = el[0];
       el.focus();
     }
-  }
+  };
 
   /**
    * Build a nice object out of our string attribute which specifies the

diff --git a/src/components/menu/js/menuDirective.js b/src/components/menu/js/menuDirective.js
@@ -191,8 +191,11 @@ function MenuDirective($mdUtil) {
     mdMenuCtrl.init(menuContainer, { isInMenuBar: isInMenuBar });
 
     scope.$on('$destroy', function() {
-      menuContainer.remove();
-      mdMenuCtrl.close();
+      mdMenuCtrl
+        .destroy()
+        .finally(function(){
+          menuContainer.remove();
+        });
     });
 
   }

diff --git a/src/components/menu/js/menuServiceProvider.js b/src/components/menu/js/menuServiceProvider.js
@@ -22,7 +22,7 @@ function MenuProvider($$interimElementProvider) {
     });
 
   /* @ngInject */
-  function menuDefaultOptions($mdUtil, $mdTheming, $mdConstant, $document, $window, $q, $$rAF, $animateCss, $animate, $timeout) {
+  function menuDefaultOptions($mdUtil, $mdTheming, $mdConstant, $document, $window, $q, $$rAF, $animateCss, $animate) {
     var animator = $mdUtil.dom.animator;
 
     return {
@@ -63,14 +63,8 @@ function MenuProvider($$interimElementProvider) {
        * Hide and destroys the backdrop created by showBackdrop()
        */
       return function hideBackdrop() {
-        if (options.backdrop) {
-          // Override duration to immediately remove invisible backdrop
-          options.backdrop.off('click');
-          $animate.leave(options.backdrop, {duration:0});
-        }
-        if (options.disableParentScroll) {
-          options.restoreScroll();
-        }
+        if (options.backdrop) options.backdrop.remove();
+        if (options.disableParentScroll) options.restoreScroll();
       };
     }
 
@@ -83,14 +77,28 @@ function MenuProvider($$interimElementProvider) {
       opts.cleanupResizing();
       opts.hideBackdrop();
 
-      return $animateCss(element, {addClass: 'md-leave'})
-        .start()
-        .then(function() {
-          element.removeClass('md-active');
+      // For navigation $destroy events, do a quick, non-animated removal,
+      // but for normal closes (from clicks, etc) animate the removal
+
+      return (opts.$destroy === true) ? detachAndClean() : animateRemoval().then( detachAndClean );
+
+      /**
+       * For normal closes, animate the removal.
+       * For forced closes (like $destroy events), skip the animations
+       */
+      function animateRemoval() {
+        return $animateCss(element, {addClass: 'md-leave'}).start();
+      }
+
+      /**
+       * Detach the element
+       */
+      function detachAndClean() {
+        element.removeClass('md-active');
+        detachElement(element, opts);
+        opts.alreadyOpen = false;
+      }
 
-          detachElement(element, opts);
-          opts.alreadyOpen = false;
-        });
     }
 
     /**
@@ -361,7 +369,7 @@ function MenuProvider($$interimElementProvider) {
     }
 
     /**
-     * Use browser to remove this element without triggering a $destory event
+     * Use browser to remove this element without triggering a $destroy event
      */
     function detachElement(element, opts) {
       if (!opts.preserveElement) {

diff --git a/src/components/menu/menu.scss b/src/components/menu/menu.scss
@@ -17,7 +17,6 @@ $max-dense-menu-height: 2 * $baseline-grid + $max-visible-items * $dense-menu-it
     margin-top: $baseline-grid / 2;
     margin-bottom: $baseline-grid / 2;
     height: 1px;
-    min-height: 1px;
     width: 100%;
   }
 

diff --git a/src/components/menu/menu.spec.js b/src/components/menu/menu.spec.js
@@ -86,6 +86,7 @@ describe('material.components.menu', function() {
     });
 
     it('closes on backdrop click', inject(function($document) {
+
       openMenu(setup());
 
       expect(getOpenMenuContainer().length).toBe(1);
@@ -96,6 +97,7 @@ describe('material.components.menu', function() {
       expect(getOpenMenuContainer().length).toBe(0);
     }));
 
+
     it('closes on escape', inject(function($document, $mdConstant) {
       openMenu(setup());
       expect(getOpenMenuContainer().length).toBe(1);
@@ -108,6 +110,16 @@ describe('material.components.menu', function() {
       expect(getOpenMenuContainer().length).toBe(0);
     }));
 
+    it('closes on $destroy', inject(function($document, $rootScope) {
+      var scope = $rootScope.$new();
+      openMenu( setup(null,false,scope) );
+
+      expect(getOpenMenuContainer().length).toBe(1);
+      scope.$destroy();
+
+      expect(getOpenMenuContainer().length).toBe(0);
+    }));
+
     describe('closes with -', function() {
       it('closes on normal option click', function() {
         expect(getOpenMenuContainer().length).toBe(0);
@@ -164,7 +176,7 @@ describe('material.components.menu', function() {
       }
     });
 
-    function setup(triggerType, noEvent) {
+    function setup(triggerType, noEvent, scope) {
       var menu,
         template = $mdUtil.supplant('' +
           '<md-menu>' +
@@ -180,7 +192,7 @@ describe('material.components.menu', function() {
         $rootScope.doSomething = function($event) {
           menuActionPerformed = true;
         };
-        menu = $compile(template)($rootScope);
+        menu = $compile(template)(scope || $rootScope);
       });
 
       attachedElements.push(menu);
@@ -193,7 +205,6 @@ describe('material.components.menu', function() {
   // Internal methods
   // ********************************************
 
-
   function getOpenMenuContainer() {
     var res;
     inject(function($document) {