xwp / wp-core-media-widgets

Feature plugin for development of new core media widgets for image, video, and audio (and others)
https://wordpress.org/plugins/wp-core-media-widgets/
35 stars 8 forks source link

Set video media source automatically #99

Closed obenland closed 7 years ago

obenland commented 7 years ago

This may be out of the scope for media widgets, but when using an audio or video file, the edit screen allows users to upload and select alternative sources for full html5 availability.

In the case of video, when selecting a mp4 or ogv or webm file, we could automatically set those as alternative sources.

This might not make a difference to the html5 compatibility, but it's one less thing a user has to worry about.

westonruter commented 7 years ago

@obenland Can you elaborate further? You can currently upload and select the alternative sources:

image

obenland commented 7 years ago

Right, so when the original file is of one of these three formats, could we not just automatically set it as an alternate source? (even though it's not really alternate)

westonruter commented 7 years ago

Good point. If I select an MP4 to begin with, actually I think then that there shouldn't even be an “MP4” button shown since there is no need alternative. I think this would be better than pre-populating the MP4 alternative source, since then you could click to remove the alternative source and that wouldn't make sense.

timmyc commented 7 years ago

Would this apply to the audio widget as well? Here is a screen grab for editing audio on an mp3 file:

customize__wordpress_develop_ _just_another_wordpress_site
obenland commented 7 years ago

It does, yes

westonruter commented 7 years ago

It's true that this issue does reflect a core deficiency in the media JS, so it would probably be better to fix in core rather than focus on patching it in the plugin only.

westonruter commented 7 years ago

We do need to revisit this. I just tried inserting an MP4 into a post and it generated a shortcode:

[video width="480" height="270" mp4="http://src.wordpress-develop.dev/wp-content/uploads/2017/04/echo-hereweare.mp4"][/video]

Note that it used the mp4 attribute and not the url attribute. As such, when editing the video it showed with that source pre-selected:

image

westonruter commented 7 years ago

By not using the url field, we'll have to make some changes in how we detect in the base media widget for whether or not a selection has been made or not. Namely logic like this in \WP_Widget_Media::widget() will need to be changed or made extensible (the $instance['url'] part):

// Short-circuit if no media is selected.
if ( ( ! $instance['attachment_id'] || 'attachment' !== get_post_type( $instance['attachment_id'] ) ) && ! $instance['url'] ) {
    return;
}
timmyc commented 7 years ago

So it seems there are a few different issues at play here ( too early in the AM for puns? ) - so to summarize:

  1. Original Issue: Pre-selection or better yet, not showing the alternate source buttons. Which we originally said should be deferred to core.
  2. The secondary issue is the details panel being shown when a video from the library is selected ( a bit unsure if that is buggy or not )
  3. Another issue I just discovered on that flow, but done via the widget - the preview of the local mp4 file is not shown i the "edit" panel.

Shall we break out all of these into individual issues?

westonruter commented 7 years ago

I think all of these 3 issues are closely related and will be resolved by just just opting to use the video extension prop (e.g. mp4) instead of only using url.