Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Commit a08173e

Browse files
chrisirhcajoslin
authored andcommitted
fix(tabs): initial tab selection
Closes #834, Fixes #747 - Avoid re-initializing `tab.active` - `setActive` only when all `tab.active` are set up (on the next digest cycle) Before I go to explain, there are up to two expressions that indicate whether a tab is active: 1. `active` variable in the isolate scope of the tab directive 2. The expression from the active attribute (`attrs.active`) if it is set, I'll call this `getActive`, as that's the variable that refers to it. During initial linking (adding of tabs), the `active` variable in the tab's isolate scope tracks the active tab. When the first tab is added, it's `active` is set to true since there's no way to know if subsequent tabs are active since they haven't been added yet. As such, at this point, it is not meaningful to set assign the `getActive` with the value of `active`. At least not until all the tabs have been added. Hence, a good time would be to wait until the next $digest cycle. A watcher is called asynchronously after initialization even if there is no change on the expression's value. As such, we can leave that to the watcher for the `active` expression to initialize getActive by calling setActive during its initlization cycle. However, there is a chance (if the $digest cycles and planets align...) that the `active` variable gets initialized a second time using the `getActive` (in the watcher for `getActive`). Since we're already setting `active` to `getActive`, and the `active` variable should now be carrying the *truth*. Avoid this re-initialization.
1 parent 509357e commit a08173e

File tree

2 files changed

+60
-5
lines changed

2 files changed

+60
-5
lines changed

src/tabs/tabs.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -196,15 +196,22 @@ angular.module('ui.bootstrap.tabs', [])
196196
if (attrs.active) {
197197
getActive = $parse(attrs.active);
198198
setActive = getActive.assign;
199-
scope.$parent.$watch(getActive, function updateActive(value) {
200-
scope.active = !!value;
199+
scope.$parent.$watch(getActive, function updateActive(value, oldVal) {
200+
// Avoid re-initializing scope.active as it is already initialized
201+
// below. (watcher is called async during init with value ===
202+
// oldVal)
203+
if (value !== oldVal) {
204+
scope.active = !!value;
205+
}
201206
});
202207
scope.active = getActive(scope.$parent);
203208
} else {
204209
setActive = getActive = angular.noop;
205210
}
206211

207212
scope.$watch('active', function(active) {
213+
// Note this watcher also initializes and assigns scope.active to the
214+
// attrs.active expression.
208215
setActive(scope.$parent, active);
209216
if (active) {
210217
tabsetCtrl.select(scope);
@@ -231,9 +238,6 @@ angular.module('ui.bootstrap.tabs', [])
231238
scope.$on('$destroy', function() {
232239
tabsetCtrl.removeTab(scope);
233240
});
234-
if (scope.active) {
235-
setActive(scope.$parent, true);
236-
}
237241

238242

239243
//We need to transclude later, once the content container is ready.

src/tabs/test/tabsSpec.js

+51
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,57 @@ describe('tabs', function() {
102102

103103
});
104104

105+
describe('basics with initial active tab', function() {
106+
107+
beforeEach(inject(function($compile, $rootScope) {
108+
scope = $rootScope.$new();
109+
110+
function makeTab(active) {
111+
return {
112+
active: !!active,
113+
select: jasmine.createSpy()
114+
};
115+
}
116+
scope.tabs = [
117+
makeTab(), makeTab(), makeTab(true), makeTab()
118+
];
119+
elm = $compile([
120+
'<tabset>',
121+
' <tab active="tabs[0].active" select="tabs[0].select()">',
122+
' </tab>',
123+
' <tab active="tabs[1].active" select="tabs[1].select()">',
124+
' </tab>',
125+
' <tab active="tabs[2].active" select="tabs[2].select()">',
126+
' </tab>',
127+
' <tab active="tabs[3].active" select="tabs[3].select()">',
128+
' </tab>',
129+
'</tabset>'
130+
].join('\n'))(scope);
131+
scope.$apply();
132+
}));
133+
134+
function expectTabActive(activeTab) {
135+
var _titles = titles();
136+
angular.forEach(scope.tabs, function(tab, i) {
137+
if (activeTab === tab) {
138+
expect(tab.active).toBe(true);
139+
//It should only call select ONCE for each select
140+
expect(tab.select).toHaveBeenCalled();
141+
expect(_titles.eq(i)).toHaveClass('active');
142+
expect(contents().eq(i)).toHaveClass('active');
143+
} else {
144+
expect(tab.active).toBe(false);
145+
expect(_titles.eq(i)).not.toHaveClass('active');
146+
}
147+
});
148+
}
149+
150+
it('should make tab titles and set active tab active', function() {
151+
expect(titles().length).toBe(scope.tabs.length);
152+
expectTabActive(scope.tabs[2]);
153+
});
154+
});
155+
105156
describe('ng-repeat', function() {
106157

107158
beforeEach(inject(function($compile, $rootScope) {

0 commit comments

Comments
 (0)