Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Responsive Videos: does not support Full Width and Wide Width block settings #10678

Closed
jeherve opened this issue Nov 20, 2018 · 16 comments
Closed
Assignees
Labels
[Feature] Theme Tools [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended

Comments

@jeherve
Copy link
Member

jeherve commented Nov 20, 2018

Steps to reproduce the issue

  1. Enable Twenty Nineteen on your site.
  2. In a new post, add some text, and then a new video (e.g. Youtube) on its own line.
  3. Pick the wide width setting:

screenshot 2018-11-20 at 15 51 09

4. On the frontend, see that the video is not as wide as the rest of the post content.

screenshot 2018-11-20 at 15 59 44

When disabling Responsive Videos, the width is fine:

screenshot 2018-11-20 at 15 59 27

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Theme Tools [Pri] Normal [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Nov 20, 2018
@oandregal
Copy link

For the record, I ran into this issue WordPress/gutenberg#12676

Tried disabling some Jetpack modules but it didn't work. I didn't find how to disable the "Responsive Videos" and the issue went away if I disconnected from Jetpack.

@jasmussen
Copy link
Member

To elaborate a little bit here: Gutenberg comes with responsive embed support out of the box.

The theme has to opt in to support these, because often themes themselves provide their own responsive embeds support, and that conflicts with the support that Gutenberg comes with.

Twenty Nineteen opts into gutenberg responsive embeds. But Jetpack is not aware of this, and applies its responsive magic on top of that, which causes the issue reported here. Ideally Jetpack can turn off the responsive embed stuff it does, when it detects that the theme has opted into Gutenberg responsiveness. See more here: https://wordpress.org/gutenberg/handbook/designers-developers/developers/themes/theme-support/#responsive-embedded-content

add_theme_support( 'responsive-embeds' );

jeherve added a commit that referenced this issue Dec 7, 2018
Fixes #10678

If a theme already includes support for Core's Responsive Embeds feature, they don't need our
implementation.
@jeherve
Copy link
Member Author

jeherve commented Dec 7, 2018

I didn't find how to disable the "Responsive Videos"

Here is how you would go about it:

function jeherve_dont_load_resp( $tools ) {
	$index = array_search( 'theme-tools/responsive-videos.php', $tools );
	if ( false !== $index ) {
		unset( $tools[ $index ] );
	}
	return $tools;
}
add_filter( 'jetpack_tools_to_include', 'jeherve_dont_load_resp' );

Twenty Nineteen opts into gutenberg responsive embeds.

I'll not load Responsive Videos when a theme supports those, I think that would be a first solution. #10880

@jeherve jeherve self-assigned this Dec 7, 2018
@dartiss
Copy link

dartiss commented Dec 11, 2018

Hi @jeherve 👋🏼

I'm experiencing the same problem - embedded YouTube videos on my Twenty Nineteen theme are not responsive at all. You can see an example here - https://artiss.blog/2018/09/overwatch-the-life-of-a-bastion-main/ (video is half way down).

De-activating Jetpack resolves the issue.

The reason I mention is that on my test site I'm running an older version of Jetpack - 6.5 - and it actually works. Is this relevant at all?

@jeherve
Copy link
Member Author

jeherve commented Dec 11, 2018

on my test site I'm running an older version of Jetpack - 6.5 - and it actually works. Is this relevant at all?

We've only added Responsive Videos support to Twenty Nineteen (as well as other custom changes to ensure the best compatibility with the new default theme) in the last release, so yes, it is expected that you do not see the problem with older versions of Jetpack.

@flexerd
Copy link

flexerd commented Dec 20, 2018

I didn't find how to disable the "Responsive Videos"

Here is how you would go about it:

function jeherve_dont_load_resp( $tools ) {
	$index = array_search( 'theme-tools/responsive-videos.php', $tools );
	if ( false !== $index ) {
		unset( $tools[ $index ] );
	}
	return $tools;
}
add_filter( 'jetpack_tools_to_include', 'jeherve_dont_load_resp' );

Twenty Nineteen opts into gutenberg responsive embeds.

I'll not load Responsive Videos when a theme supports those, I think that would be a first solution. #10880

Hi
The code did not work on Jetpack Version 6.8.1 had to disable JP as required full width video.

Any ideas?
Thanks

@jeherve
Copy link
Member Author

jeherve commented Dec 20, 2018

@flexerd Could you let me know where and how you inserted that code snippet? Could you try inserting it as explained here?

@flexerd
Copy link

flexerd commented Dec 20, 2018

@jeherve In functions.php in the child-theme

@jeherve
Copy link
Member Author

jeherve commented Dec 20, 2018

@flexerd Could you try as I suggested instead, and let me know how it goes?

@Aurorum
Copy link
Contributor

Aurorum commented Dec 30, 2018

Enable Twenty Nineteen on your site.

I don't think it just needs to be Twenty Nineteen. I've tested with Photos on WordPress.com which still causes the same issue. Independent Publisher 2, for example, doesn't cause an issue in the post itself but creates a huge space in the editor. Would the fix work for all themes? :)

I've included several screenshots in Automattic/themes#457. Visually, the result seems to look way worse for some reason. I'm fairly sure WordPress.com sites can't disable Responsive Videos either, which causes the bug to be more prevalent over there too.

@jeherve
Copy link
Member Author

jeherve commented Jan 2, 2019

I don't think it just needs to be Twenty Nineteen.

That is correct. The problem will apply to all themes that declare support for core responsive embeds (which most of the Automattic themes do). The fix will apply to all those themes.

jeherve added a commit that referenced this issue Jan 2, 2019
…0880)

Fixes #10678

If a theme already includes support for Core's Responsive Embeds feature, they don't need our
implementation.
@supernovia
Copy link

Is there any way this could have caused sites that aren't using blocks to display improperly cropped videos?

@jeherve
Copy link
Member Author

jeherve commented Jan 8, 2019

Yes, see #11097

@webmandesign
Copy link
Contributor

Hi @jeherve & @jasmussen,

I think you are missing one important usecase here:

The theme may not opt into Gutenberg's responsive embeds and will use Jetpack's responsive videos functionality with fallback to a custom solution (FitVids). The reason for this is that even if we use block editor in WordPress, it is not enabled for all post types, yet the theme has to provide responsive video styles for all content.

For example, a theme needs to provide responsive styles for videos in WooCommerce product content. WooCommerce products are not block editor (Gutenberg) enabled. They can only be edited with classic editor. Our theme has these options:

  • Opt into Jetpack responsive videos functionality - this will cause issues in posts written with Gutenberg, so we need to...
  • ...opt into Gutenberg responsive embeds - but this will switch off the Jetpack responsive videos altogether, rendering Gutenberg disabled post type's content videos un-responsive, thus the only solution here is...
  • ...using a custom solution (FitVids) only - basically, this seems to be the only solution for a theme if it wants to support all WordPress content, rendering Jetpack responsive videos functionality basically obsolete nowadays.

I'm not sure if you can do something about it in Jetpack though. I just posted this as an info in case someone would come across the similar situation.

I personally use FitVids as fallback when Jetpack is not used on the site. Unfortunately, I'm switching to FitVids only solution from now on.

Best regards,

Oliver

@jeherve
Copy link
Member Author

jeherve commented May 29, 2019

@webmandesign Thanks for the feedback! We ended up going a different route, and disabling Jetpack's Responsive Videos on a case-by-base basis:
#11171

This may be more in line with what you have in mind, but it requires using the classic block instead of the classic editor. Would that be an acceptable solution?

@webmandesign
Copy link
Contributor

@jeherve Thank you for the update! This should work perfectly fine for my future themes and I will definitely test it then. For existing themes I stick with fallback solution for now though. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Theme Tools [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

No branches or pull requests

8 participants