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

Compat: Social Links: Remove legacy renderers from packages #20098

Merged
merged 2 commits into from Feb 7, 2020

Conversation

@mcsf
Copy link
Contributor

mcsf commented Feb 7, 2020

Description

As of #19887, Social Link is implemented as a single block type with multiple variations, whereas previously each supported service (e.g. WordPress, Twitter) was implemented as its own cloned block type:

Old format: <!-- wp:social-link-wordpress {"url":"foo"} /-->
New format: <!-- wp:social-link {"service":"wordpress","url":"foo"} /-->

However, Social Link is defined as a dynamic block. Thus, for backwards compatibility, #19887 preserved the server-side registration of all the "cloned" block types. However, this requirement only applies to the Gutenberg plugin, since Social Links haven't landed in core yet. Thus, this PR moves the server-side registration of cloned block types out of packages — specifically, block-library and into lib/blocks.php.

This will ensure that, the next time packages are published and pulled into WordPress core, no added work is needed to make sure that core installations do not register unneeded legacy block types.

How has this been tested?

  • Make sure that both new-format and old-format Social Link blocks are correctly rendered on the front-end.
  • If the process were more ergonomic, it would be ideal to test that publishing packages and building core from them pulls in core/social-link but no core/social-link-xyz clones.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.
@mcsf mcsf requested review from gziolo, aduth and jorgefilipecosta Feb 7, 2020
@mcsf mcsf requested review from mkaz and TimothyBJacobs as code owners Feb 7, 2020
@mcsf mcsf added this to Needs Review in WordPress 5.4 Must Have Feb 7, 2020
@mcsf mcsf mentioned this pull request Feb 7, 2020
0 of 6 tasks complete
'type' => 'string',
),
),
'render_callback' => 'gutenberg_render_core_social_link',

This comment has been minimized.

Copy link
@mcsf

mcsf Feb 7, 2020

Author Contributor

Will need rebasing after #20085.

Copy link
Member

gziolo left a comment

Code-wise this is exactly what I expected to see 👍 A nice improvement that we can fully test next week after it is integrated with WordPress core.

* @see https://github.com/WordPress/gutenberg/pull/19887
*/
function gutenberg_register_legacy_social_link_blocks() {
$sites = array(

This comment has been minimized.

Copy link
@gziolo

gziolo Feb 7, 2020

Member

It probably could be:

Suggested change
$sites = array(
$services = array(

but I see why it is called $sites. It's your call what to land.

This comment has been minimized.

Copy link
@mcsf

mcsf Feb 7, 2020

Author Contributor

That works — done.

@@ -85,6 +63,87 @@ function gutenberg_reregister_core_block_types() {
}
add_action( 'init', 'gutenberg_reregister_core_block_types' );

/**
* Complements the implementation of block type `core/social-icon`, whether it

This comment has been minimized.

Copy link
@gziolo

gziolo Feb 7, 2020

Member

I like where you are going with this, but I think we decided to leave the old name:

Suggested change
* Complements the implementation of block type `core/social-icon`, whether it
* Complements the implementation of block type `core/social-link`, whether it

😃

@gziolo
gziolo approved these changes Feb 7, 2020
Copy link
Member

gziolo left a comment

I can confirm it works as expected. When I remove this code, I can't see Social Icons on the frontend with the legacy version saved in the post content. With this patch, everything works as before.

@gziolo gziolo merged commit 48133c7 into master Feb 7, 2020
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
WordPress 5.4 Must Have automation moved this from Needs Review to Done Feb 7, 2020
@gziolo gziolo deleted the rm/social-link-multiple-render-callbacks branch Feb 7, 2020
@github-actions github-actions bot added this to the Gutenberg 7.5 milestone Feb 7, 2020
@mcsf

This comment has been minimized.

Copy link
Contributor Author

mcsf commented Feb 7, 2020

Hah, thanks for wrapping this one up for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.