Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Allow custom aspect ratio on <video> #8399

Closed
wants to merge 2 commits into from
Closed

Conversation

jakearchibald
Copy link
Contributor

This is part of GoogleChrome/webdev-infra#28, to allow custom aspect ratios on video elements and reduce CLS.

@netlify
Copy link

netlify bot commented Jul 25, 2022

Deploy Preview for web-dev-staging ready!

Name Link
🔨 Latest commit 88d3444
🔍 Latest deploy log https://app.netlify.com/sites/web-dev-staging/deploys/6305e4327672a00008e96b13
😎 Deploy Preview https://deploy-preview-8399--web-dev-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@matthiasrohmer matthiasrohmer added eng For items that require engineering work. eng - UI Tasks related to UI/CSS labels Aug 18, 2022
@jakearchibald
Copy link
Contributor Author

I haven't tested this btw, so it'll need an eyeball to check that it works with existing video content.

@matthiasrohmer matthiasrohmer self-requested a review August 29, 2022 20:43
@matthiasrohmer
Copy link
Collaborator

Thanks for adding this, @jakearchibald! I did run a Percy test for this to see if any existing videos break.

What I did: check out this branch, merge latest changes from main and manually update the web-infra package with the changes from GoogleChrome/webdev-infra#32.

There's only one slight change, with the Mishipay case study (https://web.dev/mishipay/#scanning-products, just above this headline). The second portrait video now renders slightly smaller. Jake, are you fine with me committing to your branch with a potential fix for this?

@jakearchibald
Copy link
Contributor Author

I did run a Percy test for this to see if any existing videos break.

Nice!

There's only one slight change, with the Mishipay case study (https://web.dev/mishipay/#scanning-products, just above this headline). The second portrait video now renders slightly smaller.

Ohh, good catch! Here's the source for that embed:

{% Video
  src="video/8WbTDNrhLsU0El80frMBGE4eMCD3/NqyMBZGYzGSNqLE7soXR.mp4",
  autoplay=true,
  muted=true,
  playsinline=true,
  loop=true,
  width=1296,
  height=600
%}

Whereas the video resource is 600x1296, so this is author error, although due to the forced 16:9 ratio, it wasn't spotted. Correcting the width & height will fix this.

Jake, are you fine with me committing to your branch with a potential fix for this?

Yep!

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Did some manual testing of this btw and LGTM (author errors aside).

@stale
Copy link

stale bot commented Dec 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To prevent this from happening, leave a comment.

@stale stale bot added the stale label Dec 21, 2022
@tunetheweb
Copy link
Member

Not stale

@devnook
Copy link
Contributor

devnook commented Oct 11, 2023

Now really stale.

@devnook devnook closed this Oct 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng - UI Tasks related to UI/CSS eng For items that require engineering work. stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants