Add an audio and video player to the media manager modal

[johnbillion]

When browsing for audio or video files from the Add Media button, there is no means of playing the media in order to preview or verify that it's the file you expect.

When viewing a media item in the media grid, or editing a media item, a player for both audio and video files is shown.

It would be good to have a player embedded in the sidebar of the media manager (above Attachment Details) when viewing a media item.

[joemcgill]

Hi @johnbillion, welcome to Trac! I think this is a great suggestion.

[johnbillion]

A thought just came to mind, we'll need the ability to disable or replace this player (eg via a filter) because sites with heavily customised media management may need to play the media through a custom player.,

[antpb]

I would also like to see this. I'm going to poke around the Media Library to see what this would take. It seems pretty straightforward to me. Worth calling out, if the solution is in the Core frames, we need to make sure to mirror changes in the Gutenberg repository.

[Mista-Flo]

Fix indentation

[Mista-Flo]

Hello,

I have uploaded a patch for this feature request that would be nice for the user experience.

You can also check the two screenshots, one for the audio player and the other for the video player.

It seems good to me, but it's better to double check that there is no regression across media modals.

I decided to not display the default video/audio icon along the details since there is no need for that with the player.

[hellofromTonya]

Per Media scrub today, @Mista-Flo noted this ticket will benefit for design feedback. @karmatosed is on it! Capturing here to keep it on our radar.

[hellofromTonya]

Capturing testing conversation from the Media scrub today, ie so we don't forget:

@antpb will doing testing on it.

I owe testing on this one.
I'd really love to see this in before Beta.
Let's see if I can get some testing in today.

[antpb]

When we get Design thumbs up or suggestions, we can get this merged before Beta 1 next Tuesday. I'll be on standby for the thumbs up!

[paaljoachim]

Looks great to me!
I also pinged @mapk and @nrqsnchz about this ticket.

[mapk]

Thanks for the ping, @paaljoachim. I agree, the design and location of the media player looks great! Thumbs up on design. Let's get this shipped.

[hellofromTonya]

Per conversation with @antpb, this one is ready.

[nrqsnchz]

Agree with @mapk, this looks good and ready to ship 👍

[johnbillion]

This is working nicely and is a very useful addition. I'm just seeing two display issues with the tracker. Screenshots above.

1. On an audio file, the tooltip for the time gets cut off at the top.
2. On an audio or video file, there is a square white artefact around the position marker.

Tested with Firefox and Chrome on macOS .

[garrett-eclipse]

Minor conditional logic refresh.

[johnbillion]

@garrett-eclipse It looks like with your patch the media-progress-bar doesn't get shown for audio and video files that are still uploading. Can you check and confirm?

[garrett-eclipse]

From an initial code review I've uploaded 43640.3.diff that combines the <# if ( 'video' !== data.type && 'audio' !== data.type ) { #> conditional into the previous statement as an else since both these conditions were checked in that chain.

Note: This doesn't address @johnbillion's flagged items, just wanted to improve that conditional after my code review.

[garrett-eclipse]

Hmm I'll definitely look, that combining of conditionals shouldn't have changed the logic... but then again I do live/work around 2 little kids so I very well could have flubbed.

[garrett-eclipse]

Test Results: Upload video shows uploading and displays the embed upon upload.

[garrett-eclipse]

Test Result: Audio shows the uploader but upon upload the audio player isn't presented instead there's an empty space until you switch to a different item and back.

[garrett-eclipse]

Test Result: Deleting a media element changes select focus to the next remaining item but the side panel doesn't load the item.

[garrett-eclipse]

Got a chance to test @johnbillion and I did see the media uploader for both audio and video. Could you check if a larger file to see if it's just not uploading too fast to see the loader?

I did note that;

1. "On an audio file, the tooltip for the time gets cut off at the top." Confirmed
2. "On an audio or video file, there is a square white artefact around the position marker." Can't Reproduce (Mac Chrome)
3. While the video player becomes available upon upload the audio one doesn't. You need to switch to another item and return before it appears.
4. When deleting an item it's focus moves to the next available item, but the side panel is blank. This may be expected but would assume when focus is on an item it's details would be displayed.

Screengrabs of the above are attached.

[garrett-eclipse]

Apply wp-audio class to the audio wp-media-wrapper and use it to apply a 12px top margin to provide space for the timer tooltip.

[garrett-eclipse]

Test of the latest patch to show timer tooltip fully in view.

[garrett-eclipse]

1. "On an audio file, the tooltip for the time gets cut off at the top." Confirmed

With 43640.4.diff I've applied a 12px top margin to the audio media wrapper in order to give it the space so the timer tooltip wasn't cut off. The alternative I was looking at was just removing the overflow hidden on the .attachment-info but I was worried that might introduce a back-compact issue with plugins using the forced overflow as their bounds.

[garrett-eclipse]

Replying to johnbillion:

2. On an audio or video file, there is a square white artefact around the position marker.

Tested with Firefox and Chrome on macOS .

Hi @johnbillion can you do some inspecting on this issue, I'm unable to reproduce on mac Chrome/Firefox/Safari/Opera. Possibly a browser extension? Hoping an inspect can at least tell us the artifact class or id or markup.

[garrett-eclipse]

Replying to garrett-eclipse:

4. When deleting an item it's focus moves to the next available item, but the side panel is blank. This may be expected but would assume when focus is on an item it's details would be displayed.

I can confirm now that this is expected, if you open the modal clean and tab through focus will land on the media elements but the sidepanel doesn't load until enter/click action is taken. So when a deletion happens and the focus goes to the next item it is expected a click or enter action is needed for the attachment details to display.

[garrett-eclipse]

Replying to garrett-eclipse:

3. While the video player becomes available upon upload the audio one doesn't. You need to switch to another item and return before it appears.

I can no longer reproduce this behaviour after multiple attempts so can disregard.

The latest patch in my opinion is good to go beyond a retest and investigation from @johnbillion on that artifact.

@antpb / @Mista-Flo would you like to do some more testing and confirm things are looking good here.

[johnbillion]

The square white artefact around the position marker is from the mejs-time-handle-content element and its corresponding style is here: https://core.trac.wordpress.org/browser/tags/5.5.1/src/js/_enqueues/vendor/mediaelement/mediaelementplayer-legacy.css?marks=417#L403 . Screenshot above.

I've been giving this some testing today and it's all looking good apart from the above.

I'm going to get this committed and we can investigate further and iterate if necessary.

[johnbillion]

In 49195:

Media: Add an audio and video player to the media manager modal.

This introduces a means of playing existing audio and video files while browsing them prior to selecting them for use.

Props antpb, Mista-Flo, garrett-eclipse, mapk

Fixes #43640

[garrett-eclipse]

Reopening for further investigation on the remaining white artifact.

[helen]

43640.diff​ fixes the placement issue in the admin context - wp-mediaelement.css seems to need a general review of colors so I'm going to ignore that I don't really love the styling here and leave that for another ticket and time :)

[helen]

Looked at this more deeply, comparing between media modal and front-end display via shortcode ([audio src="blah"]), 43640.2.diff​ addresses the base issue better, which is that box-sizing was being overridden by another media modal rule.

[helen]

In 49343:

Media: Fix styling for MediaElement player in media modal.

Fixes #43640.