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

[Typography] Add system props #24496

Merged
merged 27 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c1e9832
init
mnajdova Jan 19, 2021
fd9ca5b
tests
mnajdova Jan 19, 2021
1c1e658
proptypes, docs:api
mnajdova Jan 19, 2021
be94410
prettier
mnajdova Jan 19, 2021
3fdc9d3
fixed tests, updated mgiration guide
mnajdova Jan 19, 2021
4a984b7
framer
mnajdova Jan 19, 2021
9f39a3f
typings improvements
mnajdova Jan 19, 2021
aa0327c
prettier
mnajdova Jan 19, 2021
ec780e6
ts expected error fixes
mnajdova Jan 19, 2021
132cdeb
lint
mnajdova Jan 19, 2021
c879dd7
Update docs/src/pages/guides/migration-v4/migration-v4.md
mnajdova Jan 19, 2021
d3b9907
Update docs/src/pages/guides/migration-v4/migration-v4.md
mnajdova Jan 19, 2021
5f1bf49
add system props section in the components pages
mnajdova Jan 20, 2021
e125ae6
review comments
mnajdova Jan 20, 2021
983d7a3
improved typings
mnajdova Jan 20, 2021
a3b95e0
ignore color prop
mnajdova Jan 20, 2021
f7fae77
declared as CSS component
mnajdova Jan 21, 2021
5f56256
fix CI
oliviertassinari Jan 27, 2021
659b8ea
reproduce docs/src/pages/guides/composition/ListRouter.tsx demo
oliviertassinari Jan 27, 2021
b415748
we already have visual regression tests coving these cases
oliviertassinari Jan 27, 2021
04358c0
avoid having to rely on browser tests
oliviertassinari Jan 27, 2021
cbe4fd4
no need to skip JSDOM, only for inheritance
oliviertassinari Jan 27, 2021
d1f0013
avoid skipping tests in JSDOM
oliviertassinari Jan 27, 2021
b76d366
test public modules when possible
oliviertassinari Jan 27, 2021
2f4ef9e
avoid skiping tests in JSDOM
oliviertassinari Jan 27, 2021
6fbb847
yarn docs:typescript:formatted
oliviertassinari Jan 27, 2021
a030c46
different solution
oliviertassinari Jan 27, 2021
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
8 changes: 1 addition & 7 deletions docs/pages/api-docs/link.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@
"props": {
"children": { "type": { "name": "node" } },
"classes": { "type": { "name": "object" } },
"color": {
"type": {
"name": "enum",
"description": "'error'<br>&#124;&nbsp;'inherit'<br>&#124;&nbsp;'initial'<br>&#124;&nbsp;'primary'<br>&#124;&nbsp;'secondary'<br>&#124;&nbsp;'textPrimary'<br>&#124;&nbsp;'textSecondary'"
},
"default": "'primary'"
},
"color": { "type": { "name": "any" }, "default": "'primary'" },
"component": { "type": { "name": "custom", "description": "element type" } },
"sx": { "type": { "name": "object" } },
"TypographyClasses": { "type": { "name": "object" } },
Expand Down
26 changes: 2 additions & 24 deletions docs/pages/api-docs/typography.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,7 @@
},
"children": { "type": { "name": "node" } },
"classes": { "type": { "name": "object" } },
"color": {
"type": {
"name": "enum",
"description": "'error'<br>&#124;&nbsp;'inherit'<br>&#124;&nbsp;'initial'<br>&#124;&nbsp;'primary'<br>&#124;&nbsp;'secondary'<br>&#124;&nbsp;'textPrimary'<br>&#124;&nbsp;'textSecondary'"
},
"default": "'initial'"
},
"component": { "type": { "name": "elementType" } },
"display": {
"type": {
"name": "enum",
"description": "'block'<br>&#124;&nbsp;'initial'<br>&#124;&nbsp;'inline'"
},
"default": "'initial'"
},
"gutterBottom": { "type": { "name": "bool" } },
"noWrap": { "type": { "name": "bool" } },
"paragraph": { "type": { "name": "bool" } },
Expand Down Expand Up @@ -64,15 +50,7 @@
"alignJustify",
"noWrap",
"gutterBottom",
"paragraph",
"colorInherit",
"colorPrimary",
"colorSecondary",
"colorTextPrimary",
"colorTextSecondary",
"colorError",
"displayInline",
"displayBlock"
"paragraph"
],
"globalClasses": {},
"name": "MuiTypography"
Expand All @@ -83,5 +61,5 @@
"inheritance": null,
"demos": "<ul><li><a href=\"/components/breadcrumbs/\">Breadcrumbs</a></li>\n<li><a href=\"/components/typography/\">Typography</a></li></ul>",
"styledComponent": true,
"cssComponent": false
"cssComponent": true
}
2 changes: 1 addition & 1 deletion docs/scripts/buildApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ interface DescribeablePropDescriptor {
type: PropTypeDescriptor;
}

const cssComponents = ['Box', 'Grid'];
const cssComponents = ['Box', 'Grid', 'Typography'];

const generateClassName = createGenerateClassName();

Expand Down
4 changes: 4 additions & 0 deletions docs/src/pages/components/box/box.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,7 @@ import Box from '@material-ui/core/Box';
| <span class="prop-name">clone</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the box will recycle its children DOM element. It's using `React.cloneElement` internally. |
| <span class="prop-name">component</span> | <span class="prop-type">union:&nbsp;string&nbsp;&#124;<br>&nbsp;func&nbsp;&#124;<br>&nbsp;object<br></span> | <span class="prop-default">'div'</span> | The component used for the root node. Either a string to use a DOM element or a component. |
| <span class="prop-name">sx</span> | <span class="prop-type">object</span> | <span class="prop-default">{}</span> | Accepts all system properties, as well as any valid CSS properties. |

## System props

As a CSS utility component, the `Box` also supports all [`system`](/system/properties/) properties. You can use them as prop directly on the component.
11 changes: 5 additions & 6 deletions docs/src/pages/components/breadcrumbs/RouterBreadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react';
import { makeStyles, Theme, createStyles } from '@material-ui/core/styles';
import List from '@material-ui/core/List';
import Link, { LinkProps } from '@material-ui/core/Link';
import ListItem from '@material-ui/core/ListItem';
import ListItem, { ListItemProps } from '@material-ui/core/ListItem';
import Collapse from '@material-ui/core/Collapse';
import ListItemText from '@material-ui/core/ListItemText';
import Typography from '@material-ui/core/Typography';
Expand All @@ -12,9 +12,8 @@ import ExpandMore from '@material-ui/icons/ExpandMore';
import Breadcrumbs from '@material-ui/core/Breadcrumbs';
import { Route, MemoryRouter } from 'react-router';
import { Link as RouterLink } from 'react-router-dom';
import { Omit } from '@material-ui/types';

interface ListItemLinkProps extends LinkProps {
interface ListItemLinkProps extends ListItemProps {
to: string;
open?: boolean;
}
Expand All @@ -27,13 +26,13 @@ const breadcrumbNameMap: { [key: string]: string } = {
'/drafts': 'Drafts',
};

function ListItemLink(props: Omit<ListItemLinkProps, 'ref'>) {
const { to, open, ...other } = props;
function ListItemLink(props: ListItemLinkProps) {
const { to, open } = props;
const primary = breadcrumbNameMap[to];

return (
<li>
<ListItem button component={RouterLink} to={to} {...other}>
<ListItem button component={RouterLink} to={to}>
<ListItemText primary={primary} />
{open != null ? open ? <ExpandLess /> : <ExpandMore /> : null}
</ListItem>
Expand Down
4 changes: 4 additions & 0 deletions docs/src/pages/components/typography/typography.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ const theme = createMuiTheme({

In addition to using the default typography variants, you can add custom ones, or disable any you don't need. See the [Adding & disabling variants](/customization/typography/#adding-amp-disabling-variants) example for more info.

## System props

As a CSS utility component, the `Typography` supports all [`system`](/system/properties/) properties. You can use them as prop directly on the component.

## Accessibility

A few key factors to follow for an accessible typography:
Expand Down
22 changes: 22 additions & 0 deletions docs/src/pages/guides/migration-v4/migration-v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -1233,6 +1233,28 @@ As the core components use emotion as a styled engine, the props used by emotion
+<Span>Create a user</Span>
```

- The following `classes` and style overrides keys were removed: "colorInherit", "colorPrimary", "colorSecondary", "colorTextPrimary", "colorTextSecondary", "colorError", "displayInline" and "displayBlock". These props are now considered part of the system, not on the `Typography` component itself. If you still wish to add overrides for them, you can use the `theme.components.MuiTypography.variants` options. For example

```diff
const theme = createMuiTheme({
components: {
MuiTypography: {
- styleOverrides: {
- colorSecondary: {
- marginTop: '20px',
- },
- },
+ variants: {
+ props: { color: "secondary" },
+ style: {
+ marginTop: '20px',
+ },
+ }],
},
},
});
```

### System

- Replace `css` prop with `sx` to avoid collision with styled-components & emotion CSS props.
Expand Down
42 changes: 0 additions & 42 deletions docs/translations/api-docs/typography/typography.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
"align": "Set the text-align on the component.",
"children": "The content of the component.",
"classes": "Override or extend the styles applied to the component. See <a href=\"#css\">CSS API</a> below for more details.",
"color": "The color of the component. It supports those theme colors that make sense for this component.",
"component": "The component used for the root node. Either a string to use a HTML element or a component.",
"display": "Controls the display type",
"gutterBottom": "If <code>true</code>, the text will have a bottom margin.",
"noWrap": "If <code>true</code>, the text will not wrap, but instead will truncate with a text overflow ellipsis.<br>Note that text overflow can only happen with block or inline-block level elements (the element needs to have a width in order to overflow).",
"paragraph": "If <code>true</code>, the text will have a bottom margin.",
Expand Down Expand Up @@ -120,46 +118,6 @@
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>paragraph={true}</code>"
},
"colorInherit": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>color=\"inherit\"</code>"
},
"colorPrimary": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>color=\"primary\"</code>"
},
"colorSecondary": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>color=\"secondary\"</code>"
},
"colorTextPrimary": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>color=\"textPrimary\"</code>"
},
"colorTextSecondary": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>color=\"textSecondary\"</code>"
},
"colorError": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>color=\"error\"</code>"
},
"displayInline": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>display=\"inline\"</code>"
},
"displayBlock": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the root element",
"conditions": "<code>display=\"block\"</code>"
}
}
}
22 changes: 0 additions & 22 deletions framer/Material-UI.framerfx/code/Typography.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@ import MuiTypography from '@material-ui/core/Typography';

interface Props {
align: 'center' | 'inherit' | 'justify' | 'left' | 'right';
color:
| 'error'
| 'inherit'
| 'initial'
| 'primary'
| 'secondary'
| 'textPrimary'
| 'textSecondary';
noWrap: boolean;
label: string;
width: number | string;
Expand All @@ -25,7 +17,6 @@ export function Typography(props: Props): JSX.Element {

Typography.defaultProps = {
align: 'inherit' as 'inherit',
color: 'initial' as 'initial',
noWrap: false,
label: 'Typography',
width: 100,
Expand All @@ -38,19 +29,6 @@ addPropertyControls(Typography, {
title: 'Align',
options: ['center', 'inherit', 'justify', 'left', 'right'],
},
color: {
type: ControlType.Enum,
title: 'Color',
options: [
'error',
'inherit',
'initial',
'primary',
'secondary',
'textPrimary',
'textSecondary',
],
},
noWrap: {
type: ControlType.Boolean,
title: 'No wrap',
Expand Down
49 changes: 34 additions & 15 deletions packages/material-ui-system/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import * as React from 'react';
import * as CSS from 'csstype';
import { CSSProperties } from './CSSProperties';
import {
OverwriteCSSProperties,
AliasesCSSProperties,
StandardCSSProperties,
} from './styleFunctionSx';
// disable automatic export
export {};

Expand Down Expand Up @@ -288,23 +293,37 @@ export interface CSSOthersObjectForCSSObject {
[propertiesName: string]: CSSInterpolation;
}

export type SystemProps = PropsFor<
ComposedStyleFunction<
[
typeof borders,
typeof display,
typeof flexbox,
typeof grid,
typeof palette,
typeof positions,
typeof shadows,
typeof sizing,
typeof spacing,
typeof typography
]
>
export type CustomSystemProps = OverwriteCSSProperties & AliasesCSSProperties;

export type SimpleSystemKeys = keyof Omit<
PropsFor<
ComposedStyleFunction<
[
typeof borders,
typeof display,
typeof flexbox,
typeof grid,
typeof palette,
typeof positions,
typeof shadows,
typeof sizing,
typeof spacing,
typeof typography
]
>
>,
keyof CustomSystemProps
>;

// The SimpleSystemKeys are subset of the StandardCSSProperties, so this should be ok
// This is needed as these are used as keys inside StandardCSSProperties
type StandardSystemKeys = Extract<SimpleSystemKeys, keyof StandardCSSProperties>;

export type SystemProps = {
[K in StandardSystemKeys]?: ResponsiveStyleValue<StandardCSSProperties[K]>;
} &
CustomSystemProps;

export {
default as unstable_styleFunctionSx,
extendSxProp as unstable_extendSxProp,
Expand Down
6 changes: 1 addition & 5 deletions packages/material-ui/src/Box/Box.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,7 @@ describe('<Box />', () => {
});
});

it('combines system properties with the sx prop', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
this.skip();
}

it('combines system properties with the sx prop', () => {
const { container } = render(<Box mt={2} mr={1} sx={{ marginRight: 5, mb: 2 }} />);

expect(container.firstChild).toHaveComputedStyle({
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/CardHeader/CardHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ const CardHeader = React.forwardRef(function CardHeader(inProps, ref) {
<Typography
variant={avatar ? 'body2' : 'body1'}
className={classes.subheader}
color="textSecondary"
color="text.secondary"
component="span"
display="block"
{...subheaderTypographyProps}
Expand Down
9 changes: 3 additions & 6 deletions packages/material-ui/src/CardHeader/CardHeader.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as React from 'react';
import { expect } from 'chai';
import { createMount, createClientRender, describeConformanceV5 } from 'test/utils';
import CardHeader from './CardHeader';
import { typographyClasses } from '../Typography';
import classes from './cardHeaderClasses';
import { typographyClasses } from '@material-ui/core/Typography';
import CardHeader, { cardHeaderClasses as classes } from '@material-ui/core/CardHeader';

describe('<CardHeader />', () => {
const mount = createMount();
Expand Down Expand Up @@ -31,14 +30,13 @@ describe('<CardHeader />', () => {
expect(title).to.have.class(typographyClasses.h5);
});

it('should render the subheader as body1 secondary text', () => {
it('should render the subheader as body1', () => {
const cardHeader = render(<CardHeader title="Title" subheader="Subheader" />).container
.firstChild;
const wrapper = cardHeader.firstChild;
const subheader = wrapper.childNodes[1];
expect(subheader).to.have.class(typographyClasses.root);
expect(subheader).to.have.class(typographyClasses.body1);
expect(subheader).to.have.class(typographyClasses.colorTextSecondary);
});

it('should not render the subheader if none is given', () => {
Expand Down Expand Up @@ -81,7 +79,6 @@ describe('<CardHeader />', () => {
const subHeader = titleWrapper.childNodes[1];
expect(subHeader).to.have.class(typographyClasses.root);
expect(subHeader).to.have.class(typographyClasses.body2);
expect(subHeader).to.have.class(typographyClasses.colorTextSecondary);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ const DialogContentTextTest = () => {
<DialogContentText align="inherit" color="textPrimary" />
<DialogContentText align="inherit" color="textSecondary" />
<DialogContentText align="inherit" color="error" />
{/* @ts-expect-error */}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the system props are a bit too relaxed and cannot catch these errors :\

{/* TODO: system props did not catch this error. Add @ts-expect-error after it is fixed. */}
<DialogContentText display="incorrectValue" />
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
<DialogContentText component="a" href="url" display="block" />
<DialogContentText component="label" htmlFor="html" display="block" />
{/* @ts-expect-error */}
{/* TODO: system props did not catch this error. Add @ts-expect-error after it is fixed. */}
<DialogContentText component="a" href="url" display="incorrectValue" />
{/* @ts-expect-error */}
<DialogContentText component="a" incorrectAttribute="url" />
Expand Down
Loading