Skip to content

Commit f2b1510

Browse files
arturbienarturbien
authored
Merge pull request #95 from arturbien/feature/tabs-enhancements-and-tests
Feature/tabs enhancements and tests
2 parents 263d13a + fab899f commit f2b1510

File tree

7 files changed

+221
-89
lines changed

7 files changed

+221
-89
lines changed

‎src/components/Tab/Tab.js‎

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@ import styled from 'styled-components';
55
import { createBorderStyles, createBoxStyles } from '../common';
66
import { blockSizes, padding } from '../common/system';
77

8-
const StyledTab = styled.div`
8+
const StyledTab = styled.button`
99
${createBoxStyles()}
1010
${createBorderStyles()}
1111
position: relative;
12+
display: inline-flex;
13+
align-items: center;
14+
justify-content: center;
15+
font-size: 1rem;
1216
height: ${blockSizes.md};
1317
line-height: ${blockSizes.md};
1418
padding: 0 ${padding.sm};
@@ -18,12 +22,14 @@ const StyledTab = styled.div`
1822
margin-bottom: -2px;
1923
cursor: default;
2024
color: ${({ theme }) => theme.text};
25+
user-select: none;
26+
2127
${props =>
22-
props.active &&
28+
props.selected &&
2329
`
2430
z-index: 1;
2531
height: calc(${blockSizes.md} + 4px);
26-
top: -4px;
32+
top: -3px;
2733
margin-bottom: -6px;
2834
padding: 0 calc(${padding.sm} + 8px);
2935
margin-left: -8px;
@@ -33,47 +39,42 @@ const StyledTab = styled.div`
3339
content: '';
3440
position: absolute;
3541
width: calc(100% - 4px);
36-
height: 4px;
42+
height: 6px;
3743
3844
background: ${({ theme }) => theme.material};
39-
bottom: -2px;
45+
bottom: -3px;
4046
left: 2px;
4147
}
4248
`;
43-
const Tab = ({
44-
value,
45-
onClick,
46-
active,
47-
children,
48-
className,
49-
style,
50-
...otherProps
51-
}) => (
52-
<StyledTab
53-
className={className}
54-
active={active}
55-
style={style}
56-
{...otherProps}
57-
onClick={() => onClick(value)}
58-
>
59-
{children}
60-
</StyledTab>
61-
);
49+
50+
// TODO handle tabIndex
51+
const Tab = React.forwardRef(function Tab(props, ref) {
52+
const { value, onClick, selected, children, ...otherProps } = props;
53+
54+
return (
55+
<StyledTab
56+
selected={selected}
57+
aria-selected={selected}
58+
onClick={() => onClick(value)}
59+
role='tab'
60+
ref={ref}
61+
{...otherProps}
62+
>
63+
{children}
64+
</StyledTab>
65+
);
66+
});
6267

6368
Tab.defaultProps = {
6469
onClick: () => {},
65-
active: false,
66-
children: null,
67-
className: '',
68-
style: {}
70+
selected: false,
71+
children: null
6972
};
7073

7174
Tab.propTypes = {
7275
value: propTypes.number.isRequired,
7376
onClick: propTypes.func,
74-
active: propTypes.bool,
75-
children: propTypes.node,
76-
className: propTypes.string,
77-
style: propTypes.shape([propTypes.string, propTypes.number])
77+
selected: propTypes.bool,
78+
children: propTypes.node
7879
};
7980
export default Tab;

‎src/components/Tab/Tab.spec.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import React from 'react';
2+
3+
import { renderWithTheme } from '../../../test/utils';
4+
import Tab from './Tab';
5+
6+
describe('<Tab />', () => {
7+
describe('prop: children', () => {
8+
it('should render passed child', () => {
9+
const child = 'Hey there!';
10+
const { getByText } = renderWithTheme(<Tab>{child}</Tab>);
11+
12+
expect(getByText(child)).toBeInTheDocument();
13+
});
14+
});
15+
16+
describe('prop: selected', () => {
17+
it('should render with correct aria attribute', () => {
18+
const { getByRole } = renderWithTheme(<Tab selected />);
19+
20+
expect(getByRole('tab')).toHaveAttribute('aria-selected', 'true');
21+
});
22+
});
23+
24+
describe('prop: onClick', () => {
25+
it('should be called when a click is triggered', () => {
26+
const handleClick = jest.fn();
27+
const { getByRole } = renderWithTheme(<Tab onClick={handleClick} />);
28+
29+
getByRole('tab').click();
30+
31+
expect(handleClick).toHaveBeenCalledTimes(1);
32+
});
33+
});
34+
});

‎src/components/TabBody/TabBody.js‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,16 @@ const StyledTabBody = styled.div`
1212
display: block;
1313
height: 100%;
1414
padding: ${padding.md};
15-
padding-top: calc(1.5 * ${padding.md});
1615
`;
17-
const TabBody = ({ children, className, style, ...otherProps }) => (
18-
<StyledTabBody className={className} style={style} {...otherProps}>
19-
{children}
20-
</StyledTabBody>
16+
const TabBody = ({ children, ...otherProps }) => (
17+
<StyledTabBody {...otherProps}>{children}</StyledTabBody>
2118
);
2219

2320
TabBody.defaultProps = {
24-
children: null,
25-
className: '',
26-
style: {}
21+
children: null
2722
};
2823

2924
TabBody.propTypes = {
30-
children: propTypes.node,
31-
className: propTypes.string,
32-
style: propTypes.shape([propTypes.string, propTypes.number])
25+
children: propTypes.node
3326
};
3427
export default TabBody;

‎src/components/TabBody/TabBody.spec.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import React from 'react';
2+
3+
import { renderWithTheme } from '../../../test/utils';
4+
import TabBody from './TabBody';
5+
6+
describe('<TabBody />', () => {
7+
describe('prop: children', () => {
8+
it('should render passed child', () => {
9+
const child = 'Hey there!';
10+
const { getByText } = renderWithTheme(<TabBody>{child}</TabBody>);
11+
12+
expect(getByText(child)).toBeInTheDocument();
13+
});
14+
});
15+
});

‎src/components/Tabs/Tabs.js‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,47 +3,40 @@ import propTypes from 'prop-types';
33

44
import styled from 'styled-components';
55

6-
const StyledTabs = styled.nav`
6+
const StyledTabs = styled.div`
77
position: relative;
88
left: 8px;
99
text-align: left;
1010
`;
1111

12-
const Tabs = ({
13-
value,
14-
onChange,
15-
children,
16-
className,
17-
style,
18-
...otherProps
19-
}) => {
12+
const Tabs = React.forwardRef(function Tabs(props, ref) {
13+
const { value, onChange, children, ...otherProps } = props;
2014
const childrenWithProps = React.Children.map(children, child => {
15+
if (!React.isValidElement(child)) {
16+
return null;
17+
}
2118
const tabProps = {
22-
active: child.props.value === value,
19+
selected: child.props.value === value,
2320
onClick: onChange
2421
};
2522
return React.cloneElement(child, tabProps);
2623
});
2724
return (
28-
<StyledTabs className={className} style={style} {...otherProps}>
25+
<StyledTabs {...otherProps} role='tablist' ref={ref}>
2926
{childrenWithProps}
3027
</StyledTabs>
3128
);
32-
};
29+
});
3330

3431
Tabs.defaultProps = {
3532
value: 0,
3633
onChange: () => {},
37-
children: null,
38-
className: '',
39-
style: {}
34+
children: null
4035
};
4136

4237
Tabs.propTypes = {
4338
value: propTypes.number,
4439
onChange: propTypes.func,
45-
children: propTypes.node,
46-
className: propTypes.string,
47-
style: propTypes.shape([propTypes.string, propTypes.number])
40+
children: propTypes.node
4841
};
4942
export default Tabs;

‎src/components/Tabs/Tabs.spec.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import React from 'react';
2+
import { fireEvent } from '@testing-library/react';
3+
4+
import { renderWithTheme } from '../../../test/utils';
5+
import { Tab } from '..';
6+
import Tabs from './Tabs';
7+
8+
describe('<Tabs />', () => {
9+
describe('prop: children', () => {
10+
it('should accept a null child', () => {
11+
const { getAllByRole } = renderWithTheme(
12+
<Tabs value={0}>
13+
{null}
14+
<Tab />
15+
</Tabs>
16+
);
17+
expect(getAllByRole('tab').length).toBe(1);
18+
});
19+
});
20+
21+
describe('prop: value', () => {
22+
const tabs = (
23+
<Tabs value={1}>
24+
<Tab value={0} />
25+
<Tab value={1} />
26+
</Tabs>
27+
);
28+
29+
it('should pass selected prop to children', () => {
30+
const { getAllByRole } = renderWithTheme(tabs);
31+
const tabElements = getAllByRole('tab');
32+
expect(tabElements[0]).toHaveAttribute('aria-selected', 'false');
33+
expect(tabElements[1]).toHaveAttribute('aria-selected', 'true');
34+
});
35+
36+
it('should accept any value as selected tab value', () => {
37+
const tab0 = {};
38+
const tab1 = {};
39+
expect(tab0).not.toBe(tab1);
40+
41+
const { getAllByRole } = renderWithTheme(
42+
<Tabs value={tab0}>
43+
<Tab value={tab0} />
44+
<Tab value={tab1} />
45+
</Tabs>
46+
);
47+
const tabElements = getAllByRole('tab');
48+
expect(tabElements[0]).toHaveAttribute('aria-selected', 'true');
49+
expect(tabElements[1]).toHaveAttribute('aria-selected', 'false');
50+
});
51+
});
52+
describe('prop: onChange', () => {
53+
it('should call onChange when clicking', () => {
54+
const handleChange = jest.fn();
55+
const { getAllByRole } = renderWithTheme(
56+
<Tabs value={0} onChange={handleChange}>
57+
<Tab value={0} />
58+
<Tab value={1} />
59+
</Tabs>
60+
);
61+
62+
fireEvent.click(getAllByRole('tab')[1]);
63+
expect(handleChange).toBeCalledTimes(1);
64+
expect(handleChange).toHaveBeenCalledWith(1);
65+
});
66+
});
67+
});

0 commit comments

Comments
 (0)