Closed callinmullaney closed 1 year ago
@ryanhagerty This is ready for another review from you. I converted the old padding-bottom
hack to the preferred aspect-ratio
method you mentioned. I think the JS is still necessary here because the CSS defaults to a 16/9 aspect ratio video which is the most common. However, youtube (and I assume vimeo as well) support other aspect ratios. Our component here should be flexible enough to support any type without the weird horizontal/vertical black bars.
@mndonx Thanks for the comments/review. I re-ordered the styles to alpha and set the placeholder text to a string translation. We landed on "impactful properties should be towards the top and less impactful at the bottom" but we wanted this to be done in an automated fashion. Sorted to alpha for now until that script is in place.
@ryanhagerty Lets discuss in our group review but I think the JS is still needed here. We are defaulting to 16/9 aspect ratio as this is an industry standard but it's possible for a video to be something else (16/10, 21/9, and whatever portrait videos are). Youtube (and I believe vimeo as well) do not enforce a specific aspect ratio so we should accommodate for variations.
:tada: This PR is included in version 1.9.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:
Summary
Documentation Update (required)
No documentation changes needed as this is an existing component rewrite
How to review this PR
<video>
tag.