Move self-hosted video dimensions to Source object#15556
Conversation
55c2b65 to
88796bb
Compare
88796bb to
652d6dd
Compare
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
| height={height} | ||
| width={width} |
There was a problem hiding this comment.
are these okay to be undefined?
There was a problem hiding this comment.
I think this is better than providing a guess at width & height (i.e. by using the largest available), then possibly updating later if it's not correct, but I'm not 100% on that. We can revisit later if we find out this causes an issue, although I think it'll be fine.
| const [width, setWidth] = useState<number | undefined>(); | ||
| const [height, setHeight] = useState<number | undefined>(); |
There was a problem hiding this comment.
Are these derived from the loaded asset?
There was a problem hiding this comment.
Yep. On the client, once the video data has loaded, these values will be set
|
Overdue on PROD (merged by @domlander 30 minutes and 7 seconds ago) What's gone wrong? |
|
Seen on PROD (merged by @domlander 33 minutes and 59 seconds ago) Please check your changes! |
What does this change?
Updates the
MainMediatype:widthandheightaspectRatioUpdates the
Sourcetype:widthandheightWhy?
We will soon be transcoding multiple videos per MIME type. This has the goal of improving the experience for users, by downloading a smaller video if they have a small screen. Since each source will have different width and height values, the dimensions of the video should live on each source, rather than the parent.
Dimensions are no longer passed in to the SelfHostedVideo component as these are now indeterminable on the server. The aspect ratio is known, however, and it is passed to the component so that the browser is still informed of the size that the video will take up, to prevent CLS.