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

Resp. videos: don't load if theme supports core responsive embeds #10880

Merged
merged 1 commit into from
Jan 2, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Dec 7, 2018

Fixes #10678
Fixes Automattic/wp-calypso#28323

Changes proposed in this Pull Request:

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

Testing instructions:

Proposed changelog entry for your changes:

  • Responsive videos: don't load if theme supports core responsive embeds

Fixes #10678

If a theme already includes support for Core's Responsive Embeds feature, they don't need our
implementation.
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Theme Tools [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Dec 7, 2018
@jeherve jeherve self-assigned this Dec 7, 2018
@jeherve jeherve requested a review from a team December 7, 2018 14:26
@matticbot
Copy link
Contributor

D22005-code. (newly created revision)

@jetpackbot
Copy link
Collaborator

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS

@jasmussen
Copy link
Member

Awesome work ♥️♥️

Copy link
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well 🚢

@brbrr brbrr added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 18, 2018
@jeherve jeherve added this to the 6.9 milestone Dec 18, 2018
@jeherve jeherve merged commit 3bf755f into master Jan 2, 2019
@ghost ghost removed [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 2, 2019
@jeherve jeherve deleted the fix/responsive-gutenberg branch January 2, 2019 13:05
jeherve added a commit that referenced this pull request Jan 3, 2019
jeherve added a commit that referenced this pull request Jan 3, 2019
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
jeherve added a commit that referenced this pull request Jan 21, 2019
Fixes #11097

This is a different approach than #10880. Instead of shortcircuiting
Jetpack responsive videos as soon as we detect that the theme supports
core responsive embeds, we check the HTML content of each post for blocks.
If we find blocks, and if we find embed blocks among those, we do not use Jetpack responsive videos.
If however all we find are classic blocks (even with videos inside) or shortcodes, we carry on and use
Jetpack responsive videos.
jeherve added a commit that referenced this pull request Jan 29, 2019
* Responsive videos: do not load when embed blocks are in use

Fixes #11097

This is a different approach than #10880. Instead of shortcircuiting
Jetpack responsive videos as soon as we detect that the theme supports
core responsive embeds, we check the HTML content of each post for blocks.
If we find blocks, and if we find embed blocks among those, we do not use Jetpack responsive videos.
If however all we find are classic blocks (even with videos inside) or shortcodes, we carry on and use
Jetpack responsive videos.

* Responsive Videos: rm Jetpack wrapper around embed blocks
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 Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants