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

Fix for current_parsed_blocks value when block has inner blocks #26291

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Oct 19, 2020

In #26192 we refactored the block supports mechanism. It works by holding a global $current_parsed_blocks reference that it uses to determine the attributes that it needs to add to blocks.

However, when a block has inner blocks, that reference isn't updated so inner blocks end up inheriting the parent's properties (style and classes).

How to test

This is what I've done:

  • In your WordPress environment, patch the wp-includes/class-wp-block.php class with the contents of this PR.
  • Create a post that adds the social links block. Publish.
  • Verify that the individual social links (the li elements) don't have the wp-social-links class attached. It should be present in the parent (ul element).

@oandregal oandregal self-assigned this Oct 19, 2020
@oandregal oandregal added the [Type] Bug An existing feature does not function as intended label Oct 19, 2020
@github-actions
Copy link

github-actions bot commented Oct 19, 2020

Size Change: 0 B

Total Size: 1.2 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.54 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.6 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/index.js 144 kB 0 B
build/block-library/style-rtl.css 7.71 kB 0 B
build/block-library/style.css 7.71 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 170 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.63 kB 0 B
build/core-data/index.js 12.1 kB 0 B
build/data-controls/index.js 684 B 0 B
build/data/index.js 8.63 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 4.43 kB 0 B
build/edit-navigation/index.js 10.6 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/index.js 21.6 kB 0 B
build/edit-site/style-rtl.css 3.8 kB 0 B
build/edit-site/style.css 3.81 kB 0 B
build/edit-widgets/index.js 26.6 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 42.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.35 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.61 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.75 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.23 kB 0 B

compressed-size-action

Copy link
Member

@mkaz mkaz left a comment

Choose a reason for hiding this comment

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

Tested this out and works as expected. 👍

The change makes sense, make sure the global has the right value before render and replace it with the previous value after.

@oandregal
Copy link
Member Author

oandregal commented Oct 19, 2020

cc @tellthemachines we need this (or an alternative that fixes the bug) for the release. Flagging in case I'm not around when that happens.

@oandregal oandregal added this to the Gutenberg 9.2 milestone Oct 19, 2020
@oandregal oandregal force-pushed the fix/current-parsed-block-inner-blocks branch from f9ce774 to 519a496 Compare October 19, 2020 19:58
@oandregal
Copy link
Member Author

I think the change is solid, but there's a couple of things I don't know how to do:

  • how does the patch work for the plugin while WP core is not updated?
  • how/when we backport these kinds of changes to WP core?

It's a bit late over here so I'll leave the PR unmerged so others can chime in.

@mkaz
Copy link
Member

mkaz commented Oct 19, 2020

@nosolosw I believe the process is to open a core ticket with the diff.

I created a ticket and diff here: https://core.trac.wordpress.org/ticket/51569

@gaambo
Copy link
Contributor

gaambo commented Nov 4, 2020

I think this could have also been solved by looking at #25900 - because if pre_render_block would be called for innerBlocks as well (as I would have expected) than the original code from #26192 would have also worked.

@oandregal
Copy link
Member Author

Thanks for flagging, Fabian. I see there's a core ticket https://core.trac.wordpress.org/ticket/51612 for this as well as a potential solution at WordPress/wordpress-develop#691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants