WICG / document-picture-in-picture

https://wicg.github.io/document-picture-in-picture/
Other
58 stars 9 forks source link

Remove `initialAspectRatio` #21

Closed beaufortfrancois closed 1 year ago

beaufortfrancois commented 1 year ago

With lockAspectRatio gone, I think we should remove initialAspectRatio from the spec and tell web developers to only use width and height to set the desired PiP window size. We could make sure that the aspect ratio is respected as well when specifying those implementation wise.

const pipWindow = await documentPictureInPicture.requestWindow({
  initialAspectRatio: player.clientWidth / player.clientHeight,
});

can be replaced with

const pipWindow = await documentPictureInPicture.requestWindow({
  width: player.clientWidth,
  height: player.clientHeight,
});

@steimelchrome What do you think?

steimelchrome commented 1 year ago

Hmm that probably makes sense. My main concern was the given that the window size is restricted and we don't currently have a way to give the website those restrictions, it seems like it could be easier for them to just specify an aspect ratio. That said, if you think we should change it so that ones that fall outside the range try to keep aspect ratio anyway, then maybe that's okay. WDYT? Do we know of anyone who was planning on using aspect ratio?

beaufortfrancois commented 1 year ago

I'm happy with using initialAspectRatio only as well.

steimelchrome commented 1 year ago

We have some partners who would definitely prefer to have width and height parameters, so I don't think we'd want to remove them

steimelchrome commented 1 year ago

Fixed by #36