Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tabs] Add false as a valid index value #6945

Merged
merged 1 commit into from
May 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/src/pages/component-api/Tabs/Tabs.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
| children | node | | The content of the component. |
| classes | object | | Useful to extend the style applied to components. |
| fullWidth | bool | false | If `true`, the tabs will grow to use all the available space. This property is intended for small views. |
| <span style="color: #31a148">index *</span> | number | | The index of the currently selected `Tab`. |
| <span style="color: #31a148">index *</span> | union:&nbsp;[object Object]<br>&nbsp;number<br> | | The index of the currently selected `Tab`. If you don't want any selected `Tab`, you can set this property to `false`. |
| indicatorClassName | string | | The CSS class name of the indicator element. |
| indicatorColor | union:&nbsp;[object Object]<br>&nbsp;string<br> | 'accent' | Determines the color of the indicator. |
| <span style="color: #31a148">onChange *</span> | function | | Function called when the index change. |
Expand Down
41 changes: 28 additions & 13 deletions src/Tabs/Tabs.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @flow weak

import React, { Component, Children, cloneElement } from 'react';
import warning from 'warning';
import PropTypes from 'prop-types';
import compose from 'recompose/compose';
import classNames from 'classnames';
Expand Down Expand Up @@ -146,9 +147,18 @@ class Tabs extends Component {
};

getTabsMeta = index => {
const tabsMeta = this.tabs.getBoundingClientRect();
tabsMeta.scrollLeft = this.tabs.scrollLeft;
const tabMeta = this.tabs.children[0].children[index].getBoundingClientRect();
let tabsMeta;
if (this.tabs) {
tabsMeta = this.tabs.getBoundingClientRect();
tabsMeta.scrollLeft = this.tabs.scrollLeft;
}

let tabMeta;
if (this.tabs && index !== false) {
const tab = this.tabs.children[0].children[index];
warning(tab, `Material-UI: the index provided \`${index}\` is invalid`);
tabMeta = tab ? this.tabs.children[0].children[index].getBoundingClientRect() : null;
}
return { tabsMeta, tabMeta };
};

Expand All @@ -158,22 +168,26 @@ class Tabs extends Component {
};

updateIndicatorState(props) {
if (this.tabs) {
const { tabsMeta, tabMeta } = this.getTabsMeta(props.index);
const { tabsMeta, tabMeta } = this.getTabsMeta(props.index);

const indicatorStyle = {
left: tabMeta.left + (tabsMeta.scrollLeft - tabsMeta.left),
width: tabMeta.width, // May be wrong until the font is loaded.
};
const indicatorStyle = {
left: tabMeta && tabsMeta ? tabMeta.left + (tabsMeta.scrollLeft - tabsMeta.left) : 0,
// May be wrong until the font is loaded.
width: tabMeta ? tabMeta.width : 0,
};

if (!isEqual(indicatorStyle, this.state.indicatorStyle)) {
this.setState({ indicatorStyle });
}
if (!isEqual(indicatorStyle, this.state.indicatorStyle)) {
this.setState({ indicatorStyle });
}
}

scrollSelectedIntoView = () => {
const { tabsMeta, tabMeta } = this.getTabsMeta(this.props.index);

if (!tabMeta || !tabsMeta) {
return;
}

if (tabMeta.left < tabsMeta.left) {
// left side of button is out of view
const nextScrollLeft = tabsMeta.scrollLeft + (tabMeta.left - tabsMeta.left);
Expand Down Expand Up @@ -302,8 +316,9 @@ Tabs.propTypes = {
fullWidth: PropTypes.bool,
/**
* The index of the currently selected `Tab`.
* If you don't want any selected `Tab`, you can set this property to `false`.
*/
index: PropTypes.number.isRequired,
index: PropTypes.oneOfType([PropTypes.oneOf([false]), PropTypes.number]).isRequired,
/**
* The CSS class name of the indicator element.
*/
Expand Down
30 changes: 29 additions & 1 deletion src/Tabs/Tabs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import React from 'react';
import { assert } from 'chai';
import { spy, stub } from 'sinon';
import scroll from 'scroll';
import { createShallow, createMount } from '../test-utils';
import { createShallow, createMount, consoleErrorMock } from '../test-utils';
import Tabs, { styleSheet } from './Tabs';
import TabScrollButton from './TabScrollButton';
import TabIndicator from './TabIndicator';
import Tab from './Tab';

const noop = () => {};
Expand Down Expand Up @@ -73,6 +74,10 @@ describe('<Tabs />', () => {
);
});

after(() => {
consoleErrorMock.reset();
});

it('should pass selected prop to children', () => {
assert.strictEqual(
wrapper.find(Tab).at(0).props().selected,
Expand All @@ -95,6 +100,29 @@ describe('<Tabs />', () => {
'should have switched to false',
);
});

it('should accept a false value', () => {
const wrapper2 = mount(
<Tabs width="md" onChange={noop} index={false}>
<Tab />
<Tab />
</Tabs>,
);
assert.strictEqual(wrapper2.find(TabIndicator).props().style.width, 0);
});

it('should warn when the index is invalid', () => {
consoleErrorMock.spy();
mount(
<Tabs width="md" onChange={noop} index={2}>
<Tab />
<Tab />
</Tabs>,
);
assert.strictEqual(consoleErrorMock.callCount(), 2);
assert.strictEqual(consoleErrorMock.args()[0][0],
'Warning: Material-UI: the index provided `2` is invalid');
});
});

describe('prop: onChange', () => {
Expand Down
8 changes: 8 additions & 0 deletions src/test-utils/consoleErrorMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ class ConsoleErrorMock {

throw new Error('Requested call count before spy() was called');
};

args = () => {
if (this.consoleErrorContainer) {
return console.error.args;
}

throw new Error('Requested call count before spy() was called');
};
}

export default new ConsoleErrorMock();
22 changes: 13 additions & 9 deletions src/test-utils/consoleErrorMock.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,20 @@ import { assert } from 'chai';
import consoleErrorMock from './consoleErrorMock';

describe('consoleErrorMock()', () => {
it('should throw error when calling callCount() before spy()', () => {
let errorCalledFlag = false;

try {
consoleErrorMock.callCount();
} catch (error) {
errorCalledFlag = true;
}
describe('callCount()', () => {
it('should throw error when calling callCount() before spy()', () => {
assert.throws(() => {
consoleErrorMock.callCount();
}, 'Requested call count before spy() was called');
});
});

assert.strictEqual(errorCalledFlag, true);
describe('args()', () => {
it('should throw error when calling args() before spy()', () => {
assert.throws(() => {
consoleErrorMock.args();
}, 'Requested call count before spy() was called');
});
});

describe('spy()', () => {
Expand Down