Video Editor: Add VideoPress poster picker

[donnapep]

This PR enables choosing a custom VideoPress poster by either selecting a particular frame of the video or by uploading a custom image.

Testing

Test using a site that has VideoPress enabled (e.g. a WP.com site that has a Premium plan or a Jetpack site that has a Jetpack Premium plan and VideoPress enabled in Jetpack settings).

The Video Editor can be accessed in the post editor by selecting "Add Media" from the toolbar dropdown, selecting a video, clicking the "Edit" button, and then clicking the "Edit Thumbnail" button.

It can also be accessed by clicking "Media" in the sidebar, selecting a video, clicking the "Edit" button, and then clicking the "Edit Thumbnail" button:

If the video cannot be loaded or an error occurs when updating the poster, the following message is shown:

[matticbot]

Test live: https://calypso.live/?branch=add/videopress-poster-picker

[a8ci18n]

Hi! I've found a possible matching string that has already been translated 79 times:
translate( 'Thumbnail' ) ES Score: 7

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

[dmsnell]

is there a reason this isn't unloaded in componentWillUnmount?

[donnapep]

It's being removed in the destroy function, which is called by componentWillUnmount. destroy is also called in componentDidUpdate, since if the VideoPress ID changes, the player needs to be re-initialized.

[dmsnell]

very good!

[dmsnell]

was it with @gwwar that I recently held a discussion on this? there's something about this construction I don't like (also it's unsafe because JSON.parse() throws on invalid syntax) because it assumes that string data is JSON data

I would be far more in favor of something else like this…

const parseEventData = data => {
	try {
		return JSON.parse( data );
	} catch (e) {
		return data;
	}
}

…

const data = parseEventData( event.data );

[donnapep]

Actually, the response is already JSON. I have no idea why I was trying to parse it. 🤦‍♀️

[dmsnell]

if we want to use JSON data we will need to parse it, otherwise it's just a string…

[donnapep]

Huh? Not sure I follow, since event.data is already an object. I discovered parsing wasn't necessary by actually trying to parse it and then realizing the catch handler fired every time.

I was like, 'Calypso, JSON.parse( event.data )!', and Calypso was all like Unexpected token o in JSON at position 1! So buh bye JSON.parse!

[donnapep]

@ockham Awesome! Tested everything again and it's all still working so I'm going to go ahead and deploy. Thx! 😃

[kraftbj]

We noticed that this is live for staging, but still disabled in production ( https://github.com/Automattic/wp-calypso/blob/master/config/stage.json#L82 vs https://github.com/Automattic/wp-calypso/blob/master/config/production.json#L77 ). Is there a timeline for rolling it out to prod?

634025-zen

[chaselivingston]

Also requested in production here: 624739-zen

[donnapep]

@kraftbj @chaselivingston I'll get a PR together to deploy to production. Cheers.

[donnapep]

PR is here and is awaiting review.