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.

Sign up for GitHub

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

[material-ui][SvgIcon] Convert to support CSS extraction #41779

Merged
merged 8 commits into from
Apr 25, 2024
Merged

[material-ui][SvgIcon] Convert to support CSS extraction #41779

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0fc6c38
Migrate SvgIcon to Pigment CSS
aarongarciah Apr 5, 2024
cac0b4e
Generate Pigment CSS demos
aarongarciah Apr 5, 2024
b060d54
Circumvent compile error using inline styles
aarongarciah Apr 8, 2024
09d13a2
Generate TS demo preview
aarongarciah Apr 8, 2024
469ef99
Undo inline styles changes
aarongarciah Apr 9, 2024
4e66d18
Check for theme vars
aarongarciah Apr 9, 2024
d207897
remove TwoTone from demo
siriwatknp Apr 25, 2024
8a3fa51
Merge branch 'next' of https://github.com/mui/material-ui into aarong…
siriwatknp Apr 25, 2024
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
79 changes: 79 additions & 0 deletions apps/pigment-css-next-app/src/app/material-ui/icons/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
'use client';
import * as React from 'react';
import CreateSvgIcon from '../../../../../../docs/data/material/components/icons/CreateSvgIcon';
import FontAwesomeIcon from '../../../../../../docs/data/material/components/icons/FontAwesomeIcon';
import FontAwesomeIconSize from '../../../../../../docs/data/material/components/icons/FontAwesomeIconSize';
import FontAwesomeSvgIconDemo from '../../../../../../docs/data/material/components/icons/FontAwesomeSvgIconDemo';
import Icons from '../../../../../../docs/data/material/components/icons/Icons';
import SvgIconChildren from '../../../../../../docs/data/material/components/icons/SvgIconChildren';
import SvgIconsColor from '../../../../../../docs/data/material/components/icons/SvgIconsColor';
import SvgIconsSize from '../../../../../../docs/data/material/components/icons/SvgIconsSize';
import SvgMaterialIcons from '../../../../../../docs/data/material/components/icons/SvgMaterialIcons';
import TwoToneIcons from '../../../../../../docs/data/material/components/icons/TwoToneIcons';

export default function IconsPage() {
aarongarciah marked this conversation as resolved.
Show resolved Hide resolved
return (
<React.Fragment>
<section>
<h2> Create Svg Icon</h2>
<div className="demo-container">
<CreateSvgIcon />
</div>
</section>
<section>
<h2> Font Awesome Icon</h2>
<div className="demo-container">
<FontAwesomeIcon />
</div>
</section>
<section>
<h2> Font Awesome Icon Size</h2>
<div className="demo-container">
<FontAwesomeIconSize />
</div>
</section>
<section>
<h2> Font Awesome Svg Icon Demo</h2>
<div className="demo-container">
<FontAwesomeSvgIconDemo />
</div>
</section>
<section>
<h2> Icons</h2>
<div className="demo-container">
<Icons />
</div>
</section>
<section>
<h2> Svg Icon Children</h2>
<div className="demo-container">
<SvgIconChildren />
</div>
</section>
<section>
<h2> Svg Icons Color</h2>
<div className="demo-container">
<SvgIconsColor />
</div>
</section>
<section>
<h2> Svg Icons Size</h2>
<div className="demo-container">
<SvgIconsSize />
</div>
</section>
<section>
<h2> Svg Material Icons</h2>
<div className="demo-container">
<SvgMaterialIcons />
</div>
</section>
<section>
<h2> Two Tone Icons</h2>
<div className="demo-container">
<TwoToneIcons />
</div>
</section>
</React.Fragment>
);
}
80 changes: 80 additions & 0 deletions apps/pigment-css-vite-app/src/pages/material-ui/icons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import * as React from 'react';
import MaterialUILayout from '../../Layout';
import CreateSvgIcon from '../../../../../docs/data/material/components/icons/CreateSvgIcon.tsx';
import FontAwesomeIcon from '../../../../../docs/data/material/components/icons/FontAwesomeIcon.tsx';
import FontAwesomeIconSize from '../../../../../docs/data/material/components/icons/FontAwesomeIconSize.tsx';
import FontAwesomeSvgIconDemo from '../../../../../docs/data/material/components/icons/FontAwesomeSvgIconDemo.tsx';
import Icons from '../../../../../docs/data/material/components/icons/Icons.tsx';
import SvgIconChildren from '../../../../../docs/data/material/components/icons/SvgIconChildren.tsx';
import SvgIconsColor from '../../../../../docs/data/material/components/icons/SvgIconsColor.tsx';
import SvgIconsSize from '../../../../../docs/data/material/components/icons/SvgIconsSize.tsx';
import SvgMaterialIcons from '../../../../../docs/data/material/components/icons/SvgMaterialIcons.tsx';
import TwoToneIcons from '../../../../../docs/data/material/components/icons/TwoToneIcons.tsx';

export default function IconsPage() {
aarongarciah marked this conversation as resolved.
Show resolved Hide resolved
return (
<MaterialUILayout>
<h1>Icons</h1>
<section>
<h2> Create Svg Icon</h2>
<div className="demo-container">
<CreateSvgIcon />
</div>
</section>
<section>
<h2> Font Awesome Icon</h2>
<div className="demo-container">
<FontAwesomeIcon />
</div>
</section>
<section>
<h2> Font Awesome Icon Size</h2>
<div className="demo-container">
<FontAwesomeIconSize />
</div>
</section>
<section>
<h2> Font Awesome Svg Icon Demo</h2>
<div className="demo-container">
<FontAwesomeSvgIconDemo />
</div>
</section>
<section>
<h2> Icons</h2>
<div className="demo-container">
<Icons />
</div>
</section>
<section>
<h2> Svg Icon Children</h2>
<div className="demo-container">
<SvgIconChildren />
</div>
</section>
<section>
<h2> Svg Icons Color</h2>
<div className="demo-container">
<SvgIconsColor />
</div>
</section>
<section>
<h2> Svg Icons Size</h2>
<div className="demo-container">
<SvgIconsSize />
</div>
</section>
<section>
<h2> Svg Material Icons</h2>
<div className="demo-container">
<SvgMaterialIcons />
</div>
</section>
<section>
<h2> Two Tone Icons</h2>
<div className="demo-container">
<TwoToneIcons />
</div>
</section>
</MaterialUILayout>
);
}
2 changes: 1 addition & 1 deletion docs/data/material/components/icons/TwoToneIcons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export default function TwoToneIcons() {

return (
<Icon
sx={{ ...(isDarkMode && { filter: 'invert(1)' }) }}
baseClassName="material-icons-two-tone"
style={isDarkMode ? { filter: 'invert(1)' } : undefined}
Copy link
Member

@siriwatknp siriwatknp Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think we should change this. We should create a fix from Pigment CSS first and then apply it to this PR. It's okay to have this PR open and label it as on hold because it is blocked by another issue.

If you don't mind @aarongarciah, let's wait for a fix from @brijeshb42 and then apply to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, let's wait.

Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted this change.

Copy link
Contributor

@brijeshb42 brijeshb42 Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TBH, the fix will take some time. It's not just about the sx transform here but also about the support for transformed sx prop to be passed to the Icon that Marija is working on.
Even right now, one particular transform is supported, that is, sx={isDarkMode ? {} : undefined}. This would work as far as transformation is concerned. But we'll still need to support the sx prop in underlying component.
If there is an alternate, we should do that and not wait for a proper fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: moving from sx to style changes the specificity.

Copy link
Contributor

Choose a reason for hiding this comment

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

True. But it's a component in our docs, not a generic component or library component. So it should be fine since it doesn't affect end-users.

>
add_circle
</Icon>
Expand Down
2 changes: 1 addition & 1 deletion docs/data/material/components/icons/TwoToneIcons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export default function TwoToneIcons() {

return (
<Icon
sx={{ ...(isDarkMode && { filter: 'invert(1)' }) }}
baseClassName="material-icons-two-tone"
style={isDarkMode ? { filter: 'invert(1)' } : undefined}
>
add_circle
</Icon>
Expand Down
2 changes: 1 addition & 1 deletion docs/data/material/components/icons/TwoToneIcons.tsx.preview
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Icon
sx={{ ...(isDarkMode && { filter: 'invert(1)' }) }}
baseClassName="material-icons-two-tone"
style={isDarkMode ? { filter: 'invert(1)' } : undefined}
>
add_circle
</Icon>
71 changes: 51 additions & 20 deletions packages/mui-material/src/SvgIcon/SvgIcon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import PropTypes from 'prop-types';
import clsx from 'clsx';
import composeClasses from '@mui/utils/composeClasses';
import capitalize from '../utils/capitalize';
import useThemeProps from '../styles/useThemeProps';
import styled from '../styles/styled';
import { styled, createUseThemeProps } from '../zero-styled';
import { getSvgIconUtilityClass } from './svgIconClasses';

const useThemeProps = createUseThemeProps('MuiSvgIcon');

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

Expand All @@ -34,32 +35,60 @@ const SvgIconRoot = styled('svg', {
styles[`fontSize${capitalize(ownerState.fontSize)}`],
];
},
})(({ theme, ownerState }) => ({
})(({ theme }) => ({
userSelect: 'none',
width: '1em',
height: '1em',
display: 'inline-block',
// the <svg> will define the property that has `currentColor`
// for example heroicons uses fill="none" and stroke="currentColor"
fill: ownerState.hasSvgAsChild ? undefined : 'currentColor',
flexShrink: 0,
transition: theme.transitions?.create?.('fill', {
duration: theme.transitions?.duration?.shorter,
}),
fontSize: {
inherit: 'inherit',
small: theme.typography?.pxToRem?.(20) || '1.25rem',
medium: theme.typography?.pxToRem?.(24) || '1.5rem',
large: theme.typography?.pxToRem?.(35) || '2.1875rem',
}[ownerState.fontSize],
// TODO v5 deprecate, v6 remove for sx
color:
(theme.vars || theme).palette?.[ownerState.color]?.main ??
variants: [
{
props: (props) => !props.hasSvgAsChild,
style: {
// the <svg> will define the property that has `currentColor`
// for example heroicons uses fill="none" and stroke="currentColor"
fill: 'currentColor',
},
},
{
props: { fontSize: 'inherit' },
style: { fontSize: 'inherit' },
},
{
props: { fontSize: 'small' },
style: { fontSize: theme.typography?.pxToRem?.(20) || '1.25rem' },
},
{
props: { fontSize: 'medium' },
style: { fontSize: theme.typography?.pxToRem?.(24) || '1.5rem' },
},
{
props: { fontSize: 'large' },
style: { fontSize: theme.typography?.pxToRem?.(35) || '2.1875rem' },
},
// TODO v5 deprecate color prop, v6 remove for sx
aarongarciah marked this conversation as resolved.
Show resolved Hide resolved
...Object.entries(theme.palette)
.filter(([, value]) => value.main || value.action || value.disabled)
.map(([color]) => ({
props: { color },
style: { color: theme.palette?.[color]?.main },
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
style: { color: theme.palette?.[color]?.main },
style: { color: (theme.vars ?? theme).palette?.[color]?.main },

x the other usages.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated (except function calls like theme.typography?.pxToRem() or theme.transitions?.create() which don't exist in the vars object).

})),
{
props: { color: 'action' },
style: { color: theme.palette?.action?.active },
},
{
props: { color: 'disabled' },
style: { color: theme.palette?.action?.disabled },
},
{
action: (theme.vars || theme).palette?.action?.active,
disabled: (theme.vars || theme).palette?.action?.disabled,
inherit: undefined,
}[ownerState.color],
props: { color: 'inherit' },
style: { color: undefined },
},
],
}));

const SvgIcon = React.forwardRef(function SvgIcon(inProps, ref) {
Expand Down Expand Up @@ -211,6 +240,8 @@ SvgIcon.propTypes /* remove-proptypes */ = {
viewBox: PropTypes.string,
};

SvgIcon.muiName = 'SvgIcon';
if (SvgIcon) {
aarongarciah marked this conversation as resolved.
Show resolved Hide resolved
SvgIcon.muiName = 'SvgIcon';
}

export default SvgIcon;
Loading