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

Allow custom <video> aspect ratio #3295

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 ratio on <video> and reduce CLS.

@jakearchibald
Copy link
Contributor Author

I'm not sure how to interpret the lighthouse failure here.

@@ -23,5 +23,9 @@ main:focus {
}

video {
// These custom properties are set by the Video shortcode if both the width
// and height attributes are set.
aspect-ratio: var(--vid-width) / var(--vid-height);
Copy link
Member

Choose a reason for hiding this comment

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

What if these attributes are not set? Should be add fallback values or simply allow this value to become invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fallback would be the default object size of 300x150 right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Falling back to invalid, which is a no-op, is probably more compatible with existing content on the site.

@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.

@chrome-devrel-infra chrome-devrel-infra bot deleted a comment from netlify bot Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants