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

Post Images: look for images in inner blocks. #13358

Merged
merged 3 commits into from
Sep 19, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Aug 28, 2019

Fixes #13338

Changes proposed in this Pull Request:

Until now, we only parsed blocks that included images right away. However, some blocks may include some other, inner, blocks.
Let's go 2 levels deep to look for blocks that we support and that may include images.

This allows us to support, for example, a colunms block that would include an image block as well as a text block.

Testing instructions:

  • Start on a site without this patch, with the Sharing module active.
  • In a new post, add an image block and upload a large image. Publish your post.
  • View the post, and view source.
  • Search for an og:image meta tag. It should list your image.
  • Now start a new post.
  • Insert a Columns block. In that block, insert a Paragraph block as well as an image block. Add the same image as before to your image block.
  • Publish the post, view it, view source.
  • Search for an og:image meta tag. Your image is not listed.
  • Apply the patch.
  • Note that og:image meta tags now work even with posts with columns blocks.

You can repeat this with Gallery blocks, tiled galleries, slideshows, all inside columns blocks.

You can also try installing the Kadence Blocks – Gutenberg Page Builder Toolkit plugin, and insert a Row layout block instead of a columns block. See that everything keeps working.

Proposed changelog entry for your changes:

  • Post Images: look for representative images in inner blocks.

This adds 3 new tests, to cover columns blocks with attachment images as well as external images
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Sharing Post sharing, sharing buttons [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] Normal [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Aug 28, 2019
@jeherve jeherve added this to the 7.8 milestone Aug 28, 2019
@jeherve jeherve requested a review from a team August 28, 2019 16:29
@jeherve jeherve self-assigned this Aug 28, 2019
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello jeherve! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D32151-code before merging this PR. Thank you!

Fixes #13338

Until now, we only parsed blocks that included images right away.
However, some blocks may include some other, inner, blocks.
Let's go 2 levels deep to look for blocks that we support and that may include images.

This allows us to support, for example, a colunms block that would include an image block as well as a text block.
@jeherve jeherve force-pushed the fix/post-images-detection-column-blocks branch from 80844f2 to af615a4 Compare August 28, 2019 16:30
@matticbot
Copy link
Contributor

jeherve, Your synced wpcom patch D32151-code has been updated.

@jetpackbot
Copy link

jetpackbot commented Aug 28, 2019

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: October 1, 2019.
Scheduled code freeze: September 24, 2019

Generated by 🚫 dangerJS against f7b0cb0

@kraftbj kraftbj self-requested a review September 18, 2019 17:21
@matticbot
Copy link
Contributor

jeherve, Your synced wpcom patch D32151-code has been updated.

@kraftbj
Copy link
Contributor

kraftbj commented Sep 18, 2019

One gap I discovered in testing is the core/media-text block. This block is unique in that it is a nested block, but the ID of the media is part of the parent block's attribute—not an inner block.

I added a commit to handle this, so leaving as needs review for at least that second commit. Approving it so @jeherve can be that reviewer of my change. :)

@jeherve jeherve 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 Sep 19, 2019
@jeherve
Copy link
Member Author

jeherve commented Sep 19, 2019

This looks good to me, thanks for the addition! Merging.

@jeherve jeherve merged commit 65c8230 into master Sep 19, 2019
@jeherve jeherve deleted the fix/post-images-detection-column-blocks branch September 19, 2019 09:57
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 19, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Sharing Post sharing, sharing buttons [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal 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.

Post Images: from_blocks cannot grab images inside nested blocks
4 participants