Add new core media widget

[melchoyce]

Adding images to your widget areas is a common, yet currently incredibly tedious task -- you need to upload it in your media library, find the url, copy the url, and then manually add an image tag inside of a text widget. That's a lot to ask for a beginner user to do. We should include a default image widget in core to make this task easier.

[philiparthurmoore]

Huge huge huge +1.

[melchoyce]

Or: we could go further and make a catch-all media widget. I've wireframed a basic flow in widgets.png​.

[valendesigns]

+1 Has anyone started a patch yet? I would be happy to create this widget if not.

[designsimply]

I started this as a plugin! I think I might need a little help to get it into proper working order, but I think it's a good start! I'm having trouble getting the widget to update in the customizer. I would really, really love to see this through.

Here's what I have so far: ​https://github.com/designsimply/simple-media-widget

@valendesigns, I will work it into a patch as a next step.

[valendesigns]

@designsimply and I are discussing her plugin privately in Slack, we'll move the conversation back here when we have something to report, like an action plan.

[designsimply]

I was having trouble getting the customizer preview to update when input values were updated using jQuery (​37s, ​slack), and I got that fixed with help from @westonruter. Yay! See ​f05c566

Another part I'm having trouble with is getting previously selected images to stay selected when the media manager is re-opened (​15s).

Next up: I'll attempt converting all of this to a patch.

[paaljoachim]

Having an add media widget in core would be awesome!

From my understanding of what is going on here the focus is right now on getting one image in place.
Then later expanding it to add media such as multiple images, a galleri, video, audio etc.
I have contacted a few image and video widget plugin developers letting them know about this widget project.

Have a great weekend!

[celloexpressions]

Once this works for images, I like the idea of adding support for other media types too (with the same flow). Based on implementing the media controls for the Customizer, it shouldn't be too hard to add support for other things like audio once the basic image code is in place, although the Widgets API is much more difficult to work with than the Customizer API.

[peterchester]

For what it's worth, I'm happy to offer our image widget as a candidate for merging in to core. It's hugely popular. ​https://wordpress.org/plugins/image-widget/

[melchoyce]

@designsimply, how are things going here? Do you need any help with anything?

[Stagger Lee]

Folks, if you spend your energy and time helping people here you get all you wish and much more. I wont explain how now. Possibilities are endless.

#32474

[bradyvercher]

While it does more than core needs, I'm also happy to contribute any code from Simple Image Widget: ​https://wordpress.org/plugins/simple-image-widget/

[valendesigns]

We need to decide on the best path moving forward before attempting patches, review of all the existing options above, and make a decision on which has the most viable codebase. I'll do a review personally in the next few days if time permits between Menu Customizer work, but everyone who is interested in seeing this become a real feature should do the same. We can always cherry pick from all of them.

I'm attaching a list of what I expect the widget to do. Discussion around this list is encourage and I will update it as needed. All control media types need a mockup once we've landed on functionality. Even though this isn't a feature plugin, it will likely be developed in a similar way so be familiar with the ​Feature Plugin Checklist standards. I can setup a repo that has tools so automated unit testing and code standards can be done with Travis CI.

I'll edit and mark items complete as we go. If I've missed something or we disagree on implementation please let me know.

Expectations

• Control supports:
  • Image - This should be displayed.
  • Gallery - How should this be displayed in the widget, or should it even show at all?
  • Audio - Should audio be playable and responsive?
  • Video - Should video be playable and responsive?
• Mockup the real UI for each control
• Must work in the Customizer
• Has PHP & JS unit tests
• Output is extensible with filters, at least at a basic level, so 3rd-party players and galleries can override the display

[melchoyce]

I imagine it working exactly like the media views in TinyMCE — preview images, gallery, audio, video, with an option to play the interactive media. Everything should be responsive. Things like title, alt text, links, gallery settings, etc., should all be controllable via the media modal, rather than inputs in content of the the widget itself. The majority of the UI is already there, either in TinyMCE or the media modal.

[Stagger Lee]

Can you borrow code from this plugin ? Half million active installs. I dont get it what do you want to achieve, no offence.

​https://wordpress.org/plugins/black-studio-tinymce-widget/screenshots/

[helen]

Some things here:

• Let's please look at existing, stress-tested plugins like those offered by @peterchester and @bradyvercher. We can come up with core-ideal UI, that's fine, but they've got working code and can be trusted (as can @designsimply). We don't need to reinvent wheels all the time.
• Regarding media besides images, shortcodes don't work in widgets, see #10457.
• @Stagger Lee: please stop spamming tickets and Make/Core threads with unrelated and non-constructive comments.

[valendesigns]

These plugins have something in common that doesn't work, which is support for other media types and they save additional input fields which over complicates the situation. I agree we should not reinvent the wheel here, but none of them are a simple patch away from being merged. So even though they are stress-tested they only solve 1/4 of the problem.

I feel like we could make this work if we used AJAX to load the markup and saved data as a shortcode. Obviously I would need to qualify that further, but we shouldn't rule it out completely based on what others tried to do in the past. We also don't need to run shortcode_unautop like was part of the issue in #10457.

However, we should be cautious and do a POC if we decide to go the preview route. And if we do not have previews, besides image, that is 100% ok with me too. Though I still think we should treat this like a feature plugin for testing purposes and the fact it would likely be a healthy patch. One of my main concerns is that it must work flawlessly in the Customizer when it's all complete.

[helen]

Replying to valendesigns:

These plugins have something in common that doesn't work, which is support for other media types and they save additional input fields which over complicates the situation. I agree we should not reinvent the wheel here, but none of them are a simple patch away from being merged. So even though they are stress-tested they only solve 1/4 of the problem.

It is fine to use pieces of code. I am in no way suggesting that we merge one of the plugins mentioned above. I would, however, agree that this is something that should be a feature plugin right now and move this out of 4.3.

[celloexpressions]

WP_Customize_Media_Control would be a good basis in terms of code if we want to roll our own. We can very easily handle images, audio, video, and even documents (uploads ranging from PDFs to any other formats the site supports). Shortcodes would definitely be out of scope, along with galleries, external embeds, etc. although widgets could potentially be created for those as well. We should only save one field - the attachment id - and leverage the data associated with the attachment as much as possible. Could potentially make title and caption optional or have auto-populating fields for them but I wouldn't go any further than that. Alt text for images could be taken from the attachment rather than doing additional UI in the widget.

[peterchester]

Are you sure it makes since to make a 'media' widget instead of an image widget and a video widget and maybe an audio widget (not sure there's a lot of demand for an audio widget)?

Both the image-widget and simple-image-widget use the media manager but they focus on images and I can tell you from years of feedback that people have a lot of demands that are image specific.

For example, people often want to make the image link, to have alt text, and other image specific elements. I mean I suppose we could have admin fields for all those things and find a way to apply them to all media but i'm not sure it makes sense.

[Stagger Lee]

@helen, sorry it was not meant like this.
This plugin has much of things from mockups here and what people decsribe.

• Strip TinyMce toolbars (leave TinyMce editor field), leave "Add media" button and much is already done.
• Image, gallery previews work inside widget.
• Could not get naked Youtube URL to show preview (embed shortcode works), probably easy for one talented coder to make.
• Works in Customizer (Have to say I did not understand all what is asked here for this option.)
• Probably easy to make to work with Shortcake. (@melchoyce wishes)

Cannot say anything of code in this plugin. Leave it to coders to say if it is worth anything at all.

[designsimply]

Replying to melchoyce:

@designsimply, how are things going here? Do you need any help with anything?

@melchoyce, I did need help, but I got some in Slack (which rocks). I think I have something solid enough for a code review and maybe even a feature plugin proposal. If it gets a green light, I will need help moving it from plugin to core patch.

[designsimply]

Replying to helen:

• Regarding media besides images, shortcodes don't work in widgets, see #10457.

WordPress.com has one, and I'm sure I could get that, but it looks to me like it does some code gymnastics to workaround limitations (at least as far as I can tell). :) Dropping in a short screencast here for reference: ​1m43

[designsimply]

Replying to valendesigns:

Expectations

• Control supports:
• Image - This should be displayed.
• Gallery - How should this be displayed in the widget, or should it even show at all?
• Audio - Should audio be playable and responsive?
• Video - Should video be playable and responsive?

I was thinking images to start, then adding gallery, audio, video, and/or whatever else later. The widget should handle all of those eventually in a smart way through the media manager as the main UI.

• Mockup the real UI for each control

I would like to rely as much as possible on the media manager for settings—core-symbiotic. :)

• Must work in the Customizer

I've got this working.

• Has PHP & JS unit tests
• Output is extensible with filters, at least at a basic level, so 3rd-party players and galleries can override the display

+1 on the rest, these are ares where I need help.

[designsimply]

Replying to peterchester:

Are you sure it makes since to make a 'media' widget instead of an image widget and a video widget and maybe an audio widget (not sure there's a lot of demand for an audio widget)?

I think it makes sense. Images will probably be the most popular type of media to add, but I think it's simpler to start at once place and then pick the type of media you want to add (image, gallery, audio, video, etc.). Starting with images only though. I see the rest getting added over time.

For example, people often want to make the image link, to have alt text, and other image specific elements. I mean I suppose we could have admin fields for all those things and find a way to apply them to all media but i'm not sure it makes sense.

You can get all of those settings through the media modal.

[designsimply]

If anyone's willing to have a look at what I've got so far, you can grab a copy at ​https://github.com/designsimply/simple-media-widget

@melchoyce, I would like to discuss the title field with you. For some reason, I broke it out of the media manager and now I'm thinking I should put it back so it's in line with your mock-ups. Hoping to catch you on Slack.

[designsimply]

Design decisions from today's chat:

• No title attribute
• "Open link in a new tab or window" should only appear if the image has a link
• Attachment display settings such as "Link To" should persist based on previously selected settings
• My feeling on showing description and caption is that it makes sense because they are in the settings, might as well have them as options for the users who do want them. But a proposal is on the table to leave them out for simplicity. Plan is to test it with users and re-visit.

[celloexpressions]

This is probably way too late for 4.3 at this point. Anyone planning to make a patch, or should we punt?

[obenland]

Let's look at this again in a future release.

[wonderboymusic]

@designsimply has shown interest in this for 4.4 - @melchoyce et al

[melchoyce]

Awesome, let's do it.

[wonderboymusic]

32417.diff​ supports images, audio, and video in the admin - still more work to do, but made a bunch of progress towards this working

[paaljoachim]

That is awesome Scott!

[DrewAPicture]

Docs + miscellaneous fixes

[DrewAPicture]

Did a docs pass in 32417.2.diff​ and fixed some other miscellaneous things including adding escaping.

[afercia]

Would recommend to use a <button> element instead of a link without any href attribute for the "Select Media" control in the widget form. As it is now, it's not even focusable with a keyboard.

[afercia]

Had a quick look, noticed a couple of things to check:

Cannot read property 'length' of undefined

mediaEl can be undefined when inserting the image for the first time, not sure about the logic here can't understand if mediaEl is supposed to be the image container or the image itself.

Uncaught Error: Syntax error, unrecognized expression: <img /

missing a trailing >

[paulwilde]

It would also make sense to have the title as the first field same with all the other core widgets.

[melchoyce]

Now that I've been thinking about this for half a year... What if we just added an "Add Media" button to the text widget?

[celloexpressions]

I'd say a "media" widget with just an add button (like the customizer media control) would probably be better - take the text out of the equation (there's also showing the preview vs. code resulting from the media modal issue). No need for any other fields inside the control except for maybe title.

[melchoyce]

Replying to celloexpressions:

I'd say a "media" widget with just an add button (like the customizer media control) would probably be better - take the text out of the equation (there's also showing the preview vs. code resulting from the media modal issue). No need for any other fields inside the control except for maybe title.

That's what I'd originally designed, but now I'm wondering if just including a media button in the text widget would make it:

1. More versatile for users
2. More familiar to users
3. Easier to implement

[wonderboymusic]

the only "hard" part is previewing the media in the widget pane, everything else is pretty basic

[DrewAPicture]

Replying to melchoyce:

Replying to celloexpressions:

I'd say a "media" widget with just an add button (like the customizer media control) would probably be better - take the text out of the equation (there's also showing the preview vs. code resulting from the media modal issue). No need for any other fields inside the control except for maybe title.

That's what I'd originally designed, but now I'm wondering if just including a media button in the text widget would make it:

1. More versatile for users
2. More familiar to users
3. Easier to implement

I think there's an importance in separating intent of the default core widgets. I actually really like the idea of just having an "Add Media" button, and would suggest to split the difference: Introduce a new "Media" widget and just have an "Add Media" button.

As @wonderboymusic implied, the big problem is really handling the preview in the widget. So maybe we can focus on that since we already have an experience-in-waiting by leveraging the "Add Media" flow users already know.

[moonomo]

Replying to wonderboymusic:

the only "hard" part is previewing the media in the widget pane, everything else is pretty basic

On 'SuperPack' plugin there's 'About' widget which do something similar. Here's the JS code which prompt media modal and adds images on the Widget:

​http://plugins.svn.wordpress.org/superpack/tags/0.2.1/assets/js/functions.js

And the PHP:
​http://plugins.svn.wordpress.org/superpack/tags/0.2.1/inc/widgets/widget-about.php

[DrewAPicture]

Relocated title + renamed file

[DrewAPicture]

32417.3.diff​ relocates the title field to match other core widgets and renames the WP_Widget_Media class file to fit the standard naming convention: class-wp-widget-*.php.

Noticed some JS errors when attempting to add media to the widget that would need to be resolved.

[helen]

#34260 was marked as a duplicate.

[Fab1en]

Fix JS + make audio/video work on the front

[Fab1en]

In 32417.4.patch​, I fixed the JS errors and make the widget actually display an audio or a video player on the front when such media is selected (previously, only the preview was working).

Here are the issues I see so far:

1. If you select a video, and then click again on "Select media" and choose an image, both image and video appear on the preview. Saving the widget solves the issue.
2. Captions does not work (do we really need it here ?).
3. Galleries and playlists do not work. We perhaps do not need to make it work here (its a "single media" widget), but we should at least remove the "Create Gallery" and "Create Playlist" menus in the media modal.
4. Change "Insert into Post" button text in the media modal to something more meaningful
5. When selecting an audio or video attachment, the "Embed or link" attachment display setting has no effect, whereas is has effect in case of an image.
6. In the admin preview, a click on the image displays the media modal, which is OK. But this behavior only works the first time.

[Fab1en]

Fix issues 1, 4, 5, 6

[Fab1en]

Here is a new patch (32417.5.patch​) that fixes several of the issues listed above.

I use a new media frame instead of reusing the "post" one which is not appropriate. So I was able to remove the left pane (with gallery and playlist links) and rename the title to "Select Media" and the button to "Use as widget". Feel free to change this to something more appropriate. (I forget to deal with the translation...)

I have also taken care of the "Embed or Link" setting for audio and video and fixed some bugs.

Could we imagine to have this into 4.4 beta 1 ?

[DrewAPicture]

Replying to Fab1en:

In 32417.4.patch​, I fixed the JS errors and make the widget actually display an audio or a video player on the front when such media is selected (previously, only the preview was working).

Here are the issues I see so far:

1. If you select a video, and then click again on "Select media" and choose an image, both image and video appear on the preview. Saving the widget solves the issue.

Probably just need to flush out the container before insert.

1. Captions does not work (do we really need it here ?).

If people are inserting images then we're going to need caption support, yes.

1. Galleries and playlists do not work. We perhaps do not need to make it work here (its a "single media" widget), but we should at least remove the "Create Gallery" and "Create Playlist" menus in the media modal.

I would expect to be able to insert a gallery or a playlist as a "media item", so I'm not sure doing away with the expected options is an improvement. If it "does not work" then maybe we need to look at making it work. Can you be more specific?

1. Change "Insert into Post" button text in the media modal to something more meaningful

"Use as Widgets" isn't great either. I think a simple "Insert Media" would suffice.

1. In the admin preview, a click on the image displays the media modal, which is OK. But this behavior only works the first time.

Might just need to refresh a nonce.

[Fab1en]

Thanks @DrewAPicture for your comments.

Issue 1. was due to the preview container : using <p> was preventing the audio and video players to be appended inside this container.
Issue 6. was due to click event bound to the image only at startup, and not after changing the image.

This media widget has been designed to support just one single media. The fields that are saved in the widget are

• widget title
• attachment id (single)
• size ("thumbnail", "large", etc, for images)
• link ("file", "post" or custom link)
• Open link in a new tab or window

If we want to use caption, galleries and playlist, we need to completely change this mechanism to store a shortcode and/or a bunch of HTML. I would be happy to go this way, but I just want to be sure that this is what everybody wants. That would mean to thrown away a lot of current patch code, and probably generate the shortcode preview via ajax.

[wonderboymusic]

Sorry

[designsimply]

Preferably, we should keep it as simple as possible and rely on existing media flow as much as possible. The simpler the better, especially to start and we can iterate from there.

[melchoyce]

I put together a slightly more fleshed out prototype people can click through: ​https://marvelapp.com/cdh5eb

[paaljoachim]

It looks good Mel!

[Fab1en]

OK, so from a technical point of view, we have to decide if the media widget allows to select more than one media (ie playlist and gallery). The current patch​ only allows a single media selection (image, audio or video).

In the case of multi-media, do we store the resulting shortcode in the widget, or an array of attachment ids ?

Storing the shortcode would allow to manage options easily (columns, image links, ...) whereas storing an array of ids would allow developers to customize the widget rendering a bit further.

[celloexpressions]

As much as I'd love core to support playlists and galleries in widgets, it's probably best to focus on single media files first so that we can get something in.

It looks like we're already past this point, but the UI for WP_Customize_Media_Control supports previews of some sort (in some cases just the type icon and title) for any media type, so we may be able to reuse some of that code. It does leverage the underscore.js template framework for customizer controls, though, so it may not be an easy direct port. The strategy of non-imgage/video/audio handling would probably work well though, with a simple link being rendered on the frontend.

[melchoyce]

Replying to melchoyce:

... I'm wondering if just including a media button in the text widget would make it:

1. More versatile for users
2. More familiar to users
3. Easier to implement

The more I think about it, the more I just want a media button in the text widget.

[Fab1en]

I agree that it would be more convenient for users to have a media button in the text widget.

However, a question immediately comes up : once the media (or gallery, or ...) is selected, how to show it in the widget admin area ? Current text widget is a single textarea without any rich edition capability, so the media will either look like a bunch of HTML, or like a raw shortcode.

To have the media preview in the backtend, we need to bring-in the full TinyMCE editor. And if we do this, text widget will become "rich text widget". Also, widget content is not filtered to display shortcodes, so modifying this behavior would introduce back-compat issues.

And we haven't say a word about Customizer integration ...

[westonruter]

Replying to Fab1en:

To have the media preview in the backtend, we need to bring-in the full TinyMCE editor. And if we do this, text widget will become "rich text widget". Also, widget content is not filtered to display shortcodes, so modifying this behavior would introduce back-compat issues.

See #35243 (Extending the text widget to also allow visual mode).

And we haven't say a word about Customizer integration ...

See #35760 (Provide API for TinyMCE editor to be dynamically instantiated via JS).

[celloexpressions]

I feel like even if we add rich editing to the text widget, media should remain a distinct thing. If we think about widgets like "content blocks", which they really are in a lot of ways, introducing all of the major functionality into one type of widget will likely go too far into the direction of using a single widget for everything, and then being disappointed that you need to split than content into another widget to add something from another widget somewhere in the middle of it.

Widgets are designed to be modular, so we should keep their uses distinct. A media widget is a much simpler concept than trying to add media capabilities to the text widget. I'd essentially like to see WP_Customize_Media_Control as a widget, and that code could probably be adapted pretty easily especially if the JS-templated preview can be carried over (although it would be done in PHP for the front end I suppose).

Aside: I'd also like to see an embed widget that does oembeds, rather than putting that functionality into the text widget.

[krogsgard]

Replying to celloexpressions:

I feel like even if we add rich editing to the text widget, media should remain a distinct thing. If we think about widgets like "content blocks", which they really are in a lot of ways, introducing all of the major functionality into one type of widget will likely go too far into the direction of using a single widget for everything, and then being disappointed that you need to split than content into another widget to add something from another widget somewhere in the middle of it.

Widgets are designed to be modular, so we should keep their uses distinct. A media widget is a much simpler concept than trying to add media capabilities to the text widget.

From a user experience perspective, this is also quite logical. Text widgets have long been a dumping ground but that doesn't mean they should be a dumping ground.

It makes sense from a user perspective to go to the customizer widgets screen and say, "Add an Image" or "Add a video" or whatever. Trying to turn the text widget into a tiny site builder interface without breaking lots of stuff seems unrealistic and probably a bad user experience without lots of work.

The idea of an "Add Media" button or visual editor in the text widget has merit, but it doesn't stop a need for a simpler implementation for just adding an image like the sample plugins in this thread show. Any time I build a tiny little website for someone, putting a little "About {Person}" widget is on the agenda. Ditto for folks that want small calls to action or whatever else. This is a use case for millions of websites.

Image attachments already offer a framework for available options with captions and description (that can be used for description text in a case like my "About" widget). With a link field added, the framework is there to solve a lot of use cases.

Is there a way to move forward on this without rethinking every single thing about widgets and having to deal with a 7 year old stagnated issue of #10457?

[georgestephanis]

Not to throw another wrench in the works, but there's likely to be a number of folks who want to embed an image or media that isn't in their media library -- by url.

The Jetpack image widget supports that, but I'd certainly love to see a better ui for it -- maybe one that prepopulates all the fields by a selected media library item, and then the user can just tweak text fields if they like to customize or change urls? Unsure.

​https://github.com/Automattic/jetpack/blob/master/modules/widgets/image-widget.php
​https://jetpack.com/support/extra-sidebar-widgets/image-widget/

I just have this nagging fear that something as 'simple' as an Image Widget will become so overly complicated that it won't be mergeable.

[melchoyce]

Replying to georgestephanis:

Not to throw another wrench in the works, but there's likely to be a number of folks who want to embed an image or media that isn't in their media library -- by url.

The media library already handles that scenario. See attached image.

[Fab1en]

I would be happy to continue the work on this ticket. I don't see exactly what I, as a developer, can do to make this Media Widget happen for 4.7

[ramiy]

Any chance this can be wrapped in a separate function called wp_media() the same way the TinyMCE editor is wrapped in wp_editor()?

This way, WordPress core and plugin developers can use this function in Widgets, in Meta-Boxes, Setting Pages and other places. A separate function can be used as a new method to include the WordPress media component.

[melchoyce]

What can we do to get this moving again?

[westonruter]

@melchoyce Is this entirely needed if the text widget (#35243) allows for media to be embedded?

[melchoyce]

Replying to westonruter:

@melchoyce Is this entirely needed if the text widget (#35243) allows for media to be embedded?

Waffling. I still think there's a use case for just having a media widget, especially because it makes it more discoverable. It's a little redundant, but I don't know that it's redundant in a damaging way. I'd prioritize the text widget if media is guaranteed to be included, but prioritize this if media inclusion in the text widget is still up in the air.

[karmatosed]

I would second that if the text widget includes media prioritising makes sense. I do think this is something I've seen come up again and again in user testing. People want it, they expect it.

[RDall]

This would be such a huge win for people who put a lot of media in their sidebar.

The vast majority of users I have seen use ​Black Studio's famous Visual Editor just for image placement alone. This widget would remove that extra step work around that a lot of users don't know about and "I wish I knew this six months ago" I have heard more then a few times would also be reduced.

This would also be incorporated into the customizer visual editing as well. So users could see the image in the sidebar before publishing?

[westonruter]

Replying to RDall:

This would also be incorporated into the customizer visual editing as well. So users could see the image in the sidebar before publishing?

Yes. The user would see the image not only in the TinyMCE editor in the sidebar widget control but also in the preview as well as it would appear on the site (in a widget area) once the changes are published.

[celloexpressions]

We need to be careful with adding rich media functionality to the text widget if there is a goal to integrate widgets and shortcodes as content block that can be used in content areas and sidebars. Introducing a widget that mixes plain text and multimedia content and can contain multiple "blocks" within one widget could become a backward-compatibility nightmare in the very near future.

Any approach other than a standalone media widget that serves as a single media "block" (which could potentially facilitate different types of blocks, but one block per instance) needs serious vetting against proposed widget+shortcode and content block implementations.

[westonruter]

​https://github.com/xwp/wordpress-develop/pull/215

[westonruter]

Refreshed patch in 32417.6.diff​.

Opened PR on GitHub for collaboration and testing: ​https://github.com/xwp/wordpress-develop/pull/215

New changes in PR:

• Trigger change event on attachment ID input to cause customizer preview
• Clean up php and js code style
• Opt-in WP_Media_Widget to use selective refresh

[melchoyce]

After reviewing this ticket again, I think it's a good example of scope creep and overthinking a feature. I stalled it out by talking about adding a media button to the text widget (which I still think is a good idea) We even recently prioritized this over the media widget in a ​Customizer meeting, but I actually think we should launch a standalone media widget with the work started here. I prioritized getting everything in over getting something in and I'm starting to see that was the wrong approach. Apologies.

So, let's review where we're at and the pros/cons of releasing a v1:

Pros:

• We'll get something out sooner, maybe even in a point release?
• The most recent patch is a good v1. It is better than what we have now, which is nothing.
• It also fits within our goal of "quick wins" for Customization in the next couple of months.

Cons:

• The widget won't include galleries, playlists, or captions in the first release.
• The widget won't include adding an image, video, or audio file from an external URL in the first release.

I think it's okay to keep the scope small for now, and worry about galleries, playlists, and captions once the work in #35760 has moved forward more.

I reviewed the current patch and took some screenshots here: ​https://cloudup.com/cPOxeaSOdpM

While reviewing, here were some improvements I jotted down:

• When you add a new image, video, or audio file, it should display in the widget above the "Select media" button, not below it. (See: Header Media)
• "Select media" should change to "change media" once you've make your initial media selection. (See: Site Icons)
• "Use as widget" still isn't ideal. Maybe "add to widget" or "add to sidebar?"
• When you add an image to the widget, the widget's title in the widget meta box seems to be using the image title, rather than the title entered into the title field within the widget. This doesn't happen to audio or video. (You can see this in the image widget screenshot)
• "Open link in a new tab or window" doesn't make sense for something without a link. This checkbox should probably only appear within the media modal itself, if you've opted to add a link to the image or link directly to the video or audio file instead of embedding. (Brought up in https://core.trac.wordpress.org/ticket/32417#comment:30).
• Downloading seems to be on by default for audio and video — we should probably turn this off entirely. When I add media to my sidebar, I don't expect to provide an easy way to download that media (though I understand it's possible).
• Videos aren't responsive by default and break the layout. Can we do something in core to make them display within the confines of the widget in which they are being displayed?
• Video controls overlay the video itself, and don't seem to disappear after a while. They should display underneath the video. Is this how core handles video embeds or just a quirk with the widget?
• Can we remove "caption" from the media modal since we can't display captions yet?
• Maybe a bug: When uploading an audio file, I changed its display "embedded media player" to "link to media file." I added the file, then clicked into the media modal again, and the dropdown swapped back to "embedded media player."
• The widget says "add an image, gallery, video, or audio to your sidebar," but the current iteration can't support a gallery yet. We should exclude gallery from this text.
• Let's update the widget's description to be something more like "Display media (image, video, or audio) in your sidebar" or "Display media such as images, video, or audio in your sidebar," so it's easier to search for within the Customizer widget panel.

Let's get this moving again. Thanks to everyone who's been participating over the past 21 months.

[gonom9]

I just attached a new patch that based on @westonruter's patch.

Changes in new patch:

• "Select Media" button shows up below the media preview.
• "Select Media" changes to "Change Media" once you've added media to the widget.
• "Use as widget" button text in the media browser is changed to "Add to widget"
• "Open link in a new tab or window" option is removed.
• The instruction in the widget form: "Add an image, video, or audio to your sidebar."
• The widget description: "Display media such as images, video, or audio in your sidebar."

[afercia]

"Open link in a new tab or window" doesn't make sense for something without a link.

Yep, a bit pointless even when the media is not embeddable, since clicking on the link will just download the file and target="_blank" either gets ignored by browsers or opens a new tab/window just to download a file...

Also, the usage of `target="_blank"` is under discussion and some work has already been done in that regard, admittedly mainly for the admin. Maybe opening new tabs/windows shouldn't be encouraged.

A few things noticed while checking the 2 latest patches:

• Accessibility: the "Select Media" button is not even focusable, it's just an a element without a href attribute: <a data-id="wp-media-widget-2" class="button select-media widefat">Select Media</a>; it should be a real button element
• Usability: there are no instructions that, when clicking on an image preview, users can change the media; see for example how the featured image meta box solves this with a hint "Click the image to edit or update" which is targeted by aria-describedby on the preview link. However, when the media preview is a video or audio, clicking the preview plays the audio or video so... the description (and the aria-describedby) should be used conditionally depending on the media type
• Both a11y and usability: in the preview, not embeddable audio/video media get a link with a href="#" attribute that serves no purpose (?); from an accessibility perspective it should be avoided since it's a non-link; from an usability perspective, maybe better to just display the title and a message below, something like "this media type can't be embedded in the page, a link to the file will be used instead" ?
• Usability: the media modal doesn't show the select to filter media by type, not sure if it can be added easily
• Usability: in the modal, I can actually select a .pdf, .doc, .zip, or other files types and nothing happens: ideally, the media modal should filter the attachments in order to show only the allowed ones. Or, at least use a warning when trying to insert: "this media type is not allowed"
• JavaScript: not translatable strings, e.g. 'Use as widget', 'Select Media'. Not sure about the usage of 'Change Media' as sort of fallback
• JavaScript (minor): many properties names don't need quotes, only reserved names and names with hyphens need them, e.g. 'class' or 'data-id'
• PHP: most of the uses of extract() were removed from core, no other widgets uses it. I'd recommend to avoid to introduce new ones. See #22400.
• PHP: not sure why hidden fields should be inside a <fieldset> element, which is a semantic element used in this case for elements that are not perceivable
• PHP: escaping should be consistent, for example why _e( 'Add an image, gallery, video, or audio to your sidebar.' ) and then esc_html_e( 'Select Media' )?
• PHP: not sure ternaries can be used inside translation functions: esc_html_e( $attachment ? 'Change Media' : 'Select Media' ); while this may work in PHP, I'm not sure such strings can be correctly extracted by the i18n tools /cc @ocean90
• Coding Standards: there's the need for some love, minor issues like spaces (see for example all the occurrences of if( and ('. Also (function ( $ ) { should be ( function( $ ) {
• Minor: maybe this comment should be removed? // @todo Variable is not used.
• Reminder: @since notations should be updated

[gonom9]

Thanks to @afercia the code look neater. :)

Changes in new patch:

• Add support for image caption
• Clean up PHP and JS code style to meet the WP coding standard.
• Update @since notations
• A11y: "Select Media" becomes a real button.
• Usability: show media type filters in the media modal.
• JS: all strings are translated.
• PHP: no longer use extract()

[karmatosed]

@gonom9 how easy is it to get this bundled to run some user tests? Ideally if it could be in a plugin format (no clue if asking impossible here). @melchoyce also looping you in for when you feel it's ready for that, totally only want to test when it's worth doing.

[westonruter]

@karmatosed it's straightforward to make it into a plugin, as I've done in this gist: ​https://gist.github.com/westonruter/854aba61243ef08a2e3a2a0e721930dc

We could add it as a ZIP here for users to install on their sites, except before we do that it seems that there is an issue with the selected media not appearing in the control once selected. I'm seeing this at least on the widgets admin screen.

@gonom9 Could you continue to iterate on the Gist plugin I made out of your patch? Also, eventually the patch will need to define the scripts and styles inside of script-loader.php and so the way that the plugin uses wp_default_scripts and wp_default_styles for registering is an anticipation of that.

[karmatosed]

@westonruter that's great! Once the issues are resolved that would be awesome.

[gonom9]

Changes in 32417.10.diff:

• Add support for responsive video, audio, and images. Don't forget to check "Scale to fit width"
• Updated ​a plugin for testing as well. @karmatosed

---

@westonruter Great work! I also updated the gist on my account. The script-loader.php will be addressed in next patch. Thanks. :)

---

Can anyone tell me the best practice to add ​small JS code to preview? The code will turn a normal <video> or <audio> element into an interactive media element after partial updates. I originally added the code using wp_add_inline_script() and it seemed to work well but I'm not sure if it's a right way. Would it be better to write the code in a separate file?

[gonom9]

Fix typos. Nothing else is different from 32417.9.diff

[westonruter]

Replying to gonom9:

Can anyone tell me the best practice to add ​small JS code to preview? The code will turn a normal <video> or <audio> element into an interactive media element after partial updates. I originally added the code using wp_add_inline_script() and it seemed to work well but I'm not sure if it's a right way. Would it be better to write the code in a separate file?

In the plugin form, what you have is good. However, for the core merge this logic should actually be part of selective-refresh.js itself as was done for emoji, toward the end of this snippet: ​https://github.com/WordPress/wordpress-develop/blob/4.7.2/src/wp-includes/js/customize-selective-refresh.js#L425-L454

Now, I believe an early iteration of selective refresh actually did include auto-initialization of media elements. However, it was removed due to complexities in regards to initialization which can be seen in the Jetpack infinite-scroll module. There may be other use cases in the Jetpack module that can be gleaned from for various edge cases: ​https://github.com/Automattic/jetpack/blob/master/modules/infinite-scroll/infinity.js

See also this PR which hooked up Jetpack's Infinite Scroll with Selective Refresh: ​https://github.com/Automattic/jetpack/pull/3542

Nevertheless, all of the logic for lazy-loading media element JS probably should be eliminated in favor of the Media Widget itself enqueueing the assets whenever the widget is active (or in the customize preview). In other words, the WP_Media_Widget::__construct() method can include:

<?php
if ( is_customize_preview() ) {
        wp_enqueue_style( 'wp-mediaelement' );
        wp_enqueue_script( 'wp-mediaelement' );
}

The reason for doing this only in customizer preview is for the sake of adding the first instance of the media widget to the page. Otherwise, when on the frontend and there is a media widget in a sidebar, it will have already called wp_audio_shortcode or wp_video_shortcode and thus the required assets would have already been enqueued and available for use.

[sirbrillig]

Nice work pushing this along! I did a quick scan of the JS code for the widget. It looks good! I do have a few mostly minor comments:

• Why does binding the click event for the media image have its own click handler (frame.bindImageClick) and the click event for the button does not? Presumably because the image needs to be re-bound during renderFormView, but we might as well be consistent.
• Similarly, why does the button have its selector stored in an object property, but the image does not? Presumably this is because the button selector is used elsewhere, but again it might be worth having both be properties or, if the selectors should never be overwritten, just use accessor methods to get them; something like getMediaButton().on(...) and getMediaImage().on(...).
• Why is the reference to widgetFrame stored at the top-level of the IIFE rather than within openMediaManager? It's not used anywhere else and it's always overwritten when openMediaManager is called.
• I don't know much about wp.media, but is there a possible memory leak since we are binding functions to the events open and select and then re-creating the object each time the frame is made visible? Or are those event handlers automatically removed when the media frame is hidden?
• This line ids = $( '#widget-' + widgetId + '-id' ).val().split( ',' ); will throw an error if the target cannot be found on the page. How about something like idGroup = $( '#widget-' + widgetId + '-id' ).val() || '', ids = idGroup.split( ',' );?
• This conditional is confusing to me: if ( ids[ 0 ] > 0 ) { ids.forEach(. Why check the integer value of the first element of the widget, then proceed to iterate over the array? Could we just iterate over the array regardless? If the concern is that we should ignore 0 IDs, could we just ignore them in the loop?
• Missing a @return jsdoc line for renderMediaElement.
• The translate() function uses window._mediaWidgetl10n followed by a bare reference to _.medaiWidgetl10n. It seems like we should be consistent here. I suggest passing window._mediaWidgetl10n as an argument to the IIFE so it can be used without window inside.
• renderFormView uses jQuery to find formView and scale but doesn't account for the possibility that those elements might not be found. Should we return early?
• Although it is clever, using a ternary property reference to call the appropriate class manipulation method is not obvious to me and may confuse other developers. formView.find( '.attachment-description' )[ attachment.description ? 'removeClass' : 'addClass' ]( 'hidden' ) could instead be written as formView.find( '.attachment-description' ).toggleClass( 'hidden', ! attachment.description ).html( attachment.description );
• Since we are using underscore.js anyway, why not use _.contains() instead of $.inArray() since it already returns true/false, removing the need to compare to -1?
• else if can sometimes be confusing when trying to follow the flow of code, leading to hard-to-find bugs. Could we replace it with a plain condition, so that } else if ( 'image' === attachment.type ) { becomes if ( ! _.contains( [ 'audio', 'video' ], attachment.type ) && ( 'image' === attachment.type ) ) {?
• Could each media type in renderMediaElement be its own function to make that function smaller and easier to read? We could even use another function to figure out which one to call; something like return getMediaRendererFor( attachment.type )( widgetId, props, attachment ).

[sirbrillig]

And here's a few notes on the otherwise excellent PHP in WP_Media_Widget:

• $selectedLink should be snake-case, like $selected_link.
• There are a few cases where we see .'</a>', which is missing a space between the concat operator and the string.
• Do any instances of concatenated output need to be escaped in widget()? Such as align and description here: '<p class="attachment-description align' . $instance['align'] . '">' . $instance['description'] . '</p>'? Or $title just above that? Maybe that's all handled when data is saved, but it's unclear here.
• Very minor but you could save a few lines by removing the final else clause when assigning $selected_link and setting an initial empty value (or just allowing an undefined value since it is checked with empty() everywhere it is used, but I always like using defined variables). In addition, currently $selected_link is undefined if $instance['link'] is empty, so if we want to use an initial value it should be set outside of any conditions.
• Inside widget(), there's two nested conditionals if ( ! empty( $instance['id'] ) ) { if ( $attachment = get_post( $instance['id'] ) ) { that could be one condition instead to reduce indentation: if ( ! empty( $instance['id'] ) && $attachment = get_post( $instance['id'] ) ) {.
• Similar to my suggestion for the JS, generating markup from the the different media types in widget() could be split so that each has its own function, making widget() smaller and easier to follow.
• There's two semi-colons after the first line of render_audio().
• Why do the private methods get_attachment_audio() and get_attachment_video() accept an attribute array, but not use it for anything?
• get_responsive_style, get_attachment_image, get_attachment_audio, and get_attachment_video are missing a @return docblock.

[gonom9]

Just cleaned up the code style. No features are added.

[gonom9]

@westonruter how about adding back auto-initialization of media elements to selective-refresh.js, but this time only for widgets?

---

@sirbrillig Thanks for the A LOT OF minor things. I almost entirely refactored PHP and JS codes.

[westonruter]

Replying to gonom9:

@westonruter how about adding back auto-initialization of media elements to selective-refresh.js, but this time only for widgets?

I think it would be beneficial to add auto-initialization of media elements generally for any selective refresh partial, though there could be some cases where it might initialize a media element with ME.js against the wishes of the theme/plugin author. But this would also be the case for the media widget itself currently, right? It doesn't seem to be taking into account filters added for the wp_audio_shortcode_library or wp_video_shortcode_library filters which allow plugins to override the library used for initializing media elements. To properly account for these, it would seem that you should include something like:

<?php
$scripts->localize( 'wp-media-widget', 'wpMediaWidgetSettings', array(
        /** This filter is documented in wp-includes/media.php */
        'audioLibrary' => apply_filters( 'wp_audio_shortcode_library', 'mediaelement' ),
        /** This filter is documented in wp-includes/media.php */
        'videoLibrary' => apply_filters( 'wp_video_shortcode_library', 'mediaelement' ),
) );

And then in the JS for when the partial is refreshed that contains audio media it should only call initialize ME.js if 'mediaelement' === wpMediaWidgetSettings.audioLibrary. There may be a better JS object to attach these settings to rather than create a new global variable wpMediaWidgetSettings.

If it is only supposed to be for widgets and not generally, then the logic could be added to customize-preview-widgets.js.

But yeah, I think it would be best to make it apply to all partials that are being refreshed, and so make it part of selective-refresh.js and so perhaps the settings could be exported via:

src/wp-includes/class-wp-customize-manager.php

@@ -1807 +1807 @@
                                 'stylesheet' => $this->get_stylesheet(),
                                 'active'     => $this->is_theme_active(),
                         ),
+                        'media' => array(
+                                /** This filter is documented in wp-includes/media.php */
+                                'audioLibrary' => apply_filters( 'wp_audio_shortcode_library', 'mediaelement' ),
+                                /** This filter is documented in wp-includes/media.php */
+                                'videoibrary' => apply_filters( 'wp_video_shortcode_library', 'mediaelement' ),
+                        ),
                         'url' => array(
                                 'self' => $self_url,
                                 'allowed' => array_map( 'esc_url_raw', $this->get_allowed_urls() ),

And you'd be able to access these via wp.customize.settings.media.audioLibrary and wp.customize.settings.media.videoLibrary.

[gonom9]

Changes in 32417.12.diff:

• Updated wp-media-widget.js to follow jshint-style.
• <video> and <audio> are auto-initialized after selective refresh.

@melchoyce The download icon never show up.

---

@westonruter Thanks for all of your advice. I was hoping you could review my latest patch.

[melchoyce]

Some notes from testing the latest patch:

• Getting an error: Notice: Undefined index: align in /srv/www/wordpress-develop/src/wp-includes/widgets/class-wp-widget-media.php on line 81
• Still seeing a download button on videos (though not audio anymore): ​https://cloudup.com/cKZaX_kri8j
• Some (not all) of the mp3s I'm trying to upload are telling me, "Sorry, this file type is not permitted for security reasons." Any ideas why?
• Any reason why we can't just apply "scale to fit width" automatically and not make it an option at all?
• I was able to select a PDF to add to the widget (which didn't work once I added it, unsurprisingly). Can we hide unsupported media types when you open the media library to choose a file for the widget?

[gonom9]

@melchoyce
The upload issue is filed as #39550. All of the other bugs are fixed in 32417.13.diff.

Any reason why we can't just apply "scale to fit width" automatically and not make it an option at all?

I added it as an option because I thought someone may want to show media in a specific size.

---

Never forget ​the plugin. :)

[melchoyce]

Replying to gonom9:

@melchoyce
The upload issue is filed as #39550. All of the other bugs are fixed in 32417.13.diff.

Ah, thanks. :)

Any reason why we can't just apply "scale to fit width" automatically and not make it an option at all?

I added it as an option because I thought someone may want to show media in a specific size.

Can we change it to apply automatically to videos, and remove the option entirely?

[gonom9]

The scale option is removed and all media will be responsive by default.

[westonruter]

@gonom9 I applied your latest patch to the previous pull request I prepared: ​https://github.com/xwp/wordpress-develop/pull/215

If it works for you, it would be great for collaboration if you'd be able to push up subsequent changes to the pull request itself. I added you as a writer to the repo.

I want to amend the patch with some additional changes, but I don't want to step on your toes and have to manually resolve conflicts between patch files.

[gonom9]

Replying to westonruter:

I want to amend the patch with some additional changes, but I don't want to step on your toes and have to manually resolve conflicts between patch files.

Please do it. The PR works great. I'll continue to work on the PR if I have any. :)

[melchoyce]

Just ran through the latest patch on all default themes. It looks good! Check out the gallery here: ​https://cloudup.com/cXEWkWKwUIN

There were only two places where it might be weird:

1. The dark color scheme clashes a little with Twenty Thirteen's footer background color (​https://cloudup.com/i676cwYz-zs)
2. Twenty Fourteen's Primary Sidebar is very thin, so the controls get awkwardly pushed outside of the container (​https://cloudup.com/iuc3brESgnG)

[westonruter]

Δ ​https://github.com/xwp/wordpress-develop/pull/215/files/cc2f420..a63e90e

[celloexpressions]

I made a few fixes on GitHub, along with some technical review comments. Here are a few usability questions I have/things I didn't want to change in the patch yet:

• Most default themes (and as a result, likely other themes) that implement custom styles for the MediaElement players scope their CSS to .hentry to have higher specificity than the MediaElement styling provided by WordPress. This custom styling needs to apply to widgets too, ideally without making theme changes. This is a blocker for shipping I think, and if theme changes are required I'm not sure that we should release this feature in a minor release (unless theme outreach happens for a period of time prior). There are a few options here:

1. Add a .hentry class to the media widget container for all of them. Not semantic with the intent of that class and might risk causing other issues if themes use that class for other styling.
2. Finally fix the core-provided and default MediaElement styles to load before theme styles or somehow have lower specificity so that a container isn't needed. However, themes that scope to .hentry would still need to be updated to removed that.
3. Update all bundled themes' CSS for custom MediaElement styles (may include color schemes in some cases) to also style media widgets. Start substantial theme developer outreach regarding this requirement (I know I've done this in several themes, there are likely many others). Unfortunately, this is something that was a pretty bad idea in terms of how themes should have done things, but this is the route that bundled themes suggested around the time that Audio and Video support were added to core.

• Not all bundled themes have the above issue; I quickly found these: ​https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentyfourteen/style.css#L1348, ​https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentythirteen/style.css#L1229, ​https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentyseventeen/style.css#L2801.
• There is partial, broken support for file types other than images, audio, and video. I'd suggest falling back to an assumed type of "document", linking the title to the media file (and using the file name if the title is blank). For PDFs we should also be able to show a thumbnail and link that with the title/filename being optional, for sites that support PDF thumbnails as of 4.7. I would guess that PDFs would be used more than video here, even, so we should support that since we can fairly easily. If nothing else we need to show an error message if we don't/can't support a particular file type at all.
• FYI I added an icon and updated the widget description to better align with other core widgets - see screenshot. The menu widget is an exception here, as well as with its naming, apparently. We might want to fix that while we're at it.
• Do we need the text "Add an image, video, or audio to your sidebar." in the actual widget controls? That seems redundant and works against Strive for Simplicity.
• Seeing as we have clickable placeholders, and #34323 is still unresolved, we need an accessibility review still, cc @afercia. (for what it's worth I think the approach for clickable placeholders here is appropriate for all users)

[afercia]

Replying to celloexpressions:

• Seeing as we have clickable placeholders, and #34323 is still unresolved, we need an accessibility review

Uh, apparently I missed that. Can't see a clickable placeholder, @celloexpressions could you please guide me a bit? :)

Some additional issues:

• this point from my previous comment is still unresolved: "Usability: there are no instructions that, when clicking on an image preview, users can change the media". It would be a nice improvement to do something similar to what the featured image meta box does.
• image preview: ti's a clickable <img> element so it won't be listed as an interactive control by assistive technologies. The featured image meta box instead, wraps the image in a link which is not ideal but it's better
• image: WordPress doesn't set the title attribute on images, I'm not sure why the media widget should do that. Also, in the last months we've been removing title attributes all over the places since they don't add any great value. I'd suggest to don't use title attributes

[karmatosed]

Overall, this feels like a great, useful thing for users. I can see them really finding this solves a problem and a resolves a common need to install something extra.

I did some testing and here is my feedback (I tried to focus on things not already reported):

• Changing the title and caption of the image didn't reflect in the widget until I changed the title itself of widget

• I love they are responsive but I can created a 200px wide image, uploaded it and it became very stretched - even worse on tablet sizes:

Is there any way we can stay true to the original size?

• There's a weird difference in behaviour between images and video. If I click an image it takes me to change the media, if I click a video it plays. This is in the widget sidebar here:

I would suggest remove ability to play video here and make it that you can play in preview. Otherwise it's a confusing experience having 2 different behaviours.

• Above said, if we do keep clicking to change an image, I also have queries about it's discoverability. It's hidden unless you actually do click.

• I would very much suggest we keep 'Add an image, video, or audio to your sidebar'. It's well explained and will benefit users.

• I tested all default themes myself and agree with @melchoyce's comments. Here is a visual record using both a video and image, to add to Mel's:

[gonom9]

Replying to celloexpressions:

• Most default themes (and as a result, likely other themes) that implement custom styles for the MediaElement players scope their CSS to .hentry to have higher specificity than the MediaElement styling provided by WordPress. This custom styling needs to apply to widgets too, ideally without making theme changes. This is a blocker for shipping I think, and if theme changes are required I'm not sure that we should release this feature in a minor release (unless theme outreach happens for a period of time prior).

It sounds like we should assume every theme developers want to use the same style of media elements on both content and widget area. But we don't assume that for any other widget content such as menus and links.

[gonom9]

Fix: Changing title and caption of the image don't reflect in the widget

[gonom9]

@karmatosed Fixed the bug of ignoring changes of title and caption in both 32417.16.diff and ​the plugin.

[afercia]

Replying to karmatosed:

• There's a weird difference in behaviour between images and video. If I click an image it takes me to change the media, if I click a video it plays.

Yep, I'd totally second some improvements here. See also second point on #comment:97

• Above said, if we do keep clicking to change an image, I also have queries about it's discoverability. It's hidden unless you actually do click.

+ 1

[westonruter]

Replying to gonom9:

@karmatosed Fixed the bug of ignoring changes of title and caption in both 32417.16.diff and ​the plugin.

@gonom9 Is this the right approach? Is this not resulting in the title and caption being duplicated in the widget vs. what is stored in the attachment? To eliminate that duplication I reverted ​that commit and added a ​new one which takes a different approach to explicitly refresh the widget partial in the preview whenever the form is re-rendered. Note that this doesn't account for the case where a user may change an attachment's caption but then close the modal without hitting the Add button.

Really this is starting to touch on #37887 where there is a problem previewing with the media modal. When a change is made in the media modal (such as to the caption) the change is written to the DB immediately. This will be somewhat unexpected in the customizer context because everything should be previewed until hitting Save & Publish.

[gonom9]

Replying to westonruter:

@gonom9 Is this the right approach? Is this not resulting in the title and caption being duplicated in the widget vs. what is stored in the attachment? To eliminate that duplication I reverted ​that commit and added a ​new one which takes a different approach to explicitly refresh the widget partial in the preview whenever the form is re-rendered.

No, your approach is much better than mine. I just learned the API for refreshing partials. Thanks. :)
But the latest version was still ignoring the changes in the attachment so I made ​another commit. Also, I think wp.customize.Widgets should be wp.customize.previewer in ​the previous commit.

[gonom9]

Added ​another commit which handles the case that user doesn't click on the Add button.

[gonom9]

Explicitly refresh widget partial when attachment has any changes.

[celloexpressions]

get_responsive_style() is setting a widgth: 100% on everything, which is causing stretching. Most themes should already have img, video { max-width: 100%; }, and we could also add styles for that scoped with widget instances from the core side, so this function should be removed entirely as I suggested on GitHub.

The UI controls here should be consistent with WP_Customize_Media_Control to provide a consistent user experience. That means:

• There should be a visual placeholder when no media is selected.
• The images shouldn't be clickable (this is what I was referring to @aferica).
• The media should be playable within the widget controls.
• The text within the widget controls needs to be removed.
• I don't think we should bring the remove button over since you can delete the widget, so that part would be different.

There's no need to reinvent the wheel and introduce something different here. It would be nice to make improvements here, such as clickable placeholders, but if we do that we should update the other media UI controls to match in the process.

Themes are encouraged to style MediaElement players. Usually this is done by customizing the colors and/or icons. There's no reason that that shouldn't also apply to widget by default, just like your global theme link colors will also apply to widgets unless you set a different link color in widgets with your CSS. Themes could always customize widgets further, but by default they should match media in content. At a minimum we should make this fix in bundled themes and give theme authors ample notice that they may need to update to account for this. Otherwise, sites will have oddly-inconsistent results.

The latest version of the patch has several JS errors that I can quickly tack down so I can't test and iterate further right now. Once that's fixed I can look at improving the fallback media type behavior.

[afercia]

Replying to celloexpressions:

• There should be a visual placeholder when no media is selected.
• The images shouldn't be clickable (this is what I was referring to @aferica).

Oh yep. I'd say either the image should be a usable and accessible control or it shouldn't be a control at all. However, consistency with the featured image meta box is a concern too.

[melchoyce]

I need to catch up on the recent convos, but just wanted to mention I tested some plugins alongside the patch and didn't seem to have any conflicts. I tested:

• Black Studio TinyMCE Widget by @black-studio
• Image Widget by @moderntribe
• Image Widget Deluxe by @mrommel
• Really Simple Image Widget by @rabmalin
• Simple Image Widget by @cedaro
• Swifty Image Widget by WPGens by @goran87
• Webdoone Simple Image Widget by @webdoone
• WP Canvas - Image Widget by @cbaldelomar
• WPB Image Widget by @wpbean

Note: a couple of the widgets didn't work or had errors. I tested those plugins on another test install without the media widget patch applied, and they had the same errors.

[gonom9]

Replying to celloexpressions:

get_responsive_style() is setting a widgth: 100% on everything, which is causing stretching. Most themes should already have img, video { max-width: 100%; }, and we could also add styles for that scoped with widget instances from the core side, so this function should be removed entirely as I suggested on GitHub.

I don't think video { max-width: 100% } is enough to make the video element responsive because it changes into a complex composite element with mediaelement.js library. Instead, .wp-video { max-width: 100% } or something is required for responsive video but there is no default theme which has that style. We need to add it. For images, however, you're right. I'll remove the styles for images.

The UI controls here should be consistent with WP_Customize_Media_Control to provide a consistent user experience. That means:

• There should be a visual placeholder when no media is selected.
• The images shouldn't be clickable (this is what I was referring to @aferica).
• The media should be playable within the widget controls.
• The text within the widget controls needs to be removed.
• I don't think we should bring the remove button over since you can delete the widget, so that part would be different.

I totally agree with you here. I will update my patch with these improvements.

The latest version of the patch has several JS errors that I can quickly tack down so I can't test and iterate further right now. Once that's fixed I can look at improving the fallback media type behavior.

Could you give me more details about the error?

[gonom9]

Instead, .wp-video { max-width: 100% } or something is required for responsive video but there is no default theme which has that style. We need to add it.

@celloexpressions Never mind. I've already done it.

[gonom9]

Changes in 32417.18.diff:

• Remove the widget-specific responsive styles. The get_responsive_styles() has gone. @celloexpressions
• Remove click handler from the image. No media preview is clickable. @afercia @karmatosed @celloexpressions
• Show placeholder text in empty widget preview. @celloexpressions
• Fix JavaScript errors. @celloexpressions

As always, ​the plugin has been updated for testing. @karmatosed

[rabmalin]

I was just trying the plugin. I tried to add image and add custom link for the image. But when widget is saved, that link settings is not applied to the front end. I am talking about Link To in attachment modal.

[westonruter]

Media Modal Image Attachment Display Settings

[westonruter]

Media Modal Video Attachment Display Settings

[westonruter]

Media Modal Edit Already Inserted Image Attachment Display Settings

[westonruter]

Media Modal Edit Already Inserted Video Attachment Display Settings

[westonruter]

Proposed integration of (relevant) attachment display setting fields into media widget form

[westonruter]

Proposed inclusion of edit button in widget form to open relevant media details modal

[westonruter]

Replying to rabmalin:

I was just trying the plugin. I tried to add image and add custom link for the image. But when widget is saved, that link settings is not applied to the front end. I am talking about Link To in attachment modal.

We just had a ​conversation about this topic in #core-customize, about what to do with the “Attachment Display Settings” that appear in the media modal, for example the settings that appear for an image and a video respectively are:

Two main questions arose from the discussion:

Are all of the display settings relevant for the media widget?

Are all of the display settings relevant for the media widget?
For images, should is alignment relevant? (Personally, I do not think so.)
For audio/video should there be the option to display a media link instead of embed the player? (IMHO, no.)
Does the premise of the media widget include linking to media in addition to just displaying media?

Note that per below, these attachment display settings shown in the media selection modal is actually a subset of all of the fields that a user can specify when editing the modal.

Where should the attachment display settings appear?

As it stands currently, the media widget seeks to re-use the fields for the attachment display settings from the media browser modal (above). This is in line with what core does for specifying the display settings when selecting media to add. When seeking to edit the display settings for a previously-selected media item, should the display settings fields be re-populated with what the user had previously chosen, to then make changes, and then hit the Add to Widget button again?

Something to note here is that in the context of the post editor when selecting previously-inserted media to edit there are actually different modal UI for doing editing attachment display settings:

☞ Note: There are potentially a lot of relevant attachment settings for video/audio that are not currently being accounted for in the media widget. The attachment display settings shown in the media selection modal is actually a subset of all of the fields that a user can specify when editing the modal.

Another option for where to display the attachment display setting fields would be to hide them from the media modal entirely (as is done for media modal for selecting the Custom Logo) and then to instead display the relevant fields with the selected media in the widget form itself:

This would have a potential additional benefit of the fields being more accessible and easier for users to make changes and then see those changes in the preview without the media modal being overlaid. That being said, adding display fields to the widget can make it cluttered, especially considering the many possible settings that need to be considered for the video/audio embeds. With that in mind, what if the widget re-used the modals for editing display properties for media which is invoked via a secondary “Edit Media” (or some other label) button:

When the media is an image, this could then open modal-modal-edit-image-attachment-display-settings.png​ whereas if a video it could open modal-modal-edit-video-attachment-display-settings.png​, and similarly for audio.

So, we have some questions to wrestle through.

[azaozz]

@melchoyce, @westonruter, you are aware that when we add HTML support to the Text widget #35243, it will make this widget pretty much redundant as everybody will be able to add an image there. Even if the UI in the Text widget it cut down to only supporting a handful of HTML tags, it still will be more useful as the users will be able to paste any HTML they want in there.

I'm not sure it will be a good UX to have two widget that do the exact same thing and have different UI.

[melchoyce]

@westonruter:

With that in mind, what if the widget re-used the modals for editing display properties for media which is invoked via a secondary “Edit Media” (or some other label) button:

I'd swap the buttons so edit is on the left and change is on the right, but this is the direction I think we should go in. 👍 It provides, IMO, a much more streamlined experience, and doesn't duplicate settings outside of where they normally appear in the modal.

---

Outside of this ticket, Weston and I have also been chatting with @matt about the direction we should take this widget. We've decided that to be more future-forward, we're going to split the media widget out into separate, focused widgets:

• Image widget
• Video widget
• Audio widget
• ...and if we can create these before content blocks launch, new widgets for things like slideshows, playlists, galleries, etc.

This provides a couple longterm benefits:

• We can focus on create more tailored experiences for each widget
• We'll be able to launch new widgets without having to worry about constantly updating one central widget, or potentially breaking anything
• It'll be easier for people to discover new media types since they won't be buried within one widget
• This will more closely mimic the approach we're taking to content blocks in the future, which should provide an easier transition

We're going to split this ticket out into three new tickets to cover the initial three widget types (image, video, audio), and then we might close this big ticket or keep it open for overall discussion about media widgets. Still thinking that through, so if you see a benefit to one or the other, please chime in. :)

[melchoyce]

New tickets:

• Image #39993
• Video #39994
• Audio #39995

[westonruter]

@gonom9 I've cloned your gist into its own dedicated GitHub repo for the plugin: ​https://github.com/xwp/wp-core-media-widgets

So moving forward let's focus on this feature plugin for collaborating on changes. If you would please sync the code from ​the PR into the plugin one last time and push the changes up to the new GitHub repo, if you haven't done so already.

I'll hook up Travis CI on the plugin repo and get the automated tests in place. We can get it on WordPress.org as well to facilitate user testing. We can split up the existing media widget into three separate widgets—for images, video, and audio—but focusing on image first. Once a widget has been thoroughly tested by users we can then copy it into core for a release while then also disabling the widget in the plugin. In this way we can use the repo for collaborating on development and as a testbed for users.

[westonruter]

As noted on #39993, an initial version of the image widget is available now for testing as part of the ​Core Media Widgets feature plugin. Please take it for a spin!

[ahortin]

Just tried this new Image Widget on a custom theme, Twenty Seventeen and also on Twenty Twelve and got the same result after adding an image. The image doesn't sit nicely in the actual widget.

Here's Twenty Twelve - ​https://cl.ly/3z0n3x0j0n15

Also, it would be nice if there was an Open link in a new tab checkbox on the Select Image dialog rather than having to select an image and then edit the image, just to select this checkbox. It makes for a cumbersome user experience. i.e. on this screen: ​https://cl.ly/0I3l1U1a0l1n

[westonruter]

@ahortin Thanks for the feedback. Could you please create two separate issues for the on the GitHub project? ​https://github.com/xwp/wp-core-media-widgets/issues

I should have mentioned that previously, that issues and feedback be filed there.

[ahortin]

@westonruter No probs. Will do that now.

[Stagger Lee]

I have to admit, I am following this trac and dont understand all of this.
Why not just make in the core one new widget with title & (one) input field, where you type Page/Post ID number, and all content is displayed in widget !!??

Can be custom post type for widgets, can be Page, or Post despite there is less logic to make it with Posts.

• You automatically get all media/TinyMce magic existing for Pages/Posts.
• You save yourself much time for coding (much more is to come).
• Additional one new Post status to lock Page and prevent Google indexing would be nice too.
• As it is now, it is dynamic content and need changing many times, Users would be able to enter widgets control panel. One slip with notebook mouse pad, some widget change position, is removed, etc...

Sorry, dont want to be disrespectful. You obviously gave much of your time and energy into it. Just seems as overdoing.

[ahortin]

@stagger-lee As it indicates in the Trac description, adding images into a sidebar is an extremely common occurrence. The aim of this ticket is to create a simple image widget for people to use, that makes this task considerably easier. It's not for adding page or post content. It's simply for adding an image (and optional link). The functionality is currently in a plugin purely for ease of testing. Once testing has been completed, the plan is to migrate this into core so its available for everyone.

[Stagger Lee]

Hi @ahortin

Just go after it if you feel this way. It is fine work, and for sure big improvement. I see audio, video widgets are announced so it is not only image widget.

Will try not to write anymore, to not send mails and annoy subscribers.

[westonruter]

In 40640:

Widgets: Introduce media widgets for images, audio, and video with extensible base for additional media widgets in the future.

The last time a new widget was introduced, Vuvuzelas were a thing, Angry Birds started taking over phones, and WordPress stopped shipping with Kubrick. Seven years and 17 releases without new widgets have been enough, time to spice up your sidebar!

Props westonruter, melchoyce, obenland, timmydcrawford, adamsilverstein, gonom9, wonderboymusic, Fab1en, DrewAPicture, sirbrillig, joen, matias, samikeijonen, afercia, celloexpressions, designsimply, michelleweber, ranh, kjellr, karmatosed.
Fixes #32417, #39993, #39994, #39995.

[westonruter]

In 40641:

Widgets: Utilize WP REST API oEmbed proxy endpoint in media widgets.

Props timmydcrawford.
See #32417, #40450.

[afercia]

In 40647:

Media: Clean-up some MediaElementJS CSS: no more needed after [40642].

See #40707, #32417, #39994, #39995, #30281.

[westonruter]

In 40656:

Widgets: Defer rendering controls for media widgets until container element fully expands.

Fixes issue with MediaElement.js failing to build the player in an animating container that doesn't have established dimensions.
Also utilizes MediaElement.js for the video widget instead of using a native player.

See #32417.
Fixes #40750.

[westonruter]

In 40674:

Widgets: Remove image sizes generated during media widget phpunit tests from being version controlled.

Amends [40640].
See #32417.

[pcarvalho]

Hi everyone, any hints on why this is happening? bug or feature?

https://core.trac.wordpress.org/ticket/40769

[westonruter]

In 40785:

Widgets: Ensure title field for media widget will update with sanitized value after change event in addition to input event.

Aligns behavior of field with Text widget and other widgets whereby the server-sanitized title (via the widget-update request) is supplied into the field after the change event.

See #32417.
Fixes #40805.

[afercia]

In 40812:

Widgets: clarify some context information for translators.

Props Italian polyglots team.
Amends [40640].
See #32417, #39993, #39994, #39995.

[westonruter]

In 40821:

Widgets: Further refine WP JS coding style in media widgets code.

Props afercia.
See #32417.

[westonruter]

In 40827:

Widgets: Prevent multiple items from being selectable when first populating a media widget.

Amends [40640].
Props obenland, timmydcrawford.
See #32417.

[westonruter]

In 40835:

Widgets: Supply missing descriptions for instance schema fields in media widgets.

Reuse translated strings from media templates.

Amends [40640].
See #32417.

[westonruter]

In 40836:

Widgets: Ensure that audio and video files appear in media library browser upon upload.

Props timmydcrawford.
Amends [40640].
See #32417, #39994, #39995.
Fixes #40803.

[westonruter]

In 40847:

Widgets: Normalize YouTube and Vimeo URLs in video shortcode (primarily for Video widget) to work around ME.js 2.22 bug.

Props timmydcrawford, jnylen0, westonruter.
See #32417, #39994.
Fixes #40866.

[Stagger Lee]

Now when you decided go this way, and let non-admins access to widgets panel. Can you at least generally speaking make a pop-up warning if User accidentically drag and drop widget from one place to another ? Some kind of confirmation, and Yes-No buttons. Specially if Users do it via smartphones.

[westonruter]

In 40941:

Widgets: Add accessibility mode support for TinyMCE-enhanced Text and Media widgets (Video, Audio, Images).

Amends [40640], [40631].
Props westonruter, afercia.
See #35243, #32417.
Fixes #40986.

[westonruter]

In 41028:

Widgets: Enqueue assets needed by media and text widgets in their _register_one() methods.

The WP_Widget::_register_one() method is more guaranteed to be called as opposed to its wrapper WP_Widget::_register() which plugins may bypass for performance reasons.

Amends [40631], [40640].
See #35243, #32417.
Fixes #41021.

[westonruter]

In 41051:

Widgets: Add accessibility mode support for TinyMCE-enhanced Text and Media widgets (Video, Audio, Images).

Merges [40941] to 4.8 branch.
Amends [40640], [40631].
Props westonruter, afercia.
See #35243, #32417.
Fixes #40986 for 4.8.1.

[westonruter]

In 41052:

Widgets: Enqueue assets needed by media and text widgets in their _register_one() methods.

The WP_Widget::_register_one() method is more guaranteed to be called as opposed to its wrapper WP_Widget::_register() which plugins may bypass for performance reasons.

Merges [41028] to the 4.8 branch.
Amends [40631], [40640].
See #35243, #32417.
Fixes #41021 for 4.8.1.

[westonruter]

In 41248:

Customize: Prevent attachment deletions from silently failing in media modals opened for Media widgets.

Amends [40640].
See #32417.
Fixes #41609.

[westonruter]

In 41590:

Widgets: Introduce Gallery widget for displaying image galleries.

• Galleries are managed in the widget in the same way they are managed in the post editor, both using the media manager.
• Gallery widget is merged from the Core Media Widgets v0.2.0 feature plugin and it extends WP_Widget_Media in the same way as is done for image, audio, and video widgets.
• Model syncing logic is updated to support booleans and arrays (of integers).
• Placeholder areas in media widgets are now clickable shortcuts for selecting media.
• Image widget placeholder is updated to match gallery widget where clicking preview is shortcut for editing media.

Props westonruter, joemcgill, timmydcrawford, m1tk00, obenland, melchoyce.
See #32417.
Fixes #41914.

[westonruter]

In 41951:

Widgets: Improve extensibility of the Gallery widget and of media widgets generally.

• Introduce a widget_{$id_base}_instance_schema filter for plugins to add new properties to a media widget's instance schema.
• Pass all of a gallery widget's instance props to the gallery media frame, not just the ones that core supports.

See #32417, #41914.
Fixes #42285.

[westonruter]

Dev note: ​https://make.wordpress.org/core/2017/05/26/media-widgets-for-images-video-and-audio/