From b66522ad4e6449790df781f6a5e62342c9083493 Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 5 Dec 2018 15:02:09 -0800 Subject: [PATCH 1/3] feat(tab): Get tabs by their ID --- packages/mdc-tab-bar/adapter.js | 6 +-- packages/mdc-tab-bar/foundation.js | 4 +- packages/mdc-tab-bar/index.js | 51 +++++++++++++++++++---- packages/mdc-tab/adapter.js | 12 +++++- packages/mdc-tab/index.js | 9 +++- test/unit/mdc-tab-bar/foundation.test.js | 2 +- test/unit/mdc-tab-bar/mdc-tab-bar.test.js | 6 ++- 7 files changed, 71 insertions(+), 19 deletions(-) diff --git a/packages/mdc-tab-bar/adapter.js b/packages/mdc-tab-bar/adapter.js index 24c6c8cfbf2..aa34269966c 100644 --- a/packages/mdc-tab-bar/adapter.js +++ b/packages/mdc-tab-bar/adapter.js @@ -25,7 +25,7 @@ /* eslint-disable no-unused-vars */ import {MDCTabDimensions} from '@material/tab/adapter'; -import {MDCTab} from '@material/tab/index'; +import {MDCTabFoundation} from '@material/tab/index'; /* eslint-enable no-unused-vars */ /** @@ -134,10 +134,10 @@ class MDCTabBarAdapter { /** * Returns the index of the given tab - * @param {!MDCTab} tab The tab whose index to determin + * @param {string} id The ID of the tab whose index to determine * @return {number} */ - getIndexOfTab(tab) {} + getIndexOfTabByID(id) {} /** * Emits the MDCTabBar:activated event diff --git a/packages/mdc-tab-bar/foundation.js b/packages/mdc-tab-bar/foundation.js index 93e1c92b401..bffec94f400 100644 --- a/packages/mdc-tab-bar/foundation.js +++ b/packages/mdc-tab-bar/foundation.js @@ -89,7 +89,7 @@ class MDCTabBarFoundation extends MDCFoundation { getTabDimensionsAtIndex: () => {}, getPreviousActiveTabIndex: () => {}, getFocusedTabIndex: () => {}, - getIndexOfTab: () => {}, + getIndexOfTabByID: () => {}, getTabListLength: () => {}, notifyTabActivated: () => {}, }); @@ -174,7 +174,7 @@ class MDCTabBarFoundation extends MDCFoundation { * @param {!Event} evt */ handleTabInteraction(evt) { - this.adapter_.setActiveTab(this.adapter_.getIndexOfTab(evt.detail.tab)); + this.adapter_.setActiveTab(this.adapter_.getIndexOfTabByID(evt.detail.tabId)); } /** diff --git a/packages/mdc-tab-bar/index.js b/packages/mdc-tab-bar/index.js index af6aaa4c76c..2fc89a7b03f 100644 --- a/packages/mdc-tab-bar/index.js +++ b/packages/mdc-tab-bar/index.js @@ -29,6 +29,8 @@ import {MDCTabScroller} from '@material/tab-scroller/index'; import MDCTabBarAdapter from './adapter'; import MDCTabBarFoundation from './foundation'; +let tabIdCounter = 0; + /** * @extends {MDCComponent} * @final @@ -84,13 +86,8 @@ class MDCTabBar extends MDCComponent { tabScrollerFactory = (el) => new MDCTabScroller(el)) { this.tabFactory_ = tabFactory; this.tabScrollerFactory_ = tabScrollerFactory; - - this.tabList_ = this.getTabElements_().map((el) => this.tabFactory_(el)); - - const tabScrollerElement = this.root_.querySelector(MDCTabBarFoundation.strings.TAB_SCROLLER_SELECTOR); - if (tabScrollerElement) { - this.tabScroller_ = this.tabScrollerFactory_(tabScrollerElement); - } + this.tabList_ = this.instantiateTabs_(this.tabFactory_); + this.tabScroller_ = this.instantiateTabsScroller_(this.tabScrollerFactory_); } initialSyncWithDOM() { @@ -147,7 +144,14 @@ class MDCTabBar extends MDCComponent { const activeElement = document.activeElement; return tabElements.indexOf(activeElement); }, - getIndexOfTab: (tabToFind) => this.tabList_.indexOf(tabToFind), + getIndexOfTabByID: (id) => { + for (let i = 0; i < this.tabList_.length; i++) { + if (this.tabList_[i].id === id) { + return i; + } + } + return -1; + }, getTabListLength: () => this.tabList_.length, notifyTabActivated: (index) => this.emit(MDCTabBarFoundation.strings.TAB_ACTIVATED_EVENT, {index}, true), }) @@ -170,9 +174,40 @@ class MDCTabBar extends MDCComponent { this.foundation_.scrollIntoView(index); } + /** + * Returns all the tab elements in a nice clean array + * @return {!Array} + * @private + */ getTabElements_() { return [].slice.call(this.root_.querySelectorAll(MDCTabBarFoundation.strings.TAB_SELECTOR)); } + + /** + * Instantiates tab components on all child tab elements + * @param {(function(!Element): !MDCTab)} tabFactory + * @return {!Array} + * @private + */ + instantiateTabs_(tabFactory) { + return this.getTabElements_().map((el) => { + el.id = el.id || `mdc-tab-${++tabIdCounter}`; + return tabFactory(el); + }); + } + + /** + * Instantiates tab scroller component on the child tab scroller element + * @param {(function(!Element): !MDCTabScroller)} tabScrollerFactory + * @return {!MDCTabScroller=} + * @private + */ + instantiateTabsScroller_(tabScrollerFactory) { + const tabScrollerElement = this.root_.querySelector(MDCTabBarFoundation.strings.TAB_SCROLLER_SELECTOR); + if (tabScrollerElement) { + return tabScrollerFactory(tabScrollerElement); + } + } } export {MDCTabBar, MDCTabBarFoundation}; diff --git a/packages/mdc-tab/adapter.js b/packages/mdc-tab/adapter.js index aed08db3ebd..868c53a5daa 100644 --- a/packages/mdc-tab/adapter.js +++ b/packages/mdc-tab/adapter.js @@ -31,6 +31,16 @@ */ let MDCTabDimensions; +/** + * @typedef {{ + * detail: { + * tabId: string, + * }, + * bubbles: boolean, + * }} + */ +let MDCTabInteractionEventType; + /** * Adapter for MDC Tab. * @@ -112,4 +122,4 @@ class MDCTabAdapter { focus() {} } -export {MDCTabDimensions, MDCTabAdapter}; +export {MDCTabDimensions, MDCTabInteractionEventType, MDCTabAdapter}; diff --git a/packages/mdc-tab/index.js b/packages/mdc-tab/index.js index 298f1209af6..879b603da9a 100644 --- a/packages/mdc-tab/index.js +++ b/packages/mdc-tab/index.js @@ -26,7 +26,7 @@ import MDCComponent from '@material/base/component'; /* eslint-disable no-unused-vars */ import {MDCRipple, MDCRippleFoundation, RippleCapableSurface} from '@material/ripple/index'; import {MDCTabIndicator, MDCTabIndicatorFoundation} from '@material/tab-indicator/index'; -import {MDCTabAdapter, MDCTabDimensions} from './adapter'; +import {MDCTabAdapter, MDCTabDimensions, MDCTabInteractionEventType} from './adapter'; /* eslint-enable no-unused-vars */ import MDCTabFoundation from './foundation'; @@ -41,6 +41,9 @@ class MDCTab extends MDCComponent { */ constructor(...args) { super(...args); + + /** @type {string} */ + this.id; /** @private {?MDCRipple} */ this.ripple_; /** @private {?MDCTabIndicator} */ @@ -63,6 +66,7 @@ class MDCTab extends MDCComponent { initialize( rippleFactory = (el, foundation) => new MDCRipple(el, foundation), tabIndicatorFactory = (el) => new MDCTabIndicator(el)) { + this.id = this.root_.id; const rippleSurface = this.root_.querySelector(MDCTabFoundation.strings.RIPPLE_SELECTOR); const rippleAdapter = Object.assign(MDCRipple.createAdapter(/** @type {!RippleCapableSurface} */ (this)), { addClass: (className) => rippleSurface.classList.add(className), @@ -101,7 +105,8 @@ class MDCTab extends MDCComponent { hasClass: (className) => this.root_.classList.contains(className), activateIndicator: (previousIndicatorClientRect) => this.tabIndicator_.activate(previousIndicatorClientRect), deactivateIndicator: () => this.tabIndicator_.deactivate(), - notifyInteracted: () => this.emit(MDCTabFoundation.strings.INTERACTED_EVENT, {tab: this}, true /* bubble */), + notifyInteracted: () => this.emit( + MDCTabFoundation.strings.INTERACTED_EVENT, {tabId: this.id}, true /* bubble */), getOffsetLeft: () => this.root_.offsetLeft, getOffsetWidth: () => this.root_.offsetWidth, getContentOffsetLeft: () => this.content_.offsetLeft, diff --git a/test/unit/mdc-tab-bar/foundation.test.js b/test/unit/mdc-tab-bar/foundation.test.js index 1eddd10f741..2cd07daf3ae 100644 --- a/test/unit/mdc-tab-bar/foundation.test.js +++ b/test/unit/mdc-tab-bar/foundation.test.js @@ -48,7 +48,7 @@ test('defaultAdapter returns a complete adapter implementation', () => { 'getOffsetWidth', 'isRTL', 'setActiveTab', 'activateTabAtIndex', 'deactivateTabAtIndex', 'focusTabAtIndex', 'getTabIndicatorClientRectAtIndex', 'getTabDimensionsAtIndex', - 'getPreviousActiveTabIndex', 'getFocusedTabIndex', 'getIndexOfTab', 'getTabListLength', + 'getPreviousActiveTabIndex', 'getFocusedTabIndex', 'getIndexOfTabByID', 'getTabListLength', 'notifyTabActivated', ]); }); diff --git a/test/unit/mdc-tab-bar/mdc-tab-bar.test.js b/test/unit/mdc-tab-bar/mdc-tab-bar.test.js index 625e9a67866..41bb78c3914 100644 --- a/test/unit/mdc-tab-bar/mdc-tab-bar.test.js +++ b/test/unit/mdc-tab-bar/mdc-tab-bar.test.js @@ -58,8 +58,10 @@ test('attachTo returns an MDCTabBar instance', () => { assert.isOk(MDCTabBar.attachTo(getFixture()) instanceof MDCTabBar); }); +let fakeTabIdCounter = 0; class FakeTab { constructor() { + this.id = `mdc-tab-${++fakeTabIdCounter}`; this.destroy = td.function(); this.activate = td.function(); this.deactivate = td.function(); @@ -206,10 +208,10 @@ test('#adapter.getPreviousActiveTabIndex returns the index of the active tab', ( assert.strictEqual(component.getDefaultFoundation().adapter_.getPreviousActiveTabIndex(), 1); }); -test('#adapter.getIndexOfTab returns the index of the given tab', () => { +test('#adapter.getIndexOfTabByID; returns the index of the given tab', () => { const {component} = setupTest(); const tab = component.tabList_[2]; - assert.strictEqual(component.getDefaultFoundation().adapter_.getIndexOfTab(tab), 2); + assert.strictEqual(component.getDefaultFoundation().adapter_.getIndexOfTabByID(tab.id), 2); }); test('#adapter.getTabListLength returns the length of the tab list', () => { From 1e861dad73d0132cc66f68c1363f384d4c405973 Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 5 Dec 2018 16:06:59 -0800 Subject: [PATCH 2/3] WIP updated from review --- packages/mdc-tab-bar/README.md | 2 +- packages/mdc-tab-bar/adapter.js | 3 +-- packages/mdc-tab-bar/foundation.js | 4 ++-- packages/mdc-tab-bar/index.js | 16 ++++------------ packages/mdc-tab/README.md | 2 +- test/unit/mdc-tab-bar/foundation.test.js | 2 +- test/unit/mdc-tab-bar/mdc-tab-bar.test.js | 4 ++-- 7 files changed, 12 insertions(+), 21 deletions(-) diff --git a/packages/mdc-tab-bar/README.md b/packages/mdc-tab-bar/README.md index 16f810f866b..0d3d5291c6b 100644 --- a/packages/mdc-tab-bar/README.md +++ b/packages/mdc-tab-bar/README.md @@ -133,7 +133,7 @@ Method Signature | Description `getTabListLength() => number` | Returns the number of child Tab components. `getPreviousActiveTabIndex() => number` | Returns the index of the previously active Tab. `getFocusedTabIndex() => number` | Returns the index of the focused Tab. -`getIndexOfTab(tab: MDCTab) => number` | Returns the index of the given Tab instance. +`getIndexOfTabById(id: string) => number` | Returns the index of the given Tab ID. `notifyTabActivated(index: number) => void` | Emits the `MDCTabBar:activated` event. ### `MDCTabBarFoundation` diff --git a/packages/mdc-tab-bar/adapter.js b/packages/mdc-tab-bar/adapter.js index aa34269966c..a24e8b10a6c 100644 --- a/packages/mdc-tab-bar/adapter.js +++ b/packages/mdc-tab-bar/adapter.js @@ -25,7 +25,6 @@ /* eslint-disable no-unused-vars */ import {MDCTabDimensions} from '@material/tab/adapter'; -import {MDCTabFoundation} from '@material/tab/index'; /* eslint-enable no-unused-vars */ /** @@ -137,7 +136,7 @@ class MDCTabBarAdapter { * @param {string} id The ID of the tab whose index to determine * @return {number} */ - getIndexOfTabByID(id) {} + getIndexOfTabById(id) {} /** * Emits the MDCTabBar:activated event diff --git a/packages/mdc-tab-bar/foundation.js b/packages/mdc-tab-bar/foundation.js index bffec94f400..e3b09fe3a75 100644 --- a/packages/mdc-tab-bar/foundation.js +++ b/packages/mdc-tab-bar/foundation.js @@ -89,7 +89,7 @@ class MDCTabBarFoundation extends MDCFoundation { getTabDimensionsAtIndex: () => {}, getPreviousActiveTabIndex: () => {}, getFocusedTabIndex: () => {}, - getIndexOfTabByID: () => {}, + getIndexOfTabById: () => {}, getTabListLength: () => {}, notifyTabActivated: () => {}, }); @@ -174,7 +174,7 @@ class MDCTabBarFoundation extends MDCFoundation { * @param {!Event} evt */ handleTabInteraction(evt) { - this.adapter_.setActiveTab(this.adapter_.getIndexOfTabByID(evt.detail.tabId)); + this.adapter_.setActiveTab(this.adapter_.getIndexOfTabById(evt.detail.tabId)); } /** diff --git a/packages/mdc-tab-bar/index.js b/packages/mdc-tab-bar/index.js index 2fc89a7b03f..69a6c3ea514 100644 --- a/packages/mdc-tab-bar/index.js +++ b/packages/mdc-tab-bar/index.js @@ -45,15 +45,9 @@ class MDCTabBar extends MDCComponent { /** @private {!Array} */ this.tabList_; - /** @type {(function(!Element): !MDCTab)} */ - this.tabFactory_; - /** @private {?MDCTabScroller} */ this.tabScroller_; - /** @type {(function(!Element): !MDCTabScroller)} */ - this.tabScrollerFactory_; - /** @private {?function(?Event): undefined} */ this.handleTabInteraction_; @@ -84,10 +78,8 @@ class MDCTabBar extends MDCComponent { initialize( tabFactory = (el) => new MDCTab(el), tabScrollerFactory = (el) => new MDCTabScroller(el)) { - this.tabFactory_ = tabFactory; - this.tabScrollerFactory_ = tabScrollerFactory; - this.tabList_ = this.instantiateTabs_(this.tabFactory_); - this.tabScroller_ = this.instantiateTabsScroller_(this.tabScrollerFactory_); + this.tabList_ = this.instantiateTabs_(tabFactory); + this.tabScroller_ = this.instantiateTabScroller_(tabScrollerFactory); } initialSyncWithDOM() { @@ -144,7 +136,7 @@ class MDCTabBar extends MDCComponent { const activeElement = document.activeElement; return tabElements.indexOf(activeElement); }, - getIndexOfTabByID: (id) => { + getIndexOfTabById: (id) => { for (let i = 0; i < this.tabList_.length; i++) { if (this.tabList_[i].id === id) { return i; @@ -202,7 +194,7 @@ class MDCTabBar extends MDCComponent { * @return {!MDCTabScroller=} * @private */ - instantiateTabsScroller_(tabScrollerFactory) { + instantiateTabScroller_(tabScrollerFactory) { const tabScrollerElement = this.root_.querySelector(MDCTabBarFoundation.strings.TAB_SCROLLER_SELECTOR); if (tabScrollerElement) { return tabScrollerFactory(tabScrollerElement); diff --git a/packages/mdc-tab/README.md b/packages/mdc-tab/README.md index b491f11ec7d..f9e655c2ff9 100644 --- a/packages/mdc-tab/README.md +++ b/packages/mdc-tab/README.md @@ -154,7 +154,7 @@ Method Signature | Description Event Name | Event Data Structure | Description --- | --- | --- -`MDCTab:interacted` | `{"detail": {"tab": MDCTab}}` | Emitted when the Tab is interacted with, regardless of its active state. Used by parent components to know which Tab to activate. +`MDCTab:interacted` | `{"detail": {"tabId": string}}` | Emitted when the Tab is interacted with, regardless of its active state. Used by parent components to know which Tab to activate. ## Usage within Web Frameworks diff --git a/test/unit/mdc-tab-bar/foundation.test.js b/test/unit/mdc-tab-bar/foundation.test.js index 2cd07daf3ae..5b99d5ca70e 100644 --- a/test/unit/mdc-tab-bar/foundation.test.js +++ b/test/unit/mdc-tab-bar/foundation.test.js @@ -48,7 +48,7 @@ test('defaultAdapter returns a complete adapter implementation', () => { 'getOffsetWidth', 'isRTL', 'setActiveTab', 'activateTabAtIndex', 'deactivateTabAtIndex', 'focusTabAtIndex', 'getTabIndicatorClientRectAtIndex', 'getTabDimensionsAtIndex', - 'getPreviousActiveTabIndex', 'getFocusedTabIndex', 'getIndexOfTabByID', 'getTabListLength', + 'getPreviousActiveTabIndex', 'getFocusedTabIndex', 'getIndexOfTabById', 'getTabListLength', 'notifyTabActivated', ]); }); diff --git a/test/unit/mdc-tab-bar/mdc-tab-bar.test.js b/test/unit/mdc-tab-bar/mdc-tab-bar.test.js index 41bb78c3914..b946de0b40c 100644 --- a/test/unit/mdc-tab-bar/mdc-tab-bar.test.js +++ b/test/unit/mdc-tab-bar/mdc-tab-bar.test.js @@ -208,10 +208,10 @@ test('#adapter.getPreviousActiveTabIndex returns the index of the active tab', ( assert.strictEqual(component.getDefaultFoundation().adapter_.getPreviousActiveTabIndex(), 1); }); -test('#adapter.getIndexOfTabByID; returns the index of the given tab', () => { +test('#adapter.getIndexOfTabById returns the index of the given tab', () => { const {component} = setupTest(); const tab = component.tabList_[2]; - assert.strictEqual(component.getDefaultFoundation().adapter_.getIndexOfTabByID(tab.id), 2); + assert.strictEqual(component.getDefaultFoundation().adapter_.getIndexOfTabById(tab.id), 2); }); test('#adapter.getTabListLength returns the length of the tab list', () => { From 768c5990b51a5176a7f411f08ec65ffb1d489c66 Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 5 Dec 2018 17:03:14 -0800 Subject: [PATCH 3/3] WIP ugh closure, why... --- packages/mdc-tab-bar/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/mdc-tab-bar/index.js b/packages/mdc-tab-bar/index.js index 69a6c3ea514..14615e6270b 100644 --- a/packages/mdc-tab-bar/index.js +++ b/packages/mdc-tab-bar/index.js @@ -168,7 +168,7 @@ class MDCTabBar extends MDCComponent { /** * Returns all the tab elements in a nice clean array - * @return {!Array} + * @return {!Array} * @private */ getTabElements_() { @@ -191,7 +191,7 @@ class MDCTabBar extends MDCComponent { /** * Instantiates tab scroller component on the child tab scroller element * @param {(function(!Element): !MDCTabScroller)} tabScrollerFactory - * @return {!MDCTabScroller=} + * @return {?MDCTabScroller} * @private */ instantiateTabScroller_(tabScrollerFactory) { @@ -199,6 +199,7 @@ class MDCTabBar extends MDCComponent { if (tabScrollerElement) { return tabScrollerFactory(tabScrollerElement); } + return null; } }