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.

Sign up for GitHub

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

Jump to bottom

Block Bindings: Update source registration syntax and remove APIs that should be private #58205

Merged
merged 7 commits into from
Jan 24, 2024
Merged

Block Bindings: Update source registration syntax and remove APIs that should be private #58205

merged 7 commits into from
Jan 24, 2024

Conversation

michalczaplinski
Copy link
Contributor

There are a few changes in this PR:

Following WordPress/wordpress-develop#5888 I've:

  • Moved the source registration to an "array" syntax
  • Removed the replace_html() method of the WP_Block_Bindings class
  • Moved the body of replace_html() from the WP_Block_Bindings class to the block-bindings.php file

@michalczaplinski michalczaplinski force-pushed the update/block-bindings-api-shape-and-code-quality branch from 6ada5c3 to ddd569e Compare January 24, 2024 15:52
@github-actions GitHub Actions
Copy link

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/compat/wordpress-6.5/block-bindings/block-bindings.php
❔ lib/compat/wordpress-6.5/block-bindings/class-wp-block-bindings.php
❔ lib/compat/wordpress-6.5/block-bindings/sources/pattern.php
❔ lib/compat/wordpress-6.5/block-bindings/sources/post-meta.php
❔ lib/compat/wordpress-6.5/blocks.php

@michalczaplinski michalczaplinski marked this pull request as ready for review January 24, 2024 15:56
@michalczaplinski michalczaplinski added [Type] Enhancement A suggestion for improvement. [Feature] Block API API that allows to express the block paradigm. [Feature] Custom Fields Anything related to the custom fields project - connecting block attributes and dynamic values labels Jan 24, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 24, 2024 •
edited
Loading

Flaky tests detected in c9a245e.
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/7644439081
📝 Reported issues:

gziolo
gziolo reviewed Jan 24, 2024
View reviewed changes
lib/compat/wordpress-6.5/block-bindings/sources/pattern.php
@@ -29,7 +29,9 @@
};
wp_block_bindings_register_source(
'pattern_attributes',
__( 'Pattern Attributes', 'gutenberg' ),
$pattern_source_callback
array(
Copy link
Member

Choose a reason for hiding this comment

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

There should be no textdomain used for translations expected to land in core.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I removed it for both the pattern and post-meta sources in c9a245e

Base automatically changed from update/block-bindings-remove-experimental-flag to trunk January 24, 2024 16:51
@artemiomorales artemiomorales force-pushed the update/block-bindings-api-shape-and-code-quality branch from cc29eac to 3fda653 Compare January 24, 2024 17:15
artemiomorales
artemiomorales approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@artemiomorales artemiomorales left a comment

Choose a reason for hiding this comment

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

Pending passing tests, looks good to me!

SantosGuillamot
SantosGuillamot approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@SantosGuillamot SantosGuillamot left a comment

Choose a reason for hiding this comment

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

I've tested it, and everything seems to keep working fine 🙂

@michalczaplinski michalczaplinski merged commit ea32957 into trunk Jan 24, 2024
55 checks passed
@michalczaplinski michalczaplinski deleted the update/block-bindings-api-shape-and-code-quality branch January 24, 2024 18:46
@github-actions github-actions bot added this to the Gutenberg 17.6 milestone Jan 24, 2024
cbravobernal
cbravobernal reviewed Jan 24, 2024
View reviewed changes
lib/compat/wordpress-6.5/block-bindings/block-bindings.php
Comment on lines +90 to +92
// TODO: This shouldn't be needed when the `set_inner_html` function is ready.
// Store the parent tag and its attributes to be able to restore them later in the button.
// The button block has a wrapper while the paragraph and heading blocks don't.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are backporting this file, make sure that multilines comments are with /* */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Carlos! The backport is here btw WordPress/wordpress-develop#5888 if you ever want to have a look.

@SantosGuillamot SantosGuillamot mentioned this pull request Jan 25, 2024
Closed
42 tasks
@getdave getdave mentioned this pull request Jan 26, 2024
Closed
@getdave getdave added the Needs PHP backport Needs PHP backport to Core label Jan 26, 2024
@getdave
Copy link
Contributor

getdave commented Jan 26, 2024

I noticed this PR was merged after I raised the PHP Sync Tracking Issue for WP 6.5 and has changed PHP files that may need backporting to WP Core.

Please forgive the ping, but I marked as Needs PHP backport and also added to the tracking Issue.

@michalczaplinski
Copy link
Contributor Author

@getdave Backported to Core in WordPress/wordpress-develop#5888

@michalczaplinski michalczaplinski added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Needs PHP backport Needs PHP backport to Core labels Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbravobernal cbravobernal cbravobernal left review comments

@gziolo gziolo gziolo left review comments

@SantosGuillamot SantosGuillamot SantosGuillamot approved these changes

@artemiomorales artemiomorales artemiomorales approved these changes

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey spacedmonkey is a code owner

@kevin940726 kevin940726 Awaiting requested review from kevin940726

@Mamaduka Mamaduka Awaiting requested review from Mamaduka

Assignees
No one assigned
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Feature] Block API API that allows to express the block paradigm. [Feature] Custom Fields Anything related to the custom fields project - connecting block attributes and dynamic values [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 17.6
Development

Successfully merging this pull request may close these issues.
6 participants
@michalczaplinski @getdave @gziolo @artemiomorales @SantosGuillamot @cbravobernal