Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Document the Picture-in-Picture API #3425

Closed
a2sheppy opened this issue Jun 25, 2020 · 7 comments
Closed

Document the Picture-in-Picture API #3425

a2sheppy opened this issue Jun 25, 2020 · 7 comments
Assignees
Labels
Content:WebAPI For content triage purposes: This is related to WebAPI content

Comments

@a2sheppy
Copy link
Contributor

The Picture-in-Picture API (https://w3c.github.io/picture-in-picture/) needs to be documented. Firefox does not currently implement it but it's supported in current Chrome and Edge, Safari 13.1 on macOS, and is coming up on iOS/iPadOS in Safari 14. Since it's in Blink, presumably it's coming up in Opera soon as well.

Firefox's implementation bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1463402

@torgo
Copy link

torgo commented Jun 26, 2020

Just to note that this was positively reviewed by the TAG in 2018 and as part of that review an explainer was produced. That explainer may provide fodder for a MDN page. Also, this came to my attention because of its use on Whereby.com.

@Elchi3 Elchi3 added the Content:WebAPI For content triage purposes: This is related to WebAPI content label Jul 2, 2020
@germain-gg
Copy link

germain-gg commented Oct 22, 2020

I'd be happy to take on that task if no one has claimed it already

Edit: Could someone grant me permissions to create a new page. My MDN username is germain, thank you 🙌

@chrisdavidmills
Copy link
Contributor

Happy to let you have it, @gsouquet , thanks!

Basically, the first thing to do is work out what pages need creating, then start to create the pages in the structure, as subpages of the main Web/API page. Use an existing API ref as a template. Let me know if you have any questions.

@germain-gg
Copy link

germain-gg commented Oct 24, 2020

I believe I have reached a good stage for my content to be reviewed (ping @chrisdavidmills ). I have learned quite a far bit on the way, and mess a few things on the way (damn slugs...). Below is a list of the thing that would need your attention

Two further questions that I should be able to sort out by myself but were slightly unclear:

  • How does the Events macro work? the PictureInPictureWindow interface has an event name clash with the Window object. They both have a resize event. How can I specify the correct interface to link to when using that macro?
  • My pages do not appear on Web/API, I'm not sure what I have done incorrectly here

Thank you in advance for your review 🙌

@chrisdavidmills
Copy link
Contributor

OK, reviewed!

For a start, wow, thanks for all this great work @gsouquet! This is a huge amount of work, and it is mostly looking really good.

I've looked over it and identified a bunch of nitpicks, but nothing particularly major.

General:

  • To answer your question and add some more details, the landing page API sidebar and the main Web/API page should be fixed once the addition you made to GroupData.json gets into production.
  • You need to create a set of interface pages for the PictureInPictureEvent interface object.
  • On the subject of this comment:

The embedded example on the Picture-in-Picture API page does not have pictureInPictureEnabled, the spec does not mention anything regarding PiP in iframe

I check the example in Chrome and it seemed to work correctly. Are you saying that something is missing from the MDN platform here? The subject of being able to set feature requests on example embed <iframe>s has come up a few times now.

https://wiki.developer.mozilla.org/en-US/docs/Web/API/Picture-in-Picture_API

  • This is mostly fine; it could just do with a "Concepts and usage" section, which would consist of a couple of paragraphs describing this API's reason for being — what are it's use cases, and how is it used (high level description, rather than low level code)? See https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API for an example.

  • When you say "The pictureInPictureElement property tells you the {{DOMxRef("Element")}} that's currently being displayed in full-screen mode on the DOM", what do you mean exactly — is this the video element that is being displayed in picture-in-picture mode? What does this have to do with fullscreen API, if anything? This is the kind of stuff that would be useful to briefly explain in the aforementioned "Concepts and usage" section.

  • I edited a sentence to "Requests that the user agent returns the element in picture-in-picture mode back into its original box." Does this make sense? If not, I think this may be as a result of the confusion mentioned in the above bullet ;-)

  • "The autoPictureInPicture property will automatically enter and leave the picture-in-picture mode for a video element when the user switches tab and/or applications." — sounds a bit confusing, how does this property achieve this? I'd expect this behavior to occur automatically in supporting browsers. I'm guessing this is maybe better explained in the actual property page, which I haven't looked at yet.

  • The specification is currently showing up as unknown, but this will be fixed once the addition you've made to SpecData.json gets into production.

  • For the Browser compatibility section, it'll look really messy if you list everything on this page. I'd just list a single compat table of a feature that is central to the API. PictureInPictureWindow would be a good choice, imo.

On the subject of events:

  1. To answer your question, the {{event}} macro no longer works - just use regular links to point to the event pages you want. I've done this on the event links on the API landing page.

  2. You don't need an "Event" AND an "Event listeners" section. Just amalgamate them into a single "Events" section, in the same style as https://wiki.developer.mozilla.org/en-US/docs/Web/API/Document#Events

  3. You should however provide links to the events related to picture-in-picture that appear on other interfaces such as document, in the page for those interfaces, under a "Picture-in-picture events" subsection.

https://wiki.developer.mozilla.org/en-US/docs/Web/API/Picture-in-Picture_API/Guide

This page needs to be written — it would be really helpful to have a short guide that explains how your example works.

https://wiki.developer.mozilla.org/en-US/docs/Web/API/PictureInPictureWindow

As described earlier, amalgamate the "Events" and "Event handlers" section.

https://wiki.developer.mozilla.org/en-US/docs/Web/API/PictureInPictureWindow

The main interface page, width, and height pages could do with an "Examples" section.

HTMLVideoElement pages

  • You could do with creating separate enterpictureinpicture and leavepictureinpicture event pages.
  • I still don't really understand what https://wiki.developer.mozilla.org/en-US/docs/Web/API/HTMLVideoElement/disablePictureInPicture does, from reading the page, and you've not included an example. Is this the one you're unsure about?
  • On the onenterpictureinpicture event handler page, it says "The function receives a {{domxref("FocusEvent")}} object as its sole argument." — shouldn't this be a PictureInPictureEvent event, as listed in the spec?
  • Same comment for the onleavepictureinpicture page.

@germain-gg
Copy link

Hi @chrisdavidmills

Thank you for you very thorough review!

The example is working as expected, I was just testing it in Firefox (lol)... where the feature is not yet implemented.\

https://wiki.developer.mozilla.org/en-US/docs/Web/API/Picture-in-Picture_API

  • Concepts and usage section updated
  • That sentence has nothing to do with the Fullscreen API. Copy pasta! I used the Fullscreen API as a template for my work as it is mostly the same API as the Picture-in-Picture API
  • The autoPictureInPicture doesn't occur automatically as a web page can have multiple video on at the same time
  • I listed the BCP table as in the Fullscreen API page, it looks ok from what I see but happy to update it
  • Events macro, handlers section and all of that is sorted!

https://wiki.developer.mozilla.org/en-US/docs/Web/API/PictureInPictureWindow

  • Event handlers section removed
  • Added example

HTMLVideoElement pages

  • They were created but linked to with the Event macro, page updated
  • Regarding disablePictureInPicture, some browsers will add an overlay on top of the video or add a "pip" option in the context menu, setting that property to true will hint the browsers to remove that overlay or option, a programmatic call of requestPictureInPicture() on that video will throw. Not entirely sure how to phrase all of that well nor what a good example would be, let me know your suggestions
  • Thank you for updating FocusEvent to PictureInPictureEvent

PictureInPictureEvent

  • I have created the pages
  • I have updated the BCD PR too

@chrisdavidmills
Copy link
Contributor

@gsouquet thanks again for all the great work.

Few more bits of feedback.

https://wiki.developer.mozilla.org/en-US/docs/Web/API/Picture-in-Picture_API

  • I still can't see a "Concepts and usage" section.
  • There are another couple of mentions of "full-screen", which I think you'll want to fix.
  • The interfaces list should now include PictureInPictureEvent.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Content:WebAPI For content triage purposes: This is related to WebAPI content
Projects
None yet
Development

No branches or pull requests

5 participants