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

[Card] Migrate CardMedia to emotion #24625

Merged
merged 5 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions docs/pages/api-docs/card-media.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"classes": { "type": { "name": "object" } },
"component": { "type": { "name": "elementType" } },
"image": { "type": { "name": "string" } },
"src": { "type": { "name": "string" } }
"src": { "type": { "name": "string" } },
"sx": { "type": { "name": "object" } }
},
"name": "CardMedia",
"styles": { "classes": ["root", "media", "img"], "globalClasses": {}, "name": "MuiCardMedia" },
Expand All @@ -13,6 +14,6 @@
"filename": "/packages/material-ui/src/CardMedia/CardMedia.js",
"inheritance": null,
"demos": "<ul><li><a href=\"/components/cards/\">Cards</a></li></ul>",
"styledComponent": false,
"styledComponent": true,
"cssComponent": false
}
3 changes: 2 additions & 1 deletion docs/translations/api-docs/card-media/card-media.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"classes": "Override or extend the styles applied to the component. See <a href=\"#css\">CSS API</a> below for more details.",
"component": "The component used for the root node. Either a string to use a HTML element or a component.",
"image": "Image to be displayed as a background image. Either <code>image</code> or <code>src</code> prop must be specified. Note that caller must specify height otherwise the image will not be visible.",
"src": "An alias for <code>image</code> property. Available only with media components. Media components: <code>video</code>, <code>audio</code>, <code>picture</code>, <code>iframe</code>, <code>img</code>."
"src": "An alias for <code>image</code> property. Available only with media components. Media components: <code>video</code>, <code>audio</code>, <code>picture</code>, <code>iframe</code>, <code>img</code>.",
"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."
},
"classDescriptions": {
"root": { "description": "Styles applied to the root element." },
Expand Down
6 changes: 6 additions & 0 deletions packages/material-ui/src/CardMedia/CardMedia.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { SxProps } from '@material-ui/system';
import * as React from 'react';
import { Theme } from '..';
import { OverridableComponent, OverrideProps } from '../OverridableComponent';

export interface CardMediaTypeMap<P, D extends React.ElementType> {
Expand Down Expand Up @@ -30,6 +32,10 @@ export interface CardMediaTypeMap<P, D extends React.ElementType> {
* Media components: `video`, `audio`, `picture`, `iframe`, `img`.
*/
src?: string;
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx?: SxProps<Theme>;
};
defaultComponent: D;
}
Expand Down
104 changes: 67 additions & 37 deletions packages/material-ui/src/CardMedia/CardMedia.js
Original file line number Diff line number Diff line change
@@ -1,63 +1,89 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { chainPropTypes } from '@material-ui/utils';
import withStyles from '../styles/withStyles';
import { chainPropTypes, deepmerge } from '@material-ui/utils';
import { unstable_composeClasses as composeClasses } from '@material-ui/unstyled';
import useThemeProps from '../styles/useThemeProps';
import experimentalStyled from '../styles/experimentalStyled';
import { getCardMediaUtilityClass } from './cardMediaClasses';

export const styles = {
/* Styles applied to the root element. */
root: {
display: 'block',
backgroundSize: 'cover',
backgroundRepeat: 'no-repeat',
backgroundPosition: 'center',
const overridesResolver = (props, styles) => {
const { styleProps } = props;
const { isMediaComponent, isImageComponent } = styleProps;

return deepmerge(styles.root || {}, {
...(isMediaComponent && styles.media),
...(isImageComponent && styles.img),
});
};

const useUtilityClasses = (styleProps) => {
const { classes, isMediaComponent, isImageComponent } = styleProps;

const slots = {
root: ['root', isMediaComponent && 'media', isImageComponent && 'img'],
};

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

const CardMediaRoot = experimentalStyled(
'div',
{},
{
name: 'MuiCardMedia',
slot: 'Root',
overridesResolver,
},
)(({ styleProps }) => ({
/* Styles applied to the root element. */
display: 'block',
backgroundSize: 'cover',
backgroundRepeat: 'no-repeat',
backgroundPosition: 'center',
/* Styles applied to the root element if `component="video, audio, picture, iframe, or img"`. */
media: {
...(styleProps.isMediaComponent && {
width: '100%',
},
}),
/* Styles applied to the root element if `component="picture or img"`. */
img: {
...(styleProps.isImageComponent && {
// ⚠️ object-fit is not supported by IE11.
objectFit: 'cover',
},
};
}),
}));

const MEDIA_COMPONENTS = ['video', 'audio', 'picture', 'iframe', 'img'];
const IMAGE_COMPONENTS = ['picture', 'img'];

const CardMedia = React.forwardRef(function CardMedia(props, ref) {
const {
children,
classes,
className,
component: Component = 'div',
image,
src,
style,
...other
} = props;
const CardMedia = React.forwardRef(function CardMedia(inProps, ref) {
const props = useThemeProps({ props: inProps, name: 'MuiCardMedia' });
const { children, className, component = 'div', image, src, style, ...other } = props;

const isMediaComponent = MEDIA_COMPONENTS.indexOf(Component) !== -1;
const isMediaComponent = MEDIA_COMPONENTS.indexOf(component) !== -1;
const composedStyle =
!isMediaComponent && image ? { backgroundImage: `url("${image}")`, ...style } : style;

const styleProps = {
...props,
component,
natac13 marked this conversation as resolved.
Show resolved Hide resolved
isMediaComponent,
isImageComponent: IMAGE_COMPONENTS.indexOf(component) !== -1,
};

const classes = useUtilityClasses(styleProps);

return (
<Component
className={clsx(
classes.root,
{
[classes.media]: isMediaComponent,
[classes.img]: ['picture', 'img'].indexOf(Component) !== -1,
},
className,
)}
<CardMediaRoot
className={clsx(classes.root, className)}
as={component}
ref={ref}
style={composedStyle}
styleProps={styleProps}
src={isMediaComponent ? image || src : undefined}
{...other}
>
{children}
</Component>
</CardMediaRoot>
);
});

Expand Down Expand Up @@ -106,6 +132,10 @@ CardMedia.propTypes = {
* @ignore
*/
style: PropTypes.object,
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx: PropTypes.object,
};

export default withStyles(styles, { name: 'MuiCardMedia' })(CardMedia);
export default CardMedia;
14 changes: 7 additions & 7 deletions packages/material-ui/src/CardMedia/CardMedia.test.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import { expect } from 'chai';
import { getClasses, createMount, createClientRender, describeConformance } from 'test/utils';
import { createMount, createClientRender, describeConformanceV5 } from 'test/utils';
import CardMedia from './CardMedia';
import classes from './cardMediaClasses';

describe('<CardMedia />', () => {
const mount = createMount();
let classes;
const render = createClientRender();
before(() => {
classes = getClasses(<CardMedia image="/fake.png" />);
});

describeConformance(<CardMedia image="/fake.png" />, () => ({
describeConformanceV5(<CardMedia image="/fake.png" />, () => ({
classes,
inheritComponent: 'div',
mount,
muiName: 'MuiCardMedia',
refInstanceof: window.HTMLDivElement,
testComponentPropWith: 'span',
testVariantProps: { variant: 'foo' },
skip: ['componentsProp'],
}));

it('should have the backgroundImage specified', () => {
Expand Down Expand Up @@ -79,7 +79,7 @@ describe('<CardMedia />', () => {

it('warns when neither `children`, nor `image`, nor `src`, nor `component` are provided', () => {
expect(() => {
PropTypes.checkPropTypes(CardMedia.Naked.propTypes, { classes: {} }, 'prop', 'MockedName');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was failing with propTypes cannot be read from undefined.
I looked at other Mui component with this test but none of the others have been converted yet, therefore I was not sure if this was the correct approach.
Removing the Naked got the tests passing.

Copy link
Member

Choose a reason for hiding this comment

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

That's the correct solution 👍

Copy link
Member

Choose a reason for hiding this comment

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

A bit of clarification Naked was added by the withStyles utility, we don't use it anymore, so this is correct :)

PropTypes.checkPropTypes(CardMedia.propTypes, { classes: {} }, 'prop', 'MockedName');
}).toErrorDev(
'Material-UI: Either `children`, `image`, `src` or `component` prop must be specified.',
);
Expand Down
11 changes: 11 additions & 0 deletions packages/material-ui/src/CardMedia/cardMediaClasses.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export interface CardMediaClasses {
root: string;
media: string;
img: string;
}

declare const cardMediaClasses: CardMediaClasses;

export function getCardMediaUtilityClass(slot: string): string;

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

export function getCardMediaUtilityClass(slot) {
return generateUtilityClass('MuiCardMedia', slot);
}

const cardMediaClasses = generateUtilityClasses('MuiCardMedia', ['root', 'media', 'img']);

export default cardMediaClasses;
3 changes: 3 additions & 0 deletions packages/material-ui/src/CardMedia/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
export { default } from './CardMedia';
export * from './CardMedia';

export { default as cardMediaClasses } from './cardMediaClasses';
export * from './cardMediaClasses';
3 changes: 3 additions & 0 deletions packages/material-ui/src/CardMedia/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
export { default } from './CardMedia';

export { default as cardMediaClasses } from './cardMediaClasses';
export * from './cardMediaClasses';