Skip to content

Commit

Permalink
fix(tab-bar): Early exit (#3386)
Browse files Browse the repository at this point in the history
Move the early exit for activating a tab from Tab to TabBar.
  • Loading branch information
bonniezhou authored Aug 22, 2018
1 parent 3fec896 commit f0ebfea
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 17 deletions.
7 changes: 2 additions & 5 deletions packages/mdc-tab-bar/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,15 @@ class MDCTabBarFoundation extends MDCFoundation {
*/
activateTab(index) {
const previousActiveIndex = this.adapter_.getActiveTabIndex();
if (!this.indexIsInRange_(index)) {
if (!this.indexIsInRange_(index) || index === previousActiveIndex) {
return;
}

this.adapter_.deactivateTabAtIndex(previousActiveIndex);
this.adapter_.activateTabAtIndex(index, this.adapter_.getTabIndicatorClientRectAtIndex(previousActiveIndex));
this.scrollIntoView(index);

// Only notify the tab activation if the index is different than the previously active index
if (index !== previousActiveIndex) {
this.adapter_.notifyTabActivated(index);
}
this.adapter_.notifyTabActivated(index);
}

/**
Expand Down
5 changes: 0 additions & 5 deletions packages/mdc-tab/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,6 @@ class MDCTabFoundation extends MDCFoundation {
* @param {!ClientRect=} previousIndicatorClientRect
*/
activate(previousIndicatorClientRect) {
// Early exit
if (this.isActive()) {
return;
}

this.adapter_.addClass(cssClasses.ACTIVE);
this.adapter_.setAttr(strings.ARIA_SELECTED, 'true');
this.adapter_.setAttr(strings.TABINDEX, '0');
Expand Down
9 changes: 9 additions & 0 deletions test/unit/mdc-tab-bar/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,15 @@ test('#activateTab() does nothing if the index underflows the tab list', () => {
td.verify(mockAdapter.activateTabAtIndex(td.matchers.isA(Number)), {times: 0});
});

test('#activateTab() does nothing if the index is the same as the previous active index', () => {
const {foundation, mockAdapter} = setupActivateTabTest();
td.when(mockAdapter.getActiveTabIndex()).thenReturn(0);
td.when(mockAdapter.getTabListLength()).thenReturn(13);
foundation.activateTab(0);
td.verify(mockAdapter.deactivateTabAtIndex(td.matchers.isA(Number)), {times: 0});
td.verify(mockAdapter.activateTabAtIndex(td.matchers.isA(Number)), {times: 0});
});

test(`#activateTab() does not emit the ${MDCTabBarFoundation.strings.TAB_ACTIVATED_EVENT} event if the index` +
' is the currently active index', () => {
const {foundation, mockAdapter} = setupActivateTabTest();
Expand Down
7 changes: 0 additions & 7 deletions test/unit/mdc-tab/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ test('defaultAdapter returns a complete adapter implementation', () => {

const setupTest = () => setupFoundationTest(MDCTabFoundation);

test('#activate does nothing if already active', () => {
const {foundation, mockAdapter} = setupTest();
td.when(mockAdapter.hasClass(MDCTabFoundation.cssClasses.ACTIVE)).thenReturn(true);
foundation.activate();
td.verify(mockAdapter.addClass(MDCTabFoundation.cssClasses.ACTIVE), {times: 0});
});

test('#activate adds mdc-tab--active class to the root element', () => {
const {foundation, mockAdapter} = setupTest();
foundation.activate();
Expand Down

0 comments on commit f0ebfea

Please sign in to comment.