refactor interimElement promise logic

attempt to fix issue with select.spec.js#L58-72
angular · Sep 16, 2015 · 5dc7f25 · 5dc7f25
1 parent 6f48656
commit 5dc7f25
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 19 deletions.
diff --git a/src/components/menu/js/menuServiceProvider.js b/src/components/menu/js/menuServiceProvider.js
@@ -80,7 +80,7 @@ function MenuProvider($$interimElementProvider) {
       // For navigation $destroy events, do a quick, non-animated removal,
       // but for normal closes (from clicks, etc) animate the removal
 
-      return !!opts.$destroy ? detachAndClean() : animateRemoval().then( detachAndClean );
+      return (opts.$destroy === true) ? detachAndClean() : animateRemoval().then( detachAndClean );
 
       /**
        * For normal closes, animate the removal.

diff --git a/src/components/select/select.js b/src/components/select/select.js
@@ -317,8 +317,10 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
       scope.$on('$destroy', function() {
         $mdSelect
           .$destroy()
-          .finally( function removeAndCleanup() {
-            selectContainer.remove();
+          .finally(function() {
+            if ( selectContainer ) {
+              selectContainer.remove();
+            }
 
             if (containerCtrl) {
               containerCtrl.setFocused(false);
@@ -783,14 +785,15 @@ function SelectProvider($$interimElementProvider) {
      * Interim-element onRemove logic....
      */
     function onRemove(scope, element, opts) {
+      opts = opts || { };
       opts.cleanupInteraction();
       opts.cleanupResizing();
       opts.hideBackdrop();
 
       // For navigation $destroy events, do a quick, non-animated removal,
       // but for normal closes (from clicks, etc) animate the removal
 
-      return !!opts.$destroy ? detachAndClean() : animateRemoval().then( detachAndClean );
+      return  (opts.$destroy === true) ? detachAndClean() : animateRemoval().then( detachAndClean );
 
       /**
        * For normal closes (eg clicks), animate the removal.

diff --git a/src/components/select/select.spec.js b/src/components/select/select.spec.js
@@ -55,12 +55,14 @@ describe('<md-select>', function() {
     expect(called).toBe(true);
   }));
 
-  it('closes the menu if the element on scope.$destroy()', inject(function($document, $rootScope) {
-    var body = angular.element($document[0].querySelector("body"));
-    var scope = $rootScope.$new(true);
-    var select = setupSelect('ng-model="val", md-on-close="onClose()"', [1, 2, 3], false, scope).find('md-select');
+  iit('closes the menu during scope.$destroy()', inject(function($document, $rootScope, $timeout) {
+    var container = angular.element("<div></div>");
+    var scope = $rootScope.$new();
+    var select = setupSelect(' ng-model="val" ', [1, 2, 3], false, scope).find('md-select');
+
+      $document[0].body.appendChild(container[0]);
+      container.append(select);
 
-      body.append(select);
       openSelect(select);
 
     scope.$destroy();

diff --git a/src/core/services/interimElement/interimElement.js b/src/core/services/interimElement/interimElement.js
@@ -282,7 +282,7 @@ function InterimElementProvider() {
        */
       function show(options) {
         options = options || {};
-        var interimElement = new InterimElement(options);
+        var interimElement = new InterimElement(options || {});
         var hideExisting = !options.skipHide && stack.length ? service.hide() : $q.when(true);
 
         // This hide()s only the current interim element before showing the next, new one
@@ -335,7 +335,7 @@ function InterimElementProvider() {
 
         function closeElement(interim) {
           interim
-            .remove(reason || SHOW_CLOSED, false, options)
+            .remove(reason || SHOW_CLOSED, false, options || { })
             .catch(function( reason ) {
               //$log.error("InterimElement.hide() error: " + reason );
               return reason;
@@ -361,7 +361,7 @@ function InterimElementProvider() {
         if ( !interim ) return $q.when(reason || SHOW_CANCELLED);
 
         interim
-          .remove(reason || SHOW_CANCELLED, true, options)
+          .remove(reason || SHOW_CANCELLED, true, options || { })
           .catch(function( reason ) {
             //$log.error("InterimElement.cancel() error: " + reason );
             return reason;
@@ -430,16 +430,13 @@ function InterimElementProvider() {
          * - perform optional clean up scope.
          */
         function transitionOutAndRemove(response, isCancelled, opts) {
-
-          options = angular.merge(options, opts || {});
+          options = angular.merge(options || {}, opts || {});
           options.cancelAutoHide && options.cancelAutoHide();
           options.element.triggerHandler('$mdInterimElementRemove');
 
           if ( !!options.$destroy ) {
 
-            return $$q(function(resolve){
-              hideElement(options.element, options).then(resolve);
-            });
+            return hideElement(options.element, options);
 
           } else {
 
@@ -625,21 +622,22 @@ function InterimElementProvider() {
           return $$q(function (resolve, reject) {
             try {
               // Start transitionIn
-              var action = $$q.when(element ? options.onRemove(options.scope, element, options) : true);
+              var action = $$q.when( options.onRemove(options.scope, element, options) || true );
 
               // Trigger callback *before* the remove operation starts
               announceRemoving(element, action);
 
               if ( !!options.$destroy ) {
 
+                // For $destroy, onRemove should be synchronous
                 resolve(element);
 
               } else {
 
                 // Wait until transition-out is done
                 action.then(function () {
 
-                  if (!options.preserveScope ) {
+                  if (!options.preserveScope && options.scope ) {
                     options.scope.$destroy();
                   }