search

w3c / picture-in-picture

Picture-in-Picture (PiP)
https://w3c.github.io/picture-in-picture
Other
311 stars 39 forks source link

EnterPictureInPictureEvent should maybe just be PictureInPictureEvent? #145

Closed marcoscaceres closed 4 years ago

marcoscaceres commented 5 years ago

Seems a bit weird to have "Enter" at the front of EnterPictureInPictureEvent... why not just generalize it to "PictureInPictureEvent"?

beaufortfrancois commented 5 years ago

There are other Picture-in-Picture events such as Leave and Resize. This is why the event for entering Picture-in-Picture is prefixed with Enter.

marcoscaceres commented 5 years ago

Couldn't we use just one tho? And then just use the type to say what it is for?

beaufortfrancois commented 4 years ago

The EnterPictureInPictureEvent constructor event requires a PictureInPictureWindow while others actually don't. Having a generic PictureInPictureEvent that would not apply to leavepictureinpicture seems weird.

I'm gonna close this issue but let me know if you feel strongly about it and I'll re-open for further discussion.

foolip commented 3 years ago

I found this issue while trying to figure out what happened with EnterPictureInPictureEvent, which is currently shipping in Safari 14. Turns out the renaming did happen the day after this issue was closed in https://github.com/w3c/picture-in-picture/pull/189.

beaufortfrancois commented 3 years ago

@pliu6 should be able to help with that. It seems like a simple rename would work for https://github.com/WebKit/WebKit/blob/main/Source/WebCore/Modules/pictureinpicture/EnterPictureInPictureEvent.idl et al.

pliu6 commented 3 years ago

@foolip @beaufortfrancois Thanks for the reminder! I will follow up in https://bugs.webkit.org/show_bug.cgi?id=221083 and update here when it is done.

foolip commented 3 years ago

That's great, thanks for fixing this, @pliu6!