WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 3 weeks ago

#42203 assigned task (blessed)

Ensure media & embeds in Text widget are styled properly across widget areas in bundled themes

Reported by: westonruter Owned by: davidakennedy
Milestone: 5.0 Priority: high
Severity: normal Version:
Component: Bundled Theme Keywords: needs-patch
Focuses: Cc:

Description

The Text widget now allows for media to be embedded within it as of #40854. There is an Add Media button allowing for images, videos, and other embeds to be added to the content. Themes may not have styles in them that expect media to appear in a Text widget context, since previously they may have only appeared in post content. As noted in §Default Themes Updates for Media Widgets for Images, Video, and Audio:

Themes that add custom styles to the MediaElement.js player (namely Twenty Thirteen and Twenty Fourteen) were updated from just styling it within syndicated content, to also include instances within widgets. Most themes don’t restrict styles for captioned images or media players to just post content, that is, limit CSS selectors to classes output by post_class(). If your theme does, make sure to either remove that constraint or include a .widget selector.

In the same way for the embeds in the Text widget, we need to make sure that all of the core themes are compatible. Text widgets need to be tested in all core themes and in each of their widget areas.

Once the core themes are updated then the changes will may be instructive for theme authors to make similar changes in their themes.

See same task for Gallery widget in #41969.

Change History (23)

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


2 months ago

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


8 weeks ago

#3 @alexvorn2
8 weeks ago

  1. Can we add gallery functionality to Text Widget (maybe to wp editor too)?
  2. Will everything work on Customizer page (embeds, audios, videos, adding images, tweets, YouTube videos etc)?

#4 @alexvorn2
8 weeks ago

Or we have already, I have not tested...

#5 @westonruter
8 weeks ago

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

#6 @SergeyBiryukov
8 weeks ago

  • Component changed from Widgets to Bundled Theme

#7 @davidakennedy
8 weeks ago

Overall thoughts

  • Some of the margin issues should be easy to solve.
  • The bigger challenge is dealing with widget areas that are extremely narrow, like in Twenty Fourteen and Twenty Fifteen in some cases.
  • Also, handling responsive iframes, objects, etc will probably be a challenge without adding a lot of extra CSS or a jQuery library like FitVids. Many of the video embeds didn't look bad in the widget areas, they just weren't full width in all cases.
  • I'm going to focus on the newer themes first since they likely have a higher install base and should be easier to fix in most cases.

I'm going to test #41969 tomorrow as well and come up with some patches.

Test results follow:

2017

Sidebar: Video iframes in sidebar not full width. Would need to have extra code to make full width.
Footer 1: Video iframes in sidebar not full width. Would need to have extra code to make full width.
Footer 2: Video iframes in sidebar not full width. Would need to have extra code to make full width.

The is especially noticeable in the footer widgets areas on certain screen sizes.

Screen shot: https://cloudup.com/cf0xK0ilIvv

2016

Sidebar: Video iframes in sidebar not full width. Would need to have extra code to make full width.
Footer 1: Video iframes in sidebar not full width. Would need to have extra code to make full width.
Footer 2: Video iframes in sidebar not full width. Would need to have extra code to make full width.

The is especially noticeable in the footer widgets areas on certain screen sizes.

Screen shot: https://cloudup.com/ckDlQsJL8mO

2015

The standard desktop widths in the one widget area look good. However, at smaller screen sizes, the iframes aren't full width.

Screenshot: https://cloudup.com/cA-JHysSvs5

At smaller widths with the sidebar to the left, some of the styles for the MediaElement.js player hit some stress cases.

Screenshot: https://cloudup.com/cVCIQyl-fb6

The Instagram embed styles look broken at narrow widths.

Screenshot: https://cloudup.com/clyPiTiDxBM

2014

Primary Sidebar: This sidebar has a narrow width, and some of the styles for the MediaElement.js player hit some stress cases.

Screenshot: https://cloudup.com/cM_SiDVw40S

Primary Sidebar, Content Sidebar Footer Widget Area: The YouTube embed breaks.

Screenshot: https://cloudup.com/c7TqKMb0A-A

2013

Main Widget Area: Similar to other narrow widget areas in default themes, the Instagram widget looks broken.

Screenshot: https://cloudup.com/cTSNn7gnK3T

Secondary Widget Area: Video iframes in sidebar not full width. Would need to have extra code to make full width.

Screenshot: https://cloudup.com/cmJ3M-YEmty

Also, some of the MediaElement.js containers need a margin.

2012

Main Sidebar: Some of the MediaElement.js containers need a margin. Video iframes in sidebar not full width. Would need to have a extra code to make full width. The is especially noticeable in the footer widgets areas on certain screen sizes. Same for both Front Page Widget Areas.

Screenshot: https://cloudup.com/cbl-F7BLkiv

2011

Main Sidebar: Some of the MediaElement.js containers need a margin. Video iframes in sidebar not full width. Would need to have a extra code to make full width. This is more noticable at smaller screen sizes when the sidebar drops under the main content areas.

Screenshot: https://cloudup.com/caTAILX2ZmG

Showcase Sidebar: This area has similar issues as other narrow widget areas.

Screenshot: https://cloudup.com/crdIXxAalFI

Footer Area One, Two and Three: These have the same issues as previously noted.

Screenshot: https://cloudup.com/c-QxyfB3iVO

2010

All the widget areas in this theme have similar issues to the other themes. Most notable are needing margins on the MediaElement.js containers. Although, this theme isn't responsive so it doesn't have as many width issues as other themes.

Screenshot: https://cloudup.com/cibKJF4PQ1n

#8 follow-up: @laurelfulford
8 weeks ago

@westonruter - I'm looking at this with @davidakennedy and one thing we noticed is that the Youtube/Vimeo embeds don't have a width set at all. This is causing the issues above where those embeds aren't filling the available space -- though no width appears to be set, they display at a maximum of 300px wide.

The Video widget uses MediaElement.js on YouTube/Vimeo embeds to make them responsive. Would this be a reasonable fix for these embeds in the Text widget?

Is it something that can be fixed at the widget level at this point, or better to address in the themes for now and circle back?

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


8 weeks ago

This ticket was mentioned in Slack in #core-media by davidakennedy. View the logs.


8 weeks ago

#11 follow-up: @westonruter
8 weeks ago

  • Keywords needs-patch added

@laurelfulford Interesting. This is probably why @obenland opted to continue using the video shortcode for displaying YouTube and Vimeo videos in [41765]. I think if we basically rewrote the embed shortcodes to video shortcodes specifically for YouTube and Vimeo it would only be a partial fix as other oEmbeds would continue to be broken so we should try to find a more general solution.

What if the width were to be injected into the embeds? We currently have logic in the Text widget to basically override the width that an embed may explicitly provide. But it isn't providing something in return.

See also what was done in the audio widget in [41886] to add a max-width style to the player. But it seems like something needs to be done at a lower-level to give it width to begin with?

I think it's something that should probably be fixed at the widget level, but we can do that as part of this ticket. What width injection would fix the issue?

#12 in reply to: ↑ 8 @alexvorn2
8 weeks ago

  • Keywords needs-patch removed

Replying to laurelfulford:
This plugin may help making videos wider on posts - https://wordpress.org/plugins/fluid-video-embeds/
So we can import the functionality here to the widget from the plugin...

#13 @westonruter
8 weeks ago

  • Keywords needs-patch added

#14 @alexvorn2
8 weeks ago

oh! the tag added itself! This new trac update is not good.

#15 in reply to: ↑ 11 @joemcgill
8 weeks ago

Replying to westonruter:

What if the width were to be injected into the embeds? We currently have logic in the Text widget to basically override the width that an embed may explicitly provide. But it isn't providing something in return.

This logic seems ok when all we're expecting to pass to the callback is an instance of a media embed, for which that particular logic seemed to be written. In the text widget, however, it seems to apply much too widely, trying to match any HTML element included in the text widget.

A possible approach here might be to try and add a wrapping div around iframes that appear to contain videos. That said, it's worth noting that if you remove the inject_video_max_width_style logic from the text widget, the same max-width issues that affect the text widget also seem to apply to videos included in content, unless handled by the theme. I think that's a much better situation than Core trying to over-engineer a solution here.

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


7 weeks ago

This ticket was mentioned in Slack in #core-media by davidakennedy. View the logs.


7 weeks ago

#19 @westonruter
7 weeks ago

I've limited the width/height replacements logic to only video, iframe, object, and embed elements. See [42001].

#20 @davidakennedy
7 weeks ago

  • Milestone changed from 4.9 to 4.9.1

I had a few Slack discussions with @melchoyce and @westonruter about this ticket. This could be fixed by bundling FitVids with default themes, or something similar. But it might be better to bundle the needed functionality with core itself, making all the embeds, videos, etc. responsive by default. Themes could opt into it with an add_theme_support statement. This makes even more sense with Gutenberg coming in the future, and all kinds of blocks potentially needing this functionality.

For that reason, I'd say we punt this until 4.9.1, and aim for something like that.

Until then, the default themes and the embeds will still look okay in most cases, they just won't be full width or fluid.

Happy to hear other suggestions and thoughts though.

cc: @joemcgill @bradyvercher in case they want to chime in.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


7 weeks ago

#22 @bradyvercher
7 weeks ago

@davidakennedy I'm all for a core-based solution considering this is something every theme has to deal with. I also like the add_theme_support() suggestion. While I'm a fan of FitVids.js, I would really prefer to see a vanilla JS alternative in core.

#23 @johnbillion
3 weeks ago

  • Milestone changed from 4.9.1 to 5.0
Note: See TracTickets for help on using tickets.