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

feat(tab): allow active index string #5713

Closed
wants to merge 1 commit into from
Closed

feat(tab): allow active index string #5713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/tabs/docs/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ AngularJS version of the tabs directive.
* `active`
<i class="glyphicon glyphicon-eye-open"></i>
_(Default: `Index of first tab`)_ -
Active index of tab. Setting this to an existing tab index will make that tab active.
Active index of tab. Setting this to an existing tab index will make that tab active. Can be a number or string.

* `justified`
<small class="badge">$</small>
Expand Down
4 changes: 2 additions & 2 deletions src/tabs/tabs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ angular.module('ui.bootstrap.tabs', [])
};

$scope.$watch('tabset.active', function(val) {
if (angular.isNumber(val) && val !== oldIndex) {
if ((angular.isNumber(val) || angular.isString(val)) && val !== oldIndex) {
ctrl.select(findTabIndex(val));
}
});
Expand All @@ -85,7 +85,7 @@ angular.module('ui.bootstrap.tabs', [])

function findTabIndex(index) {
for (var i = 0; i < ctrl.tabs.length; i++) {
if (ctrl.tabs[i].index === index) {
if (ctrl.tabs[i].index === +index) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was not in the pull request I started from, but it seemed needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a strange change - do you know why this is as you have it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was getting called from line 76 where we were now allowing the value to be a string.

The comparison wouldn't work for 1 === "1".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why is the type getting mutated?

return i;
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/tabs/test/tabs.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ describe('tabs', function() {
expect(titles().length).toBe(scope.tabs.length);
expectTabActive(scope.tabs[2]);
});

it("should watch active state", function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use single quotes here

var controller = elm.controller('uibTabset');
spyOn(controller, "select");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

expect(titles().length).toBe(scope.tabs.length);
expectTabActive(scope.tabs[2]);
scope.active = "7";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

scope.$apply();
expect(controller.select).toHaveBeenCalledWith(3);
});
});

describe('without active binding and index attributes', function() {
Expand Down