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

Add anchor support for dynamic blocks #44771

Merged
merged 7 commits into from
Feb 1, 2023
Merged

Add anchor support for dynamic blocks #44771

merged 7 commits into from
Feb 1, 2023

Conversation

Soean
Copy link
Member

@Soean Soean commented Oct 7, 2022

What?

  • Adds anchor support to dynamic rendered blocks.
  • Enables support for core blocks

Fixes #29401

Testing Instructions

You need to add 'id' to $attributes_to_merge in get_block_wrapper_attributes() from the WP_Block_Supports class, see https://github.com/WordPress/wordpress-develop/blob/6.0.2/src/wp-includes/class-wp-block-supports.php#L178

$attributes_to_merge = array( 'style', 'class' );

Change to:

$attributes_to_merge = array( 'style', 'class', 'id' );

@Soean Soean changed the title WIP: Add anchor support for dynamic blocks Add anchor support for dynamic blocks Oct 7, 2022
@Soean Soean added [Feature] Blocks Overall functionality of blocks [Type] Enhancement A suggestion for improvement. labels Oct 7, 2022
@cr0ybot
Copy link
Contributor

cr0ybot commented Oct 7, 2022

Yes! I have been waiting for this for so long!

@michalczaplinski michalczaplinski added the [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. label Oct 18, 2022
@michalczaplinski
Copy link
Contributor

I'm very eager to get this merged! Nice work @Soean! 👏

You need to add 'id' to $attributes_to_merge in get_block_wrapper_attributes() from the WP_Block_Supports class, see https://github.com/WordPress/wordpress-develop/blob/6.0.2/src/wp-includes/class-wp-block-supports.php#L178

Could you create a ticket for this in WordPress Core Trac so that a relevant change can be made there?

There's also a small merge conflict in the docs I see 🙂

@Soean
Copy link
Member Author

Soean commented Oct 19, 2022

@ndiego
Copy link
Member

ndiego commented Jan 30, 2023

Just tested this PR using a Social Link block, and it works great. For others testing, note that you need to manually apply the patch in WordPress/wordpress-develop#3497.

Editor:
image

Front end:
image

@carolinan
Copy link
Contributor

carolinan commented Jan 31, 2023

Would it be possible to make the anchor block support enabled by default for all blocks?

@github-actions
Copy link

github-actions bot commented Jan 31, 2023

Flaky tests detected in 291cced.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4053938383
📝 Reported issues:

@Mamaduka
Copy link
Member

@ndiego, do you see any errors in the DevTools console when testing this branch, with or without the core patch?

I think failing E2E tests are related to the changes in this PR.

@ndiego
Copy link
Member

ndiego commented Jan 31, 2023

@carolinan I am getting fatal errors now with the recent updates. Per @Mamaduka it looks like this PR needs to be updated due to #47195.

@carolinan
Copy link
Contributor

carolinan commented Jan 31, 2023

After updating to wp_should_skip_block_supports_serialization
I am still seeing this console warning in the editor:
traverse.js:26 Error while traversing the CSS: Error: undefined:1:10: missing '{'

Update: Ignore that ^, it was theme related.

@ndiego
Copy link
Member

ndiego commented Jan 31, 2023

I just retested and everything is looking good on my end.

@Soean
Copy link
Member Author

Soean commented Jan 31, 2023

Thanks for testing, what do we need to get it merged?

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

The changes here look good to me. Thank you, @Soean!

While there are many file updates, only the anchor.php contains significant changes.

@Soean, can you update WordPress/wordpress-develop#3497 and sync changes for the anchor.php after the merge?

Here's the tracking issue for PHP backports: #47187.

@Soean Soean merged commit 3bb63e4 into trunk Feb 1, 2023
@Soean Soean deleted the add-anchor-support branch February 1, 2023 07:51
@github-actions github-actions bot added this to the Gutenberg 15.1 milestone Feb 1, 2023
@Soean
Copy link
Member Author

Soean commented Feb 3, 2023

Thanks @Mamaduka. I synced the changes to the PR WordPress/wordpress-develop#3497, updated the trac ticket and added it to the tracking issue.

@Mamaduka
Copy link
Member

Mamaduka commented Feb 3, 2023

Thank you, @Soean!

@fabiankaegy fabiankaegy added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Feb 6, 2023
@fabiankaegy fabiankaegy mentioned this pull request Feb 6, 2023
47 tasks
@fabiankaegy
Copy link
Member

@Soean I added the Needs Dev Note label to this PR. Do you have time to write something up for it in the next few weeks (Deadline is March 3rd :) )

If not I can help with writing something up. :)

@t-hamano
Copy link
Contributor

I fear that this change may affect the backward compatibility of some blocks with the anchor. Please see #48232.

Comment on lines +13 to +28
function gutenberg_register_anchor_support( $block_type ) {
$has_anchor_support = _wp_array_get( $block_type->supports, array( 'anchor' ), true );
if ( ! $has_anchor_support ) {
return;
}

if ( ! $block_type->attributes ) {
$block_type->attributes = array();
}

if ( ! array_key_exists( 'anchor', $block_type->attributes ) ) {
$block_type->attributes['anchor'] = array(
'type' => 'string',
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This process would also apply to static blocks, which would add an extra comment as attributes.

@adamwoodnz
Copy link

I fear that this change may affect the backward compatibility of some blocks with the anchor. Please see #48232.

We've been building pages for wordpress.org and seeing blocks with existing anchors becoming unrecoverable. Typically Group blocks.

@bph
Copy link
Contributor

bph commented Feb 28, 2023

@fabiankaegy and @Soean Are either of you working the dev note for this PR?

  • Does it warrant a stand-alone post? If yes, please post a draft to the Make Core Blog.
  • If you only would write a paragraph or two, comment on this PR, and I'll add it to the Misc Editor Note.

If you could do write this by Thursday (March 2 EOB), that would be helpful. The release squad for 6.2 documentation, will start working on the Field guide for 6.2 on Friday.

@t-hamano
Copy link
Contributor

@bph
In this PR, we found a backward compatibility issue where blocks with anchors were broken, as reported in #47771. We are still discussing #48438 to fix this issue and how it should be addressed.

@t-hamano
Copy link
Contributor

t-hamano commented Mar 1, 2023

@bph
This change will be reverted in #48592 due to backward compatibility issues as reported in #48232. Here is the discussion on Slack. Therefore, I think Dev Note isn't necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks Needs Dev Note Requires a developer note for a major WordPress release cycle Needs User Documentation Needs new user documentation [Type] Enhancement A suggestion for improvement. [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Dynamic blocks - HTML Anchor is not supported