Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Try] Add youtube support in cover block #10708

Closed
wants to merge 5 commits into from

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Oct 17, 2018

Description

This PR is a proof of concept for the addition of youtube videos as the background of the cover block.
It depends on #10659.

In order to have a flow that allows the insert of youtube videos cover block is updated to allow the addition of files via their URL.

Youtube video background is not perfect when the video ends, we may have a black screen for one second until the loop starts again.

I checked what other tools that support youtube videos as background are doing. It looks like they are using the youtube JavaScript API, and they are not using the loop functionality the API offers but instead they set a timer for the duration of the video and when the timer ends they use the API to put the video again at the second zero. Even though this solution is not totally perfect sometimes it is possible to notice a black screen but the duration is smaller when compared to our approach, so this approach provides a better UX.

I think we are on the limit of what is possible to do with an iframe if we want to reduce the black screen between start and end we will need the API with the go-to second 0 "hack" like others are doing.
This raises some questions:
- Should we have a block in the core that requires custom javascript on the frontend?
- Should we have a block in the core that includes javascript from a third party (youtube) that we have no control over?

How has this been tested?

I added the cover block.
I checked that I can add youtube videos as the background of the cover.
I checked I can still upload or select an image or video from the media library as the background of the cover block.
I checked I can insert an image or video as a background using its URL (the domain must be a site where we are testing or a third-party that allows Ajax requests required to check if the URL is a video or an image).

@jorgefilipecosta jorgefilipecosta added the [Type] Enhancement A suggestion for improvement. label Oct 17, 2018
@jorgefilipecosta jorgefilipecosta changed the title Add youtube support in cover block [Try] Add youtube support in cover block Oct 17, 2018
@aduth
Copy link
Member

aduth commented Oct 18, 2018

Is there context to where this was discussed as an enhancement? To me, it's nice but doesn't seem very pressing, at least without considering idea of things like embeds / block nesting / relationships to use to effect the background at a high level, rather than ad hoc consideration of a single provider from the cover block. What do we do when we decide we want Vimeo support? VideoPress? Some other future hypothetical video embed provider?

@jorgefilipecosta
Copy link
Member Author

Is there context to where this was discussed as an enhancement? To me, it's nice but doesn't seem very pressing, at least without considering idea of things like embeds / block nesting / relationships to use to effect the background at a high level, rather than ad hoc consideration of a single provider from the cover block. What do we do when we decide we want Vimeo support? VideoPress? Some other future hypothetical video embed provider?

During the discussions around the "Cover Block" we discussed that it would be good to support youtube videos. The rationale was that some stats said that the youtube block had more usage than video block. So we decided to make a P.O.F, I think we will not advance with this feature on this phase so I'm closing the PR for now.

@jorgefilipecosta jorgefilipecosta deleted the add/youtube-in-cover-block branch November 8, 2018 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants