Experimenting with an aspect-ratio attribute

[mmcc]

I ran into issues trying to use media-chrome in a responsive environment and found myself hacking around the same responsive workflow each time. Instead of offloading all that work to individuals, we can take care of that directly in media-container by allowing an aspect-ratio attribute.

This probably needs some more defensiveness, but wanted to go ahead and open it up to get feedback on the approach.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mux/media-chrome/HYBJXE2dWNNj5nK9v7k65Wj6EtXX
✅ Preview: https://media-chrome-git-fork-mmcc-aspect-ratio-mux.vercel.app

[cjpillsbury]

Are you trying to enforce an aspect ratio that's different from the src Media's aspect ratio, or do you just wish <media-controller/> better respected the aspect ratio of the provided src Media?

If the former, your general direction makes sense to me, though you may want to explore https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleSheet as a "cleaner" way to dynamically add/update StyleSheets and CSS rules (assuming we don't want to simply add style or update CSS variable(s) instead).

If the latter, I think that's part of a larger CSS cleanup effort. For that particular case, we should be able to do something similar to what I did in <mux-video/> to simply respect the "intrinsic size" of the content: https://github.com/muxinc/elements/blob/main/packages/mux-video/src/CustomVideoElement.js#L24-L33

[heff]

Cooooooool. Does this work? I thought it required two elements and this is just using media-container.

https://alistapart.com/article/creating-intrinsic-ratios-for-video/

[heff]

Do we need this or can we just let the later rule override this one? i.e. can you add height: auto with width:100% below?

[heff]

Doesn't this need a height of 0px to work? Does that break the inner layout within the media container? For example the control bar is not absolutely positioned, so I'd assume it forces some height in addition to the padding creating the ratio.

[heff]

this.shadow needs to be this.shadowRoot.

[mmcc]

Ok addressed the feedback, but yes, this seems to work great where I've tested it! Keep in mind the assumption here is that you're wrapping this in an element that's controlling width.

Are you trying to enforce an aspect ratio that's different from the src Media's aspect ratio, or do you just wish better respected the aspect ratio of the provided src Media?

It's not necessarily different from the src, but it's more importantly just what the developer specifies. The issue with relying on source is that it's virtually impossible to avoid a weird resize after the file metadata kicks in, so in practice I think that's rarely what folks want. Allowing for a specified aspect ratio allows for people to either directly specify the aspect ratio based on what's in their CMS to avoid a flash, or just have a predefined fluid aspect ratio regardless of the source for better predictability in the layout.

I did take a look at CSSStyleSheet but it seemed like support was still sketch enough that I didn't know if we should be relying on it. Is that just a bad assumption?

[heff]

but yes, this seems to work great where I've tested it!

I think I understand why this works now. box-sizing: border-box makes it possible. Technically I think the content is actually breaking out of the height:0, without forcing it to get taller. Not sure if that's gonna cause issues at some point, but I don't see any yet.

Maybe related, should we set a minimum width?

[mmcc]

I don't know what you're talking about, that looks flawless to me. I need to do some more cross-browser testing anyway, but I'll see if I can make this feel a little cleaner.

But also the general responsiveness of the control bar gets pretty wonky. I think we're going to want to make sure we've got some good default stories over responsiveness (or as good as we can given the flexibility).

@cjpillsbury Any other thoughts re: CSSStyleSheet? It definitely looks much cleaner/better, but all that red in the browser support box makes me nervous.

[cjpillsbury]

Unless I'm missing something, we should be able to use it, since we will just be relying on insertRule and deleteRule for the modifications. We used the JS APIs years ago for another widely used player.

[cjpillsbury]

Not sure if we'd want to resolve this in this PR, but we probably should also be applying these rules to the media element (e.g. <video slot="media"/>) and allowing the <media-controller/> to size itself based on the media's size (e.g. still able to rely on descendant sizing, whether intrinsic or computed or some mix of the two). This will make styling a bit cleaner for devs in most standard use cases.

[heff]

@cjpillsbury let's say we wanted to merge this. What sizing changes have happened since this or should happen before this? Or can we get this updated and move forward with it?

[luwes]

@cjpillsbury last mentioned this PR could be closed if I recall correctly, the solution is documented here:
https://github.com/muxinc/media-chrome/blob/main/docs/media-controller.md#styling

I'm trying to figure out how to best do this in mux-player at the moment, in this case media-chrome's default aspect-ratio of 3 / 2 is not needed I think, media-chrome has to fill the player element dimensions. I'll add a commit reference here later.

[heff]

Awesome. Let's give @mmcc a minute to review and make sure this lines up with what he was thinking, or close it in a week otherwise.