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

[SvgIcon] Migrate to emotion #24506

Merged
merged 7 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
3 changes: 2 additions & 1 deletion docs/pages/api-docs/svg-icon.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
},
"htmlColor": { "type": { "name": "string" } },
"shapeRendering": { "type": { "name": "string" } },
"sx": { "type": { "name": "object" } },
"titleAccess": { "type": { "name": "string" } },
"viewBox": { "type": { "name": "string" }, "default": "'0 0 24 24'" }
},
Expand All @@ -43,5 +44,5 @@
"filename": "/packages/material-ui/src/SvgIcon/SvgIcon.js",
"inheritance": null,
"demos": "<ul><li><a href=\"/components/icons/\">Icons</a></li>\n<li><a href=\"/components/material-icons/\">Material Icons</a></li></ul>",
"styledComponent": false
"styledComponent": true
}
1 change: 1 addition & 0 deletions docs/translations/api-docs/svg-icon/svg-icon.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"fontSize": "The fontSize applied to the icon. Defaults to 24px, but can be configure to inherit font size.",
"htmlColor": "Applies a color attribute to the SVG element.",
"shapeRendering": "The shape-rendering attribute. The behavior of the different options is described on the <a href=\"https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/shape-rendering\">MDN Web Docs</a>. If you are having issues with blurry icons you should investigate this prop.",
"sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the <a href=\"/system/basics/#the-sx-prop\">`sx` page</a> for more details.",
"titleAccess": "Provides a human-readable title for the element that contains it. <a href=\"https://www.w3.org/TR/SVG-access/#Equivalent\">https://www.w3.org/TR/SVG-access/#Equivalent</a>",
"viewBox": "Allows you to redefine what the coordinates without units mean inside an SVG element. For example, if the SVG element is 500 (width) by 200 (height), and you pass viewBox=&quot;0 0 50 20&quot;, this means that the coordinates inside the SVG will go from the top left corner (0,0) to bottom right (50,20) and each unit will be worth 10px."
},
Expand Down
8 changes: 7 additions & 1 deletion packages/material-ui/src/ButtonBase/TouchRipple.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ describe('<TouchRipple />', () => {
mount,
refInstanceof: Object,
muiName: 'MuiTouchRipple',
skip: ['componentsProp', 'refForwarding', 'themeStyleOverrides', 'themeVariants'],
skip: [
'componentProp',
'componentsProp',
'refForwarding',
'themeStyleOverrides',
'themeVariants',
],
}));

describe('prop: center', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/material-ui/src/SvgIcon/SvgIcon.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import * as React from 'react';
import { SxProps } from '@material-ui/system';
import { Theme } from '../styles';
import { OverridableComponent, OverrideProps } from '../OverridableComponent';

export interface SvgIconTypeMap<P = {}, D extends React.ElementType = 'svg'> {
Expand Down Expand Up @@ -51,6 +53,10 @@ export interface SvgIconTypeMap<P = {}, D extends React.ElementType = 'svg'> {
* If you are having issues with blurry icons you should investigate this prop.
*/
shapeRendering?: string;
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx?: SxProps<Theme>;
/**
* Provides a human-readable title for the element that contains it.
* https://www.w3.org/TR/SVG-access/#Equivalent
Expand Down
143 changes: 83 additions & 60 deletions packages/material-ui/src/SvgIcon/SvgIcon.js
Original file line number Diff line number Diff line change
@@ -1,61 +1,75 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
import { unstable_composeClasses as composeClasses } from '@material-ui/unstyled';
import { deepmerge } from '@material-ui/utils';
import capitalize from '../utils/capitalize';
import useThemeProps from '../styles/useThemeProps';
import experimentalStyled from '../styles/experimentalStyled';
import { getSvgIconUtilityClass } from './svgIconClasses';

export const styles = (theme) => ({
/* Styles applied to the root element. */
root: {
userSelect: 'none',
width: '1em',
height: '1em',
display: 'inline-block',
fill: 'currentColor',
flexShrink: 0,
fontSize: theme.typography.pxToRem(24),
transition: theme.transitions.create('fill', {
duration: theme.transitions.duration.shorter,
}),
},
/* Styles applied to the root element if `color="primary"`. */
colorPrimary: {
color: theme.palette.primary.main,
},
/* Styles applied to the root element if `color="secondary"`. */
colorSecondary: {
color: theme.palette.secondary.main,
},
/* Styles applied to the root element if `color="action"`. */
colorAction: {
color: theme.palette.action.active,
},
/* Styles applied to the root element if `color="error"`. */
colorError: {
color: theme.palette.error.main,
},
/* Styles applied to the root element if `color="disabled"`. */
colorDisabled: {
color: theme.palette.action.disabled,
},
/* Styles applied to the root element if `fontSize="inherit"`. */
fontSizeInherit: {
fontSize: 'inherit',
},
/* Styles applied to the root element if `fontSize="small"`. */
fontSizeSmall: {
fontSize: theme.typography.pxToRem(20),
},
/* Styles applied to the root element if `fontSize="large"`. */
fontSizeLarge: {
fontSize: theme.typography.pxToRem(35),
const overridesResolver = (props, styles) => {
const { styleProps } = props;

return deepmerge(styles.root || {}, {
...(styleProps.color !== 'inherit' && styles[`color${capitalize(styleProps.color)}`]),
...styles[`fontSize${capitalize(styleProps.fontSize)}`],
});
};

const useUtilityClasses = (styleProps) => {
const { color, fontSize, classes } = styleProps;

const slots = {
root: [
'root',
color !== 'inherit' && `color${capitalize(color)}`,
`fontSize${capitalize(fontSize)}`,
],
};

return composeClasses(slots, getSvgIconUtilityClass, classes);
};

const SvgIconRoot = experimentalStyled(
'svg',
{},
{
name: 'MuiSvgIcon',
slot: 'Root',
overridesResolver,
},
});
)(({ theme, styleProps }) => ({
/* Styles applied to the root element. */
userSelect: 'none',
width: '1em',
height: '1em',
display: 'inline-block',
fill: 'currentColor',
flexShrink: 0,
transition: theme.transitions.create('fill', {
duration: theme.transitions.duration.shorter,
}),
fontSize: {
inherit: 'inherit',
small: theme.typography.pxToRem(20),
medium: theme.typography.pxToRem(24),
large: theme.typography.pxToRem(35),
}[styleProps.fontSize],
color: {
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
primary: theme.palette.primary.main,
secondary: theme.palette.secondary.main,
action: theme.palette.action.active,
error: theme.palette.error.main,
disabled: theme.palette.action.disabled,
inherit: undefined,
}[styleProps.color],
}));

const SvgIcon = React.forwardRef(function SvgIcon(props, ref) {
const SvgIcon = React.forwardRef(function SvgIcon(inProps, ref) {
const props = useThemeProps({ props: inProps, name: 'MuiSvgIcon' });
const {
children,
classes,
className,
color = 'inherit',
component: Component = 'svg',
Expand All @@ -66,16 +80,21 @@ const SvgIcon = React.forwardRef(function SvgIcon(props, ref) {
...other
} = props;

const styleProps = {
...props,
fontSize,
color,
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
component: Component,
viewBox,
};

const classes = useUtilityClasses(styleProps);

return (
<Component
className={clsx(
classes.root,
{
[classes[`color${capitalize(color)}`]]: color !== 'inherit',
[classes[`fontSize${capitalize(fontSize)}`]]: fontSize !== 'medium',
},
className,
)}
<SvgIconRoot
as={Component}
className={clsx(classes.root, className)}
styleProps={styleProps}
focusable="false"
viewBox={viewBox}
color={htmlColor}
Expand All @@ -86,7 +105,7 @@ const SvgIcon = React.forwardRef(function SvgIcon(props, ref) {
>
{children}
{titleAccess ? <title>{titleAccess}</title> : null}
</Component>
</SvgIconRoot>
);
});

Expand Down Expand Up @@ -133,6 +152,10 @@ SvgIcon.propTypes = {
* If you are having issues with blurry icons you should investigate this prop.
*/
shapeRendering: PropTypes.string,
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx: PropTypes.object,
/**
* Provides a human-readable title for the element that contains it.
* https://www.w3.org/TR/SVG-access/#Equivalent
Expand All @@ -151,4 +174,4 @@ SvgIcon.propTypes = {

SvgIcon.muiName = 'SvgIcon';

export default withStyles(styles, { name: 'MuiSvgIcon' })(SvgIcon);
export default SvgIcon;
9 changes: 5 additions & 4 deletions packages/material-ui/src/SvgIcon/SvgIcon.test.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
import * as React from 'react';
import { expect } from 'chai';
import { getClasses, createMount, describeConformance, createClientRender } from 'test/utils';
import { createMount, describeConformanceV5, createClientRender } from 'test/utils';
import classes from './svgIconClasses';
import SvgIcon from './SvgIcon';

describe('<SvgIcon />', () => {
const mount = createMount();
const render = createClientRender();
let classes;
let path;

before(() => {
classes = getClasses(<SvgIcon>foo</SvgIcon>);
path = <path d="M10 20v-6h4v6h5v-8h3L12 3 2 12h3v8z" data-testid="test-path" />;
});

describeConformance(
describeConformanceV5(
<SvgIcon>
<path />
</SvgIcon>,
() => ({
classes,
inheritComponent: 'svg',
mount,
muiName: 'MuiSvgIcon',
refInstanceof: window.SVGSVGElement,
testComponentPropWith: (props) => (
<svg {...props}>
Expand All @@ -34,6 +34,7 @@ describe('<SvgIcon />', () => {
{props.children}
</svg>
),
skip: ['themeVariants', 'componentsProp'],
}),
);

Expand Down
2 changes: 2 additions & 0 deletions packages/material-ui/src/SvgIcon/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
export { default } from './SvgIcon';
export * from './SvgIcon';
export { default as svgIconClasses } from './svgIconClasses';
export * from './svgIconClasses';
2 changes: 2 additions & 0 deletions packages/material-ui/src/SvgIcon/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { default } from './SvgIcon';
export { default as svgIconClasses } from './svgIconClasses';
export * from './svgIconClasses';
18 changes: 18 additions & 0 deletions packages/material-ui/src/SvgIcon/svgIconClasses.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export interface SvgIconClasses {
root: string;
colorPrimary: string;
colorSecondary: string;
colorAction: string;
colorError: string;
colorDisabled: string;
fontSizeInherit: string;
fontSizeSmall: string;
fontSizeMedium: string;
fontSizeLarge: string;
}

declare const svgIconClasses: SvgIconClasses;

export function getSvgIconUtilityClass(slot: string): string;

export default svgIconClasses;
20 changes: 20 additions & 0 deletions packages/material-ui/src/SvgIcon/svgIconClasses.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { generateUtilityClass, generateUtilityClasses } from '@material-ui/unstyled';

export function getSvgIconUtilityClass(slot) {
return generateUtilityClass('MuiSvgIcon', slot);
}

const svgIconClasses = generateUtilityClasses('MuiSvgIcon', [
'root',
'colorPrimary',
'colorSecondary',
'colorAction',
'colorError',
'colorDisabled',
'fontSizeInherit',
'fontSizeSmall',
'fontSizeMedium',
'fontSizeLarge',
]);

export default svgIconClasses;
2 changes: 1 addition & 1 deletion test/utils/describeConformance.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function testClassName(element, getOptions) {
* @param {React.ReactElement} element
* @param {() => ConformanceOptions} getOptions
*/
function testComponentProp(element, getOptions) {
export function testComponentProp(element, getOptions) {
describe('prop: component', () => {
it('can render another root component with the `component` prop', () => {
const { mount, testComponentPropWith: component = 'em' } = getOptions();
Expand Down
2 changes: 2 additions & 0 deletions test/utils/describeConformanceV5.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as React from 'react';
import { ThemeProvider, createMuiTheme } from '@material-ui/core/styles';
import { createClientRender } from './createClientRender';
import {
testComponentProp,
testClassName,
testPropsSpread,
describeRef,
Expand Down Expand Up @@ -217,6 +218,7 @@ function testThemeVariants(element, getOptions) {
}

const fullSuite = {
componentProp: testComponentProp,
Copy link
Member

Choose a reason for hiding this comment

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

👍 I think I removed this in the first iteration with the Slider, but then we decided we should keep the prop. We may need to disable this next on some of the converted components tough..

Copy link
Member Author

@oliviertassinari oliviertassinari Jan 20, 2021

Choose a reason for hiding this comment

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

It could create a conflict with open PRs, yes.

componentsProp: testComponentsProp,
mergeClassName: testClassName,
propsSpread: testPropsSpread,
Expand Down