WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 29 hours ago

#40854 assigned enhancement

Allow media to be embedded in Text widget

Reported by: alexvorn2 Owned by: azaozz
Milestone: 4.9 Priority: high
Severity: normal Version: 4.8
Component: Widgets Keywords: has-patch needs-testing
Focuses: ui Cc:

Description (last modified by ocean90)

Adding images, videos and audio files into Text widget without having 3 additional widgets I think will be better, because in a Text Widget you can add a description or additional information, just we need to add a Upload media button to insert media into the text field.
(Ticket related to Media Widgets - #32417) We can revert that ticked and continue to improve Text widget.
What do you think??

Attachments (6)

40854.0.diff (2.0 KB) - added by westonruter 2 weeks ago.
https://github.com/xwp/wordpress-develop/pull/253
text-widget-with-media.png (195.7 KB) - added by westonruter 2 weeks ago.
40854.2.diff (2.7 KB) - added by azaozz 2 weeks ago.
40854-widget-shortcodes.diff (3.1 KB) - added by westonruter 13 days ago.
40854-widget-shortcodes.2.diff (11.1 KB) - added by westonruter 12 days ago.
40854-widget-shortcodes.3.diff (12.6 KB) - added by westonruter 12 days ago.
Account for shortcode_unautop()

Download all attachments as: .zip

Change History (31)

#1 @alexvorn2
4 months ago

Before 4.8 Release *

#2 @ocean90
4 months ago

  • Description modified (diff)
  • Focuses accessibility removed
  • Summary changed from Before 4.8, Integrate media widgets functionality into Text widget to Integrate media widgets functionality into Text widget

#3 follow-up: @westonruter
4 months ago

  • Summary changed from Integrate media widgets functionality into Text widget to Allow media to be embedded in Text widget

The rationale for having a separate widget for each media type is explained in https://core.trac.wordpress.org/ticket/32417#comment:136

That being said, the ability to embed media inside of a Text widget should also probably facilitated.

#4 @karmatosed
4 months ago

I would +1 having media embed inside, but also agree it should not mean merging the widgets.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


4 months ago

#6 @westonruter
4 months ago

  • Milestone changed from Awaiting Review to 4.8.1

#7 @westonruter
4 months ago

Adding oEmbeds depends on #34115.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


2 months ago

#9 @westonruter
2 months ago

  • Keywords needs-patch added
  • Milestone changed from 4.8.1 to 4.9

#10 in reply to: ↑ 3 ; follow-up: @adriannees
8 weeks ago

I don't disagree that separate widgets are useful HOWEVER I'm currently in a project where I need to add an image to a text widget because the client made a last minute change request and I don't want to nor feel it should be necessary to completely re-configure the widget area and fiddle with all of the media queries and styling just to be able to add an image widget NEXT to the text. This widget is now more annoying than helpful because it's a bastard child of both a wysiwyg and a text widget.

Replying to westonruter:

The rationale for having a separate widget for each media type is explained in https://core.trac.wordpress.org/ticket/32417#comment:136

That being said, the ability to embed media inside of a Text widget should also probably facilitated.

#11 in reply to: ↑ 10 ; follow-up: @melchoyce
8 weeks ago

Replying to adriannees:

I don't disagree that separate widgets are useful HOWEVER I'm currently in a project where I need to add an image to a text widget because the client made a last minute change request and I don't want to nor feel it should be necessary to completely re-configure the widget area and fiddle with all of the media queries and styling just to be able to add an image widget NEXT to the text.

You can still flip over to the text tab and manually add an image to your widget:

<img src="your image url" alt="image description" />

#12 in reply to: ↑ 11 @adriannees
8 weeks ago

Yes I know, I'm aware of that - but it requires looking up the media URL before doing doing so and doesn't have the wysiwyg image options making it just as difficult as the text editor was before in this regard while also having the added inconvenience of it stripping your code or adding additional code if you flip to the visual editor. I know developers aren't really being considered in this widgets evolution so just imagine if one of our non-technical clients decides they want to change the picture down the line? You didn't want them to have to deal with code if they didn't want to but you're going to force them to if they want pictures (which can be confusing as hell to them)

The point is this is either too much or not enough. So for now I've switched begrudgingly back to using the Black Studio plugin (which works great no knocking their work, just preferred to use the built in)

Replying to melchoyce:

You can still flip over to the text tab and manually add an image to your widget:

<img src="your image url" alt="image description" />

#13 @westonruter
2 weeks ago

Just to re-summarize...

Follow up on #32417 (add media widget), #35243 (extending widget with visual mode), and #35760 (providing API to instantiate editor dynamically).

The work in this ticket probably will be focused on extending wp.editor.initialize() (introduced in #35760).

Note that in order for oEmbeds to be allowed, we'd have to implement #34115.

#14 follow-up: @westonruter
2 weeks ago

  • Owner set to azaozz
  • Status changed from new to assigned

@azaozz I started looking into this. I was happy with how easy it was to get the button to work initially. See 40854.0.diff. Nevertheless, you're much more familiar with it than I am, so any help would be appreciated as there are some issues that need additional work:

  • As seen in text-widget-with-media.png, the video and audio shortcodes aren't getting converted into TinyMCE views. Similarly, inserting a gallery just shows a placeholder without any preview.
  • Insert media window should show “Insert into widget” instead of “Insert into post”.
  • Media button should be shown even if rich text editing is disabled.
  • “Add Media” button needs to be translated.

Supporting oEmbed is dependent on #34115.

Last edited 2 weeks ago by westonruter (previous) (diff)

@azaozz
2 weeks ago

#15 in reply to: ↑ 14 @azaozz
2 weeks ago

Replying to westonruter:

Hm, if I remember right there were strong objections to adding support for media to the Text widget. This gives this editor pretty much equivalent capabilities as the "main" editor and makes the (new) image, video and audio widgets somewhat redundant.

40854.diff looks good, just have to enable the wpview plugin to show "previews" for galleries and embeds. Added that in 40854.2.diff.

Yeah, we can't do any oEmbeds as currently there is no way to cache the responses. Once this is solved all embeddable content will work properly inside the editor.

#16 @azaozz
2 weeks ago

  • Keywords has-patch needs-testing added; needs-patch removed

Going to commit 40854.2.diff so this can be tested easier.

#17 @azaozz
2 weeks ago

In 41344:

Text widget: add the Add Media button and enable the wpview plugin to show embedded media previews in the editor.

Props westonruter, azaozz.
See #40854.

#18 follow-up: @azaozz
2 weeks ago

Still to be done: shortcodes are not parsed on the front-end there, so audio, video and galleries fail to show up in the Customizer preview. Also still somewhat unclear about having several widgets with overlapping features.

Last edited 2 weeks ago by azaozz (previous) (diff)

#19 in reply to: ↑ 18 @westonruter
13 days ago

Thank you for working that out.

Replying to azaozz:

Still to be done: shortcodes are not parsed on the front-end there, so audio, video and galleries fail to show up in the Customizer preview.

Actually, it's not just in the Customizer preview. It's on the frontend generally, as rendering shortcodes isn't currently supported in the Text widget in core, and plugins currently are needed to add support for them. It did not occur to me that shortcodes would be required when adding media, but it slipped my mind.

Also still somewhat unclear about having several widgets with overlapping features.

It does seem somewhat of a shame to go to the trouble of implementing a Gallery widget when a user could also just add a gallery into a Text widget. Maybe it's about being able to provide guided user experiences to accomplish certain tasks.

There is overlap to be sure, but I think there may be enough use cases for separate widgets. It seems clear that users need to be able to add images along their text, in particular floating images around text in the sidebar. But maybe core shouldn't support this, and core should instead just require two Text widgets with an image widget in between if that is the desired effect (though it wouldn't be semantically right).

@melchoyce Thoughts?

#20 @melchoyce
13 days ago

Yeah, I don’t think being able to embed media in the text widget makes the media widgets pointless. I don’t mind the redundancy here at all. Sometimes you just want one thing, like a banner in your sidebar. Sometimes you want something a little more complex, like a photo of yourself and a short bio. If you’re working on your site and search for image, or video, in your widget list, the standalone media widgets provide a faster and more straightforward experience. (Plus, you’d need to know about Text supporting media to take advantage of it in the first place.) I'm totally happy to have this move forward and get committed early & often.

#21 @westonruter
13 days ago

Cool. So then next we'll need to support for (a subset of) shortcodes for the widget. Given that shortcodes often rely on a $post global, we'd need to potentially temporarily unset this global variable so that the widget's doesn't attempt to use an unexpected post as context, e.g. the last post in The Loop on an archive page.

The shortcodes and their handlers in core are:

  • [wp_caption] => img_caption_shortcode()
  • [caption] => img_caption_shortcode()
  • [gallery] => gallery_shortcode()
  • [playlist] => wp_playlist_shortcode()
  • [audio] => wp_audio_shortcode()
  • [video] => wp_video_shortcode()
  • [embed] => WP_Embed::shortcode()

The one I know for sure will needs work here to work without a $post global is embed. And for that see #34115.

#22 @westonruter
13 days ago

40854-widget-shortcodes.diff adds support for shortcodes in widgets, but unit tests need updating before committing that one.

@westonruter
12 days ago

Account for shortcode_unautop()

#23 @westonruter
12 days ago

In 41361:

Widgets: Add shortcode support inside Text widgets.

  • Used now in core to facilitate displaying inserted media. See #40854.
  • The [embed] shortcode is not supported because there is no post context for caching oEmbed responses. This depends on #34115.
  • Add do_shortcode() to the widget_text_content filter in the same way it is added for the_content at priority 11, with shortcode_unautop() called at priority 10 after wpautop().
  • For Text widget in legacy mode, manually apply do_shortcode() (and shortcode_unautop() if auto-paragraph checked) if the core-added widget_text_content filter remains, unless a plugin added do_shortcode() to widget_text to prevent applying shortcodes twice.
  • Ensure that global $post is null while filters apply in the Text widget so shortcode handlers won't run with unexpected contexts.

Props westonruter, nacin, aaroncampbell.
See #40854, #34115.
Fixes #10457.

#24 @westonruter
12 days ago

After [41361] now the main thing left is to support the [embed] shortcode. While this depends on #40854, it will also require incorporating some of the logic from WP_Embed, specifically WP_Embed::run_shortcode().

#25 @westonruter
29 hours ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

Note: See TracTickets for help on using tickets.