[CardMedia] objectFit: cover is not appropriate for iframe

[nareshbhatia]

When CardMedia is provided with a component prop of 'video', 'audio', 'picture', 'iframe', or 'img', Material UI automatically adds the following styles to the component:

  /* Styles applied to the root element if `component="video, audio, picture, iframe, or img"`. */
  media: {
    width: '100%',
    objectFit: 'cover',
  },

This is good for image components, but not good for embedded iframes with video. Specifically, on iOS, the video no longer stretches from edge to edge, but only to about 80% of the width, the remaining 20% is blank.

The standard styling for an iframe to show a 16:9 aspect ratio video is shown below:

    root: {
        position: 'relative',
        paddingBottom: '56.25%', // 16:9 aspect ratio
        paddingTop: 25,
        height: 0
    },
    frame: {
        position: 'absolute',
        top: 0,
        left: 0,
        width: '100%',
        height: '100%',
    }

Note that width is already set to 100% and there should be no objectFit style applied.

My suggestion is to remove the addition of width and objectFit styles in case of iframe CardMedia components.

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

Video should stretch edge to edge on all browsers including iOS.

Current Behavior 😯

On iOS, video only stretches to about 80% of the width.

Steps to Reproduce 🕹

See this working demo on iOS (Chorme and/or Safari). You should see a blank area on the right.

https://pxjrk.csb.dev/

Here's the source code for that demo:

https://codesandbox.io/s/cardmedia-ios-issue-pxjrk

Your Environment 🌎

Tech	Version
Material-UI	v4.1.3
React	16.8.6
Browser	iOS Chrome or Safari
TypeScript	3.4.5
etc.

[oliviertassinari]

@nareshbhatia Interesting, regarding the width prop, it seems to help. What's the harm? I can reproduce the object-fit issue. I'm wondering, what's the behavior when providing a video or audio element?

Here is one possible approach:

diff --git a/packages/material-ui/src/CardMedia/CardMedia.js b/packages/material-ui/src/CardMedia/CardMedia.js
index cc0e24ca7..93e7d1206 100644
--- a/packages/material-ui/src/CardMedia/CardMedia.js
+++ b/packages/material-ui/src/CardMedia/CardMedia.js
@@ -18,6 +18,9 @@ export const styles = {
     // ⚠️ object-fit is not supported by IE 11.
     objectFit: 'cover',
   },
+  iframe: {
+    objectFit: 'fill',
+  },
 };

 const MEDIA_COMPONENTS = ['video', 'audio', 'picture', 'iframe', 'img'];
@@ -40,6 +43,7 @@ const CardMedia = React.forwardRef(function CardMedia(props, ref) {
         classes.root,
         {
           [classes.media]: isMediaComponent,
+          [classes.iframe]: Component === 'iframe',
         },
         className,
       )}

Another one:

diff --git a/packages/material-ui/src/CardMedia/CardMedia.js b/packages/material-ui/src/CardMedia/CardMedia.js
index cc0e24ca7..d287f7ea1 100644
--- a/packages/material-ui/src/CardMedia/CardMedia.js
+++ b/packages/material-ui/src/CardMedia/CardMedia.js
@@ -15,6 +15,8 @@ export const styles = {
   /* Styles applied to the root element if `component="video, audio, picture, iframe, or img"`. */
   media: {
     width: '100%',
+  },
+  img: {
     // ⚠️ object-fit is not supported by IE 11.
     objectFit: 'cover',
   },
@@ -40,6 +42,7 @@ const CardMedia = React.forwardRef(function CardMedia(props, ref) {
         classes.root,
         {
           [classes.media]: isMediaComponent,
+          [classes.img]: ['picture', 'img'].indexOf(Component),
         },
         className,
       )}

Do you have a specific implementation in mind?

[nareshbhatia]

@oliviertassinari no harm in adding width: 100%, it's just that for getting the standard 16:9 aspect ratio, I would have to supply the additional styles to the frame anyway:

frame: {
        position: 'absolute',
        top: 0,
        left: 0,
        width: '100%',
        height: '100%',
    }

The above styles happen to be a defacto standard for the getting videos in iframes (google for it and you will get multiple hits with the same answer), so I wouldn't want half the styles in CardMedia and other half in application code. TL;DR - no harm in setting width: 100% in CardMedia.

As for objectFit: fill, I have mixed feelings. Give the standard set of styles above, there is no need for objectFit at all, so why add an unnecessary style. I can't tell in which use case it may fail.

The reason I am leaning towards CardMedia not adding anything additional for iframe is that the current approach of adding these styles automatically came as a total surprise. It took quite a lot of time to debug (literally had to download the iOS simulator to see which style was causing the issue).

If I were to choose between your proposed approaches, I would go with the first. But I would still prefer the approach where CardMedia adds nothing for iframe and lets the app decide what it wants - it gives better control with no surprises.

P.S. I have not played with video and audio elements.

[oliviertassinari]

If I were to choose between your proposed approaches, I would go with the first.

My preferred approach so far is the last one. I like how it doesn't introduce any breaking change, and how it only applies the necessary style properties.
What's wrong with the introduction of a img style rule?

[nareshbhatia]

I am good with the second approach too - introduction of an img rule. We just need to document what each component type is doing to the styling.

[oliviertassinari]

OK great, unless somebody has a better proposal, we can go for it. My concern with the initial proposal is the breaking change impact potential. Maybe I'm wrong, it's not a BC?

[neon98]

@oliviertassinari I'd like to work on this.

[joshwooding]

@neon98 I know you asked Olivier but it would be great if you worked on it :) Let us know if you need any help.

[neon98]

hey @joshwooding! I've submitted the PR with suggested changes. Feel free to look into it.