Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(tab-bar): Early exit #3386

Merged
merged 3 commits into from
Aug 22, 2018
Merged

fix(tab-bar): Early exit #3386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00fbd50
fix(tab-bar): Move activateTab early exit
bonniezhou Aug 18, 2018
7d388b2
Tests and remove tab early exit
bonniezhou Aug 20, 2018
fd1ea75
Merge branch 'master' into fix/tab-bar/early-exit
patrickrodee Aug 22, 2018
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
7 changes: 2 additions & 5 deletions packages/mdc-tab-bar/foundation.js
View file
Open in desktop
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
View file
Open in desktop
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()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this have any problems if an already active tab is activated again when the tab is outside of the tab bar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope it looks fine, based on the demos page which calls activate on the tab directly (without a tab bar)

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
View file
Open in desktop
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
View file
Open in desktop
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