Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.42k stars 1.99k forks source link

Switch Blogging Quick Start videos to VideoPress #62093

Open arunsathiya opened 2 years ago

arunsathiya commented 2 years ago

We offer a set of videos under the "Blogging Quick Start" branding. These are seen on this page:

https://wordpress.com/home/domain.wordpress.com?courseSlug=blogging-quick-start

You can navigate to that page via this modal on Calypso home:

image

The videos on that page are using the normal video block (VideosUi component) at the moment, but it'd be nice to switch this to VideoPress. I have located that the relevant code starts here:

https://github.com/Automattic/wp-calypso/blob/trunk/client/my-sites/customer-home/cards/education/blogging-quick-start/blogging-quick-start-modal.jsx#L20-L37

cc @Automattic/greenhouse

arunsathiya commented 2 years ago

Based on the history here, this may fall under @Automattic/serenity instead?

andres-blanco commented 2 years ago

The current content of the video courses is hosted in wpcommedia.wordpress.com. The videos are in the Media Library and there's a mecanism in place to create/edit the courses. If we want to change where the videos are hosted we should also update the way we add metadata for those videos and how we link them to courses.

More info on current implementation: PCYsg-DgC-p2

jgcaruso commented 2 years ago

I just did a quick check of the videos on the "Blogging Quick Start" page and it looks like they are being hosted with VideoPress, so it looks like this really just requires an edit to the page to use a different block and/or the wpvideo shortcode and/or a VideoPress iframe embed depending on how the current page was built.

The Video GUID can be found in the URL of the video's poster image https://videos.files.wordpress.com/BfK0FbCd/video-3_mp4.scrubthumb.jpg => Video Guid BfK0FbCd.

andres-blanco commented 2 years ago

For reference this is the component that renders the video element: https://github.com/Automattic/wp-calypso/blob/trunk/client/components/videos-ui/video-player.jsx#L53

jgcaruso commented 2 years ago

Oh nice, ok so a few things I noticed:

  1. There is a VTT tracks file. This would need to be uploaded directly to the video in order for the VideoPress player to load it. We have an upload button on the Video block in Gutenberg, but I don't think we have a way of uploading them directly to a video in the Media Library. A workaround here could be to create a draft post, pull in the video from the media library, then upload the VTT file to the block.
  2. The onTimeUpdate handler is being used: We don't currently expose this in the VideoPress player, but we do send postMessage events on play, pause and ended events. So if time is only tracked for "video end" detection, this could be changed to use postMessage events instead
  3. The onPlay handler is being used: We don't support this, but we do send a postMessage event on play/pause of the player.

In order to use an iframe player for VideoPress, something like the following could be done https://github.com/Automattic/wp-calypso/blob/b4c6caf9a9a796ed486320285d1af7aaf259d7ec/client/post-editor/media-modal/detail/detail-preview-videopress.jsx#L138-L142

There are examples in that file on how to listen via postMessage as well.

blackjackkent commented 2 years ago

This does seem like it would be a change that would fall within Serenity's sphere. To confirm I'm understanding the issue correctly - currently the videos UI is using the <video> HTML element, but we need to be using an iframe instead in order to load the video from VideoPress?

The onTimeUpdate handler is being used: We don't currently expose this in the VideoPress player, but we do send postMessage events on play, pause and ended events. So if time is only tracked for "video end" detection, this could be changed to use postMessage events instead

@jgcaruso Our intention with the onTimeUpdate behavior in that file was to be able to record the user as having completely watched the video about 5-10 seconds before the end (when the closing title screen starts playing), rather than the video end itself. I'm not sure if this is feasible with the VideoPress approach, based on what you are describing.

jgcaruso commented 2 years ago

I'm not sure if this is feasible with the VideoPress approach, based on what you are describing.

I think you are correct. We don't have a seconds by second update available to enable this type of behaviour. It is something we're talking about enabling to allow other types of integrations with the player but its not on our immediate roadmap.

I'm not sure what the initial intention of this request was, I just popped in because my team was CC'd.

Not sure what the priority is on this request, but maybe its something for the future once we're able to build in support for these types of interactions? Or if they could be swapped out for different events... for example, just record that the video was started not that 5-10 seconds were actually watched?

arunsathiya commented 2 years ago

I'm not sure what the initial intention of this request was Not sure what the priority is on this request, but maybe its something for the future once we're able to build in support for these types of interactions?

For some additional clarity, I created this issue just because I feel that the VideoPress player is generally snappier compared to other implementations like VideosUI or the browser <video> tag. 🙂 I'll leave the decision on whether we want to do that, and when, to Serenity.