fix(tab): Set initial tabIndex state to -1 (#690) #691
Conversation
Codecov Report
@@ Coverage Diff @@
## rc0.11.0 #691 +/- ##
=========================================
Coverage 94.88% 94.88%
=========================================
Files 68 68
Lines 2853 2853
Branches 432 432
=========================================
Hits 2707 2707
Misses 50 50
Partials 96 96
Continue to review full report at Codecov.
|
| }); | ||
|
|
||
| test('sets the tabIndex to -1 if props.active is false on mount', () => { | ||
| const wrapper = shallow(<Tab active={false} />); |
There was a problem hiding this comment.
Could we update this to set no active property instead of explicitly setting active={false}? Will the test still pass?
There was a problem hiding this comment.
Typically with these props, it wouldn't be hard-coded. It would usually look more like:
<Tab active={this.state.active} />
In that case it is more helpful to have that prop explicitly defined like that. Otherwise removing it would look like:
const props = {};
if (this.state.active) {
props.active = true;
}
<Tab {...props} />
There was a problem hiding this comment.
Fair enough, thanks for explaining. My thinking was that removing the active={false} prop tests that a Tab has tab-index=-1 by default without the user providing any active prop. However, that's more a test of Component.defaultProps working than of the change here.
| }); | ||
|
|
||
| test('sets the tabIndex to -1 if props.active is false on mount', () => { | ||
| const wrapper = shallow(<Tab active={false} />); |
There was a problem hiding this comment.
Fair enough, thanks for explaining. My thinking was that removing the active={false} prop tests that a Tab has tab-index=-1 by default without the user providing any active prop. However, that's more a test of Component.defaultProps working than of the change here.
|
That totally makes sense...and I had a double take for a second :) |
Sets the initial state for the
tabIndexthe same way it is set foraria-selected. If a tab is active then theactivatemethod on the foundation will update this state on mount.