From 7474c47b1ac43d70178d8ae128f15cbcbb91251f Mon Sep 17 00:00:00 2001
From: Johan Roxendal <johan@roxendal.com>
Date: Sun, 19 Jan 2014 13:12:04 +0100
Subject: [PATCH] fix(tabs): fire deselect before select callback

Closes #1557
Closes #1566
---
 src/tabs/tabs.js           | 19 ++++++++------
 src/tabs/test/tabs.spec.js | 51 +++++++++++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/src/tabs/tabs.js b/src/tabs/tabs.js
index 0a368a2fb6..82994682ea 100644
--- a/src/tabs/tabs.js
+++ b/src/tabs/tabs.js
@@ -13,16 +13,24 @@ angular.module('ui.bootstrap.tabs', [])
   var ctrl = this,
       tabs = ctrl.tabs = $scope.tabs = [];
 
-  ctrl.select = function(tab) {
+  ctrl.select = function(selectedTab) {
     angular.forEach(tabs, function(tab) {
-      tab.active = false;
+      if (tab.active && tab !== selectedTab) {
+        tab.active = false;
+        tab.onDeselect();
+      }
     });
-    tab.active = true;
+    selectedTab.active = true;
+    selectedTab.onSelect();
   };
 
   ctrl.addTab = function addTab(tab) {
     tabs.push(tab);
-    if (tabs.length === 1 || tab.active) {
+    // we can't run the select function on the first tab
+    // since that would select it twice
+    if (tabs.length === 1) {
+      tab.active = true;
+    } else if (tab.active) {
       ctrl.select(tab);
     }
   };
@@ -206,9 +214,6 @@ angular.module('ui.bootstrap.tabs', [])
           setActive(scope.$parent, active);
           if (active) {
             tabsetCtrl.select(scope);
-            scope.onSelect();
-          } else {
-            scope.onDeselect();
           }
         });
 
diff --git a/src/tabs/test/tabs.spec.js b/src/tabs/test/tabs.spec.js
index c261805693..1abe867a70 100644
--- a/src/tabs/test/tabs.spec.js
+++ b/src/tabs/test/tabs.spec.js
@@ -73,7 +73,7 @@ describe('tabs', function() {
       expect(titles().eq(0)).toHaveClass('active');
       expect(titles().eq(1)).not.toHaveClass('active');
       expect(scope.actives.one).toBe(true);
-      expect(scope.actives.two).toBe(false);
+      expect(scope.actives.two).toBeFalsy();
     });
 
     it('should change active on click', function() {
@@ -99,7 +99,6 @@ describe('tabs', function() {
       titles().eq(1).find('a').click();
       expect(scope.deselectFirst).toHaveBeenCalled();
     });
-
   });
 
   describe('basics with initial active tab', function() {
@@ -153,6 +152,48 @@ describe('tabs', function() {
     });
   });
 
+  describe('tab callback order', function() {
+    var execOrder;
+    beforeEach(inject(function($compile, $rootScope) {
+      scope = $rootScope.$new();
+      execOrder = [];
+      scope.actives = {};
+
+      scope.execute = function(id) {
+        execOrder.push(id);
+      };
+
+      elm = $compile([
+        '<div>',
+        '  <tabset class="hello" data-pizza="pepperoni">',
+        '    <tab heading="First Tab" active="actives.one" select="execute(\'select1\')" deselect="execute(\'deselect1\')"></tab>',
+        '    <tab select="execute(\'select2\')" deselect="execute(\'deselect2\')"></tab>',
+        '  </tabset>',
+        '</div>'
+      ].join('\n'))(scope);
+      scope.$apply();
+      return elm;
+    }));
+
+    it('should call select  for the first tab', function() {
+        expect(execOrder).toEqual([ 'select1' ]);
+    });
+
+    it('should call deselect, then select', function() {
+          execOrder = [];
+
+          // Select second tab
+          titles().eq(1).find('a').click();
+          expect(execOrder).toEqual([ 'deselect1', 'select2' ]);
+
+          execOrder = [];
+
+          // Select again first tab
+          titles().eq(0).find('a').click();
+          expect(execOrder).toEqual([ 'deselect2', 'select1' ]);
+    });
+  });
+
   describe('ng-repeat', function() {
 
     beforeEach(inject(function($compile, $rootScope) {
@@ -346,7 +387,11 @@ describe('tabs', function() {
 
   describe('tabset controller', function() {
     function mockTab(isActive) {
-      return { active: !!isActive };
+      return { 
+        active: !!isActive, 
+        onSelect : angular.noop,
+        onDeselect : angular.noop
+      };
     }
 
     var ctrl;