Allow for 4:3 Video embeds

[jeffposnick]

@argyleink has requested a clean way to use the Video shortcode to embed with a 4:3 aspect ratio, instead of 16:9.

He's currently resorting to the following hack:

  <style style="display: none">
    .adjusted-aspect-ratio {
      aspect-ratio: 4/3;
    }
  </style>
  {% Video
    src="video/vS06HQ1YTsbMKSFTIPl2iogUQP73/rx8k6w5iQnEOwqOoUNNG.mp4",
    autoplay="true",
    loop="true",
    muted="true",
    class="adjusted-aspect-ratio"
  %}

[argyleink]

a shortcode option seems right, something like?

  {% Video
    src="video/robo-vomit.mp4",
    autoplay="true",
    loop="true",
    muted="true",
    ratio="4/3"
  %}

[jeffposnick]

It's worth noting that the style for the <video> on web.dev comes from here:

/// Video related elements
video,
.youtube {
  position: relative;
  aspect-ratio: 16/9;
}

Maybe what would ultimately make sense is to keep the Video shortcode as-is, and just add

.ratio43 {
  aspect-ratio: 4/3;
}

as a class that anyone could use via class="ratio43", formalizing what @argyleink is already doing?

What's your opinion, @devnook? I'm not as familiar with the way our component/styling system is supposed to work.

[jakearchibald]

Proposal: change https://github.com/GoogleChrome/webdev-infra/blob/main/shortcodes/Video.js so if both "width" and "height" are provided, it generates an inline style of style="aspect-ratio: ${width} / ${height}".

The default <video> styles will also need height: auto, but this makes auto aspect-ratio behave more like it does with <img>.

This fixes the issue for 4:3, but also all other ratios.

[jeffposnick]

That seems reasonable to me!

[jakearchibald]

It might be safer to do style="--vid-width: ${width}; --vid-height: ${height}", then have a style rule like:

video {
  width: 100%;
  height: auto;
  aspect-ratio: var(--vid-width) / var(--vid-height);
}

Then it has lower specificity than an inline style.

[jakearchibald]

• https://github.com/GoogleChrome/webdev-infra/pull/32
• https://github.com/GoogleChrome/web.dev/pull/8399
• https://github.com/GoogleChrome/developer.chrome.com/pull/3295

[tunetheweb]

@jakearchibald would you add a default --var-width: 16; --var-height: 9; when you add that new CSS rule, so that it defaults to the current 16/9 rather than 0/0 if width and height are not passed to the shortcode?

[tunetheweb]

OK so this is what I think it needs, added to next.scss

Add --var-width, --var-height, and aspect-ratio:

/// Video related elements
video,
.youtube {
  --var-width: 16;
  --var-height: 9;
  position: relative;
  aspect-ratio: var(--vid-width) / var(--vid-height);
}

Add height: auto to video further down (probably this should just be merged with above)?

/// Media
video {
  max-width: 100%;
  height: auto;
}

[jakearchibald]

I'm wary of changing things for existing content

[tunetheweb]

Not sure what you mean?

The current CSS for web.dev has 16/9:

https://github.com/GoogleChrome/web.dev/blob/076e60738ad7c89c2f733db48c7c7c7221100784/src/scss/next.scss#L699

/// Video related elements
video,
.youtube {
  position: relative;
  aspect-ratio: 16/9;
}

So if you're planning on changing that to aspect-ratio: var(--vid-width) / var(--vid-height); then you will break that for any existing content, that is not currently supplying a width and height (basically all of them right now). And if you don't add that, then you've not solved this. So I think you need to do the first change I suggested (/// Video related elements changes above) don't you?

Also because that same stylesheet has this:

/// Media
video {
  max-width: 100%;
}

As soon as you start adding width and height (to allow the 4:3 aspect ratio to be set), then you will have a height that is not caped, and a width that is capped at 100%. So I think you need to do that second change I suggested, if you want to be able to give an explicit height and width to allow automatic setting of aspect-ratio. And if you don't add that, then you've not solved this.

So I don't think this proposal will work without both of those, or you will break existing content.

On the plus side, I think these are safe changes from some quick tests.

Does that make sense?

[tunetheweb]

The alternative is to go with the simpler ratio @argyleink suggested above. Then could just set style="aspect-ratio: ${ratio}" on the element and wouldn't need to change any of the above. It might even be easier on the author, rather than giving a width and height anyway?

[jakearchibald]

So if you're planning on changing that to aspect-ratio: var(--vid-width) / var(--vid-height); then you will break that for any existing content, that is not currently supplying a width and height (basically all of them right now)

Ah, you're right. I was under the impression that aspect-ratio: var(--vid-width) / var(--vid-height) would be ignored like aspect-ratio: invalid / invalid if the vars didn't resolve. I'll update the PR.

The alternative is to go with the simpler ratio @argyleink suggested above. Then could just set style="aspect-ratio: ${ratio}" on the element and wouldn't need to change any of the above. It might even be easier on the author, rather than giving a width and height anyway?

The reason I went for width and height is it means we can remove the workaround when https://github.com/w3c/csswg-drafts/issues/7524 is fixed.

[tunetheweb]

The reason I went for width and height is it means we can remove the workaround when https://github.com/w3c/csswg-drafts/issues/7524 is fixed.

Well that still depends on use changing our use of the Video embed to provide width and height. Which seems like we don't do now. Though maybe we should?

Also it spreads the code between here (setting the aspect-ratio) and the site (CSS to use that aspect-ratio). Do think the ratio option might be less effort to code, easier on the author, and easier to deploy to other sites that use this library.

But whichever you think is best. And, FYI, the reason I was looking was for another issue in the next release so looked at all the open PRs and saw this one. But the fix I wanted got released yesterday anyway without this, so will back away from this now and leave you to it! 😁

[jakearchibald]

PR updated

Well that still depends on use changing our use of the Video embed to provide width and height. Which seems like we don't do now. Though maybe we should?

Well, something needs to change with how we're currently doing it, since there's a problem with how we're currently doing it. Setting width & height on <video> is the intended 'web platform' way of solving this problem.

Also it spreads the code between here (setting the aspect-ratio) and the site (CSS to use that aspect-ratio).

For now. When browsers fix the issue, we can hopefully remove the code that sets the custom properties, and the code that uses them.

Do think the ratio option might be less effort to code, easier on the author, and easier to deploy to other sites that use this library.

When browsers fix the issue, we're back to the wisdom of "set a width and height on your img/video, and you avoid CLS". If we use a ratio option instead here, we're left with a rule like "set a width and height on your img/video, and you avoid CLS. Except on web.dev where we did things differently because…".

[tunetheweb]

Fair enough! It seems like it's our media uploader doesn't provide width and heights by default for videos, like it does for images. Imagine that's making it less easy to add these.

[jakearchibald]

Hmm, I wonder how easy it would be to add that.

[tunetheweb]

According to this, this is apparently the repo: https://github.com/GoogleChromeLabs/chrome-gcs-uploader but I can't see it. Not sure if restricted, or moved?

[jakearchibald]

You should have access now, unless there's an invite for you to accept.

[tunetheweb]

I'm in! Thanks.