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

Commit

Permalink
fix(tab): fix unexpected routing
Browse files Browse the repository at this point in the history
- Change anchor tag to `div` to prevent unexpected routing being triggered by the browser on generation of anchor tags with nested clickable controls

BREAKING CHANGE: This causes the cursor style to be removed from the tab - implement CSS on the `.uib-tab > div` selector, or similar, accordingly

Closes #4793
Fixes #3266
  • Loading branch information
wesleycho committed Oct 31, 2015
1 parent df211bd commit d59083b
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 20 deletions.
6 changes: 6 additions & 0 deletions src/tabs/docs/demo.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
<style>
.uib-tab > div {
cursor: pointer;
}
</style>

<div ng-controller="TabsDemoCtrl">
<p>Select a tab by setting active binding to true:</p>
<p>
Expand Down
36 changes: 18 additions & 18 deletions src/tabs/test/tabs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ describe('tabs', function() {
it('should create clickable titles', function() {
var t = titles();
expect(t.length).toBe(2);
expect(t.find('a').eq(0).text()).toBe('First Tab 1');
expect(t.find('> div').eq(0).text()).toBe('First Tab 1');
//It should put the uib-tab-heading element into the 'a' title
expect(t.find('a').eq(1).children().is('uib-tab-heading')).toBe(true);
expect(t.find('a').eq(1).children().html()).toBe('<b>Second</b> Tab 2');
expect(t.find('> div').eq(1).children().is('uib-tab-heading')).toBe(true);
expect(t.find('> div').eq(1).children().html()).toBe('<b>Second</b> Tab 2');
});

it('should bind tabs content and set first tab active', function() {
Expand All @@ -76,7 +76,7 @@ describe('tabs', function() {
});

it('should change active on click', function() {
titles().eq(1).find('a').click();
titles().eq(1).find('> div').click();
expect(contents().eq(1)).toHaveClass('active');
expect(titles().eq(0)).not.toHaveClass('active');
expect(titles().eq(1)).toHaveClass('active');
Expand All @@ -85,17 +85,17 @@ describe('tabs', function() {
});

it('should call select callback on select', function() {
titles().eq(1).find('a').click();
titles().eq(1).find('> div').click();
expect(scope.selectSecond).toHaveBeenCalled();
titles().eq(0).find('a').click();
titles().eq(0).find('> div').click();
expect(scope.selectFirst).toHaveBeenCalled();
});

it('should call deselect callback on deselect', function() {
titles().eq(1).find('a').click();
titles().eq(0).find('a').click();
titles().eq(1).find('> div').click();
titles().eq(0).find('> div').click();
expect(scope.deselectSecond).toHaveBeenCalled();
titles().eq(1).find('a').click();
titles().eq(1).find('> div').click();
expect(scope.deselectFirst).toHaveBeenCalled();
});
});
Expand Down Expand Up @@ -181,13 +181,13 @@ describe('tabs', function() {
execOrder = [];

// Select second tab
titles().eq(1).find('a').click();
titles().eq(1).find('> div').click();
expect(execOrder).toEqual([ 'deselect1', 'select2' ]);

execOrder = [];

// Select again first tab
titles().eq(0).find('a').click();
titles().eq(0).find('> div').click();
expect(execOrder).toEqual([ 'deselect2', 'select1' ]);
});
});
Expand Down Expand Up @@ -250,7 +250,7 @@ describe('tabs', function() {
});

it('should switch active when clicking', function() {
titles().eq(3).find('a').click();
titles().eq(3).find('> div').click();
expectTabActive(scope.tabs[3]);
});

Expand Down Expand Up @@ -317,7 +317,7 @@ describe('tabs', function() {
}));

function heading() {
return elm.find('ul li a').children();
return elm.find('ul li > div').children();
}

it('should create a heading bound to myHtml', function() {
Expand Down Expand Up @@ -379,7 +379,7 @@ describe('tabs', function() {

it('should preserve correct ordering', function() {
function titles() {
return elm.find('ul.nav-tabs li a');
return elm.find('ul.nav-tabs li > div');
}
scope.$apply();
expect(titles().length).toBe(9);
Expand Down Expand Up @@ -522,7 +522,7 @@ describe('tabs', function() {
expectContents(['Hello', 'content 1', 'content 2', 'content 3']);

// Select last tab
titles().find('a').eq(3).click();
titles().find('> div').eq(3).click();
expect(contents().eq(3)).toHaveClass('active');
expect(titles().eq(3)).toHaveClass('active');

Expand All @@ -536,7 +536,7 @@ describe('tabs', function() {
expect(contents().eq(2)).toHaveClass('active');

// Select 2nd tab ("tab 1")
titles().find('a').eq(1).click();
titles().find('> div').eq(1).click();
expect(titles().eq(1)).toHaveClass('active');
expect(contents().eq(1)).toHaveClass('active');

Expand Down Expand Up @@ -632,10 +632,10 @@ describe('tabs', function() {
}

it('should not switch active when clicking on title', function() {
titles().eq(2).find('a').click();
titles().eq(2).find('> div').click();
expectTabActive(scope.tabs[2]);

titles().eq(3).find('a').click();
titles().eq(3).find('> div').click();
expectTabActive(scope.tabs[2]);
});

Expand Down
4 changes: 2 additions & 2 deletions template/tabs/tab.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<li ng-class="{active: active, disabled: disabled}">
<a href ng-click="select()" uib-tab-heading-transclude>{{heading}}</a>
<li ng-class="{active: active, disabled: disabled}" class="uib-tab">
<div ng-click="select()" uib-tab-heading-transclude>{{heading}}</div>
</li>

1 comment on commit d59083b

@justin-john
Copy link

Choose a reason for hiding this comment

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

@wesleycho The change of anchor tag with href to div tag will break accessibility feature. The tab won't be switched from keyboard. Could you please share your thought on this?

Please sign in to comment.