Skip to content

Commit

Permalink
sebastian review
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Feb 9, 2020
1 parent 94902e8 commit f267ddb
Show file tree
Hide file tree
Showing 16 changed files with 88 additions and 76 deletions.
4 changes: 2 additions & 2 deletions docs/pages/api/pagination-item.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ You can learn more about the difference by [reading this guide](/guides/minimizi
| <span class="prop-name">selected</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true` the pagination item is selected. |
| <span class="prop-name">shape</span> | <span class="prop-type">'round'<br>&#124;&nbsp;'rounded'</span> | <span class="prop-default">'round'</span> | The shape of the pagination item. |
| <span class="prop-name">size</span> | <span class="prop-type">'small'<br>&#124;&nbsp;'medium'<br>&#124;&nbsp;'large'</span> | <span class="prop-default">'medium'</span> | The size of the pagination item. |
| <span class="prop-name">type</span> | <span class="prop-type">'page'<br>&#124;&nbsp;'first'<br>&#124;&nbsp;'last'<br>&#124;&nbsp;'next'<br>&#124;&nbsp;'previous'<br>&#124;&nbsp;'start-ellipsis'<br>&#124;&nbsp;'end-ellipsis'</span> | <span class="prop-default">'page'</span> | |
| <span class="prop-name">variant</span> | <span class="prop-type">'text'<br>&#124;&nbsp;'outlined'</span> | | |
| <span class="prop-name">type</span> | <span class="prop-type">'page'<br>&#124;&nbsp;'first'<br>&#124;&nbsp;'last'<br>&#124;&nbsp;'next'<br>&#124;&nbsp;'previous'<br>&#124;&nbsp;'start-ellipsis'<br>&#124;&nbsp;'end-ellipsis'</span> | <span class="prop-default">'page'</span> | The type of pagination item. |
| <span class="prop-name">variant</span> | <span class="prop-type">'text'<br>&#124;&nbsp;'outlined'</span> | <span class="prop-default">'text'</span> | The pagination item variant. |

The `ref` is forwarded to the root element.

Expand Down
30 changes: 15 additions & 15 deletions docs/pages/api/pagination.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,25 @@ You can learn more about the difference by [reading this guide](/guides/minimizi

| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| <span class="prop-name">boundaryCount</span> | <span class="prop-type">number</span> | | Number of always visible pages at the beginning and end. |
| <span class="prop-name">boundaryCount</span> | <span class="prop-type">number</span> | <span class="prop-default">1</span> | Number of always visible pages at the beginning and end. |
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | Pagination items. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">color</span> | <span class="prop-type">'default'<br>&#124;&nbsp;'primary'<br>&#124;&nbsp;'secondary'</span> | | The active color. |
| <span class="prop-name">count</span> | <span class="prop-type">number</span> | | The total number of pages. |
| <span class="prop-name">defaultPage</span> | <span class="prop-type">number</span> | | The page selected by default when the component is uncontrolled. |
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | | If `true`, all the pagination component will be disabled. |
| <span class="prop-name">getItemAriaLabel</span> | <span class="prop-type">func</span> | | Accepts a function which returns a string value that provides a user-friendly name for the current page.<br>For localization purposes, you can use the provided [translations](/guides/localization/).<br><br>**Signature:**<br>`function(type?: string, page: number, selected: bool) => string`<br>*type:* The link or button type to format ('page' | 'first' | 'last' | 'next' | 'previous').<br>*page:* The page number to format.<br>*selected:* If true, the current page is selected. |
| <span class="prop-name">hideNextButton</span> | <span class="prop-type">bool</span> | | If `true`, hide the next-page button. |
| <span class="prop-name">hidePrevButton</span> | <span class="prop-type">bool</span> | | If `true`, hide the previous-page button. |
| <span class="prop-name">color</span> | <span class="prop-type">'default'<br>&#124;&nbsp;'primary'<br>&#124;&nbsp;'secondary'</span> | <span class="prop-default">'standard'</span> | The active color. |
| <span class="prop-name">count</span> | <span class="prop-type">number</span> | <span class="prop-default">1</span> | The total number of pages. |
| <span class="prop-name">defaultPage</span> | <span class="prop-type">number</span> | <span class="prop-default">1</span> | The page selected by default when the component is uncontrolled. |
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, all the pagination component will be disabled. |
| <span class="prop-name">getItemAriaLabel</span> | <span class="prop-type">func</span> | <span class="prop-default">function defaultGetAriaLabel(type, page, selected) { if (type === 'page') { return `${selected ? '' : 'Go to '}page ${page}`; } return `Go to ${type} page`;}</span> | Accepts a function which returns a string value that provides a user-friendly name for the current page.<br>For localization purposes, you can use the provided [translations](/guides/localization/).<br><br>**Signature:**<br>`function(type?: string, page: number, selected: bool) => string`<br>*type:* The link or button type to format ('page' | 'first' | 'last' | 'next' | 'previous').<br>*page:* The page number to format.<br>*selected:* If true, the current page is selected. |
| <span class="prop-name">hideNextButton</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, hide the next-page button. |
| <span class="prop-name">hidePrevButton</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, hide the previous-page button. |
| <span class="prop-name">onChange</span> | <span class="prop-type">func</span> | | Callback fired when the page is changed.<br><br>**Signature:**<br>`function(event: object, page: number) => void`<br>*event:* The event source of the callback.<br>*page:* The page selected. |
| <span class="prop-name">page</span> | <span class="prop-type">number</span> | | The current page. |
| <span class="prop-name">renderItem</span> | <span class="prop-type">func</span> | | Render the item.<br><br>**Signature:**<br>`function(params: object) => ReactNode`<br>*params:* null |
| <span class="prop-name">shape</span> | <span class="prop-type">'round'<br>&#124;&nbsp;'rounded'</span> | | The shape of the pagination items. |
| <span class="prop-name">showFirstButton</span> | <span class="prop-type">bool</span> | | If `true`, show the first-page button. |
| <span class="prop-name">showLastButton</span> | <span class="prop-type">bool</span> | | If `true`, show the last-page button. |
| <span class="prop-name">siblingRange</span> | <span class="prop-type">number</span> | | Number of always visible pages before and after the current page. |
| <span class="prop-name">size</span> | <span class="prop-type">'small'<br>&#124;&nbsp;'medium'<br>&#124;&nbsp;'large'</span> | | The size of the pagination component. |
| <span class="prop-name">variant</span> | <span class="prop-type">'text'<br>&#124;&nbsp;'outlined'</span> | | The variant to use. |
| <span class="prop-name">renderItem</span> | <span class="prop-type">func</span> | <span class="prop-default">item => &lt;PaginationItem {...item} /></span> | Render the item.<br><br>**Signature:**<br>`function(params: object) => ReactNode`<br>*params:* null |
| <span class="prop-name">shape</span> | <span class="prop-type">'round'<br>&#124;&nbsp;'rounded'</span> | <span class="prop-default">'round'</span> | The shape of the pagination items. |
| <span class="prop-name">showFirstButton</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, show the first-page button. |
| <span class="prop-name">showLastButton</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, show the last-page button. |
| <span class="prop-name">siblingCount</span> | <span class="prop-type">number</span> | <span class="prop-default">1</span> | Number of always visible pages before and after the current page. |
| <span class="prop-name">size</span> | <span class="prop-type">'small'<br>&#124;&nbsp;'medium'<br>&#124;&nbsp;'large'</span> | <span class="prop-default">'medium'</span> | The size of the pagination component. |
| <span class="prop-name">variant</span> | <span class="prop-type">'text'<br>&#124;&nbsp;'outlined'</span> | <span class="prop-default">'text'</span> | The variant to use. |

The `ref` is forwarded to the root element.

Expand Down
18 changes: 15 additions & 3 deletions packages/material-ui-lab/src/Pagination/Pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,31 @@ function defaultGetAriaLabel(type, page, selected) {
}

const Pagination = React.forwardRef(function Pagination(props, ref) {
/* eslint-disable no-unused-vars */
const {
boundaryCount = 1,
children,
classes,
className,
color = 'standard',
count = 1,
defaultPage = 1,
disabled = false,
getItemAriaLabel: getAriaLabel = defaultGetAriaLabel,
items,
hideNextButton = false,
hidePrevButton = false,
renderItem = item => <PaginationItem {...item} />,
shape = 'round',
showFirstButton = false,
showLastButton = false,
siblingCount = 1,
size = 'medium',
variant = 'text',
...other
} = usePagination({ ...props, componentName: 'Pagination' });
} = props;
/* eslint-enable no-unused-vars */

const { items } = usePagination({ ...props, componentName: 'Pagination' });

return (
<nav
Expand Down Expand Up @@ -153,7 +165,7 @@ Pagination.propTypes = {
/**
* Number of always visible pages before and after the current page.
*/
siblingRange: PropTypes.number,
siblingCount: PropTypes.number,
/**
* The size of the pagination component.
*/
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui-lab/src/Pagination/Pagination.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import Pagination from './Pagination';
describe('<Pagination />', () => {
let classes;
let mount;
const render = createClientRender({ strict: false });
const render = createClientRender();

before(() => {
mount = createMount();
mount = createMount({ strict: true });
classes = getClasses(<Pagination />);
});

Expand Down
6 changes: 3 additions & 3 deletions packages/material-ui-lab/src/PaginationItem/PaginationItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ const PaginationItem = React.forwardRef(function PaginationItem(props, ref) {
shape = 'round',
size = 'medium',
type = 'page',
variant,
variant = 'text',
...other
} = props;

Expand Down Expand Up @@ -286,7 +286,7 @@ PaginationItem.propTypes = {
* The size of the pagination item.
*/
size: PropTypes.oneOf(['small', 'medium', 'large']),
/*
/**
* The type of pagination item.
*/
type: PropTypes.oneOf([
Expand All @@ -298,7 +298,7 @@ PaginationItem.propTypes = {
'start-ellipsis',
'end-ellipsis',
]),
/*
/**
* The pagination item variant.
*/
variant: PropTypes.oneOf(['text', 'outlined']),
Expand Down
20 changes: 10 additions & 10 deletions packages/material-ui-lab/src/PaginationItem/PaginationItem.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import PaginationItem from './PaginationItem';
describe('<PaginationItem />', () => {
let classes;
let mount;
const render = createClientRender({ strict: false });
const render = createClientRender();

before(() => {
mount = createMount();
mount = createMount({ strict: true });
classes = getClasses(<PaginationItem />);
});

Expand Down Expand Up @@ -55,10 +55,10 @@ describe('<PaginationItem />', () => {
</PaginationItem>,
);

const rootNode = getByTestId('root');
expect(rootNode).to.have.class(classes.root);
expect(rootNode).to.have.class(classes.sizeSmall);
expect(rootNode).not.to.have.class(classes.sizeLarge);
const root = getByTestId('root');
expect(root).to.have.class(classes.root);
expect(root).to.have.class(classes.sizeSmall);
expect(root).not.to.have.class(classes.sizeLarge);
});

it('should render a large button', () => {
Expand All @@ -68,9 +68,9 @@ describe('<PaginationItem />', () => {
</PaginationItem>,
);

const rootNode = getByTestId('root');
expect(rootNode).to.have.class(classes.root);
expect(rootNode).not.to.have.class(classes.sizeSmall);
expect(rootNode).to.have.class(classes.sizeLarge);
const root = getByTestId('root');
expect(root).to.have.class(classes.root);
expect(root).not.to.have.class(classes.sizeSmall);
expect(root).to.have.class(classes.sizeLarge);
});
});
2 changes: 1 addition & 1 deletion packages/material-ui-lab/src/Rating/Rating.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const styles = theme => ({
cursor: 'pointer',
WebkitTapHighlightColor: 'transparent',
'&$disabled': {
opacity: theme.palette.action.disabledOpacity,
opacity: 0.5,
pointerEvents: 'none',
},
'&$focusVisible $iconActive': {
Expand Down
16 changes: 8 additions & 8 deletions packages/material-ui-lab/src/ToggleButton/ToggleButton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ describe('<ToggleButton />', () => {
</ToggleButton>,
);

const rootNode = getByTestId('root');
expect(rootNode).to.have.class(classes.root);
expect(rootNode).to.have.class(classes.sizeSmall);
expect(rootNode).not.to.have.class(classes.sizeLarge);
const root = getByTestId('root');
expect(root).to.have.class(classes.root);
expect(root).to.have.class(classes.sizeSmall);
expect(root).not.to.have.class(classes.sizeLarge);
});

it('should render a large button', () => {
Expand All @@ -67,10 +67,10 @@ describe('<ToggleButton />', () => {
</ToggleButton>,
);

const rootNode = getByTestId('root');
expect(rootNode).to.have.class(classes.root);
expect(rootNode).not.to.have.class(classes.sizeSmall);
expect(rootNode).to.have.class(classes.sizeLarge);
const root = getByTestId('root');
expect(root).to.have.class(classes.root);
expect(root).not.to.have.class(classes.sizeSmall);
expect(root).to.have.class(classes.sizeLarge);
});

describe('prop: onChange', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Chip/Chip.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const styles = theme => {
verticalAlign: 'middle',
boxSizing: 'border-box',
'&$disabled': {
opacity: theme.palette.action.disabledOpacity,
opacity: 0.5,
pointerEvents: 'none',
},
'& $avatar': {
Expand Down
8 changes: 4 additions & 4 deletions packages/material-ui/src/FilledInput/FilledInput.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ describe('<FilledInput />', () => {

it('should have the underline class', () => {
const { container } = render(<FilledInput />);
const rootNode = container.firstChild;
expect(rootNode).to.have.class(classes.underline);
const root = container.firstChild;
expect(root).to.have.class(classes.underline);
});

it('can disable the underline', () => {
const { container } = render(<FilledInput disableUnderline />);
const rootNode = container.firstChild;
expect(rootNode).not.to.have.class(classes.underline);
const root = container.firstChild;
expect(root).not.to.have.class(classes.underline);
});
});
18 changes: 9 additions & 9 deletions packages/material-ui/src/FormControl/FormControl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,26 @@ describe('<FormControl />', () => {
describe('initial state', () => {
it('should have no margin', () => {
const { container } = render(<FormControl />);
const rootNode = container.firstChild;
const root = container.firstChild;

expect(rootNode).not.to.have.class(classes.marginNormal);
expect(rootNode).not.to.have.class(classes.marginDense);
expect(root).not.to.have.class(classes.marginNormal);
expect(root).not.to.have.class(classes.marginDense);
});

it('can have the margin normal class', () => {
const { container } = render(<FormControl margin="normal" />);
const rootNode = container.firstChild;
const root = container.firstChild;

expect(rootNode).to.have.class(classes.marginNormal);
expect(rootNode).not.to.have.class(classes.marginDense);
expect(root).to.have.class(classes.marginNormal);
expect(root).not.to.have.class(classes.marginDense);
});

it('can have the margin dense class', () => {
const { container } = render(<FormControl margin="dense" />);
const rootNode = container.firstChild;
const root = container.firstChild;

expect(rootNode).to.have.class(classes.marginDense);
expect(rootNode).not.to.have.class(classes.marginNormal);
expect(root).to.have.class(classes.marginDense);
expect(root).not.to.have.class(classes.marginNormal);
});

it('should not be filled initially', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ describe('<FormControlLabel />', () => {

it('should render the label text inside an additional element', () => {
const { container, getByText } = render(<FormControlLabel label="Pizza" control={<div />} />);
const rootNode = container.firstChild;
const root = container.firstChild;

expect(rootNode).to.have.property('nodeName', 'LABEL');
expect(rootNode).to.have.class(classes.root);
expect(root).to.have.property('nodeName', 'LABEL');
expect(root).to.have.class(classes.root);
expect(getByText(/Pizza/)).not.to.have.class(classes.root);
expect(getByText(/Pizza/)).to.have.class(classes.label);
});
Expand All @@ -41,11 +41,11 @@ describe('<FormControlLabel />', () => {
const { container, getByTestId, getByText } = render(
<FormControlLabel label="Pizza" disabled control={<div data-testid="control" />} />,
);
const rootNode = container.firstChild;
const root = container.firstChild;
const control = getByTestId('control');
const label = getByText(/Pizza/);

expect(rootNode).to.have.class(classes.disabled);
expect(root).to.have.class(classes.disabled);
expect(control).to.have.attribute('disabled');
expect(label).to.have.class(classes.disabled);
});
Expand All @@ -58,11 +58,11 @@ describe('<FormControlLabel />', () => {
control={<div data-testid="control" disabled />}
/>,
);
const rootNode = container.firstChild;
const root = container.firstChild;
const control = getByTestId('control');
const label = getByText(/Pizza/);

expect(rootNode).to.have.class(classes.disabled);
expect(root).to.have.class(classes.disabled);
expect(control).to.have.attribute('disabled');
expect(label).to.have.class(classes.disabled);
});
Expand Down
14 changes: 7 additions & 7 deletions packages/material-ui/src/IconButton/IconButton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ describe('<IconButton />', () => {

describe('prop: size', () => {
it('should render the right class', () => {
let rootNode;
rootNode = render(<IconButton size="small">book</IconButton>).container.firstChild;
expect(rootNode).to.have.class(classes.sizeSmall);
let root;
root = render(<IconButton size="small">book</IconButton>).container.firstChild;
expect(root).to.have.class(classes.sizeSmall);

rootNode = render(<IconButton size="medium">book</IconButton>).container.firstChild;
expect(rootNode).not.to.have.class(classes.sizeSmall);
root = render(<IconButton size="medium">book</IconButton>).container.firstChild;
expect(root).not.to.have.class(classes.sizeSmall);

rootNode = render(<IconButton>book</IconButton>).container.firstChild;
expect(rootNode).not.to.have.class(classes.sizeSmall);
root = render(<IconButton>book</IconButton>).container.firstChild;
expect(root).not.to.have.class(classes.sizeSmall);
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/ListItem/ListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const styles = theme => ({
backgroundColor: theme.palette.action.selected,
},
'&$disabled': {
opacity: theme.palette.action.disabledOpacity,
opacity: 0.5,
},
},
/* Styles applied to the `container` element if `children` includes `ListItemSecondaryAction`. */
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Tab/Tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const styles = theme => ({
opacity: 1,
},
'&$disabled': {
opacity: theme.palette.action.disabledOpacity,
opacity: 0.5,
},
},
/* Styles applied to the root element if the parent [`Tabs`](/api/tabs/) has `textColor="primary"`. */
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/styles/createPalette.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const light = {
selected: 'rgba(0, 0, 0, 0.08)',
selectedOpacity: 0.08,
// The color of a disabled action.
disabled: 'rgba(0, 0, 0, 0.38)',
disabled: 'rgba(0, 0, 0, 0.26)',
// The background color of a disabled action.
disabledBackground: 'rgba(0, 0, 0, 0.12)',
disabledOpacity: 0.38,
Expand Down Expand Up @@ -69,7 +69,7 @@ export const dark = {
hoverOpacity: 0.08,
selected: 'rgba(255, 255, 255, 0.16)',
selectedOpacity: 0.16,
disabled: 'rgba(255, 255, 255, 0.38)',
disabled: 'rgba(255, 255, 255, 0.3)',
disabledBackground: 'rgba(255, 255, 255, 0.12)',
disabledOpacity: 0.38,
focus: 'rgba(255, 255, 255, 0.12)',
Expand Down

0 comments on commit f267ddb

Please sign in to comment.