feat(tab): Emit selection and activation events #3139
Conversation
patrickrodee
assigned kfranqueiro, moog16, lynnmercier and acdvorak and unassigned lynnmercier, kfranqueiro and moog16
test/unit/mdc-tab/foundation.test.js
Outdated
| td.verify(mockAdapter.notifySelected(), {times: 1}); | ||
| }); | ||
|
|
||
| test('on click, do nothing if the Tab is already active', () => { |
There was a problem hiding this comment.
Isn't this basically doing the same thing as 2 tests up?
There was a problem hiding this comment.
Yes, but ensures we get 100% coverage.
There was a problem hiding this comment.
Should we just be verifying that handleClick is called here, then let the test above verify the rest?
packages/mdc-tab/README.md
Outdated
| @@ -114,12 +114,15 @@ Method Signature | Description | |||
| `getOffsetWidth() => number` | Returns the `offsetWidth` value of the root element | |||
| `getContentOffsetLeft() => number` | Returns the `offsetLeft` value of the content element | |||
| `getContentOffsetWidth() => number` | Returns the `offsetWidth` value of the content element | |||
| `notifySelected() => void` | Emits the `MDCTab:selected` event | |||
There was a problem hiding this comment.
Is there any other word we can use for this event? Like notifyInteracted or something?
My problem with "selected" is that it overlaps with another term in the states subsystem... and generally you never mix selected and activated.
There was a problem hiding this comment.
notifyInteracted is cool by me. I'll update.
packages/mdc-tab/README.md
Outdated
| @@ -96,6 +96,11 @@ Method Signature | Description | |||
| `computeIndicatorClientRect() => ClientRect` | Returns the bounding client rect of the indicator. | |||
| `computeDimensions() => MDCTabDimensions` | Returns the dimensions of the Tab. | |||
|
|
|||
| Event Name | Event Data Structure | Description | |||
| --- | --- | --- | |||
| `MDCTab:interacted` | `{"detail": {"tab": MDCTab}}` | Emitted when the Tab is interacted with but not active. Used by parent components. | |||
There was a problem hiding this comment.
I had some initial misunderstanding of when this fires, which I discussed with Pat.
I think this will be clearer to express in the context of Tab if it emits regardless of whether it's active, and update the docs from "but not active" to "regardless of active state".
test/unit/mdc-tab/foundation.test.js
Outdated
| td.verify(mockAdapter.notifyInteracted(td.matchers.anything()), {times: 0}); | ||
| }); | ||
|
|
||
| test('#handleClick emits the selected event if it is not active', () => { |
There was a problem hiding this comment.
selected -> interacted
demos/tab.html
Outdated
| @@ -332,10 +332,10 @@ <h2>Customization</h2> | |||
| activeTab = tab; | |||
| } | |||
|
|
|||
| tabEl.addEventListener('click', function() { | |||
| tabEl.addEventListener('MDCTab:selected', function(evt) { | |||
There was a problem hiding this comment.
selected -> interacted (assuming this should still be using this and not activated)
Codecov Report
@@ Coverage Diff @@
## feat/tabs/tabs #3139 +/- ##
=================================================
Coverage ? 98.42%
=================================================
Files ? 113
Lines ? 4818
Branches ? 600
=================================================
Hits ? 4742
Misses ? 76
Partials ? 0
Continue to review full report at Codecov.
|
WIP fixed bg coloring of icons fix(tab-indicator): Use absolute positioning (#2547) WIP start of tab scroller WIP fixed transition duration WIP progress on scroller WIP added demos back chore(tabs): Removed tab scroller feat(tabs): Add tab indicator inside tab (#2565) feat(tab-scroller): Add tab scroller (#2577) Merge master into feat/tabs/tabs (#3096) feat(tab): Update tab color and typography (#3108) docs(tabs): Update metadata and synopses (#3117) feat(tab): Add MDCTabDimensions computation (#3122) feat(tab): Emit selection and activation events (#3139) docs(tabs): Update new READMEs to match standard (#3142) feat(tab): Give focus to tab when activated (#3164) feat(tab): Add mixin for parent positioning; Use mixin in tab scroller (#3179) fix(tabs): Suppress area occupied by scrollbar on platforms that show it (#3149) fix(tab): Remove extraneous padding from the stacked text label in LTR (#3193) feat(tabs): Add missing docs and create helper util API (#3194) Merge master into feat/tabs/tabs (#3227) feat(tab): Update layout; Add fixed-width mixin; Add min-width class (#3220) fix(tab-scroller): Fix incorrect animation stopping scroll value in RTL (#3237) feat(tab-scroller): Add scroll content width method for use in tab bar (#3222) feat(tab): Remove activation event emitting (#3242) feat(tab-bar): Add tab bar (#3229)
Add
notifySelectedandnotifyActivatedmethods to adapter. AddhandleClickto the foundation. Update tests and demo. Update readme. Closes #3123