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

Block Bindings: Remove the experimental flag #58089

Merged
merged 10 commits into from
Jan 24, 2024

Conversation

michalczaplinski
Copy link
Contributor

@michalczaplinski michalczaplinski commented Jan 22, 2024

What?

Following up on #56867 and #57742.

Removes the experimental flag for Block Bindings:

  • Removes the experiment from the GB experiments page.
  • Removes all the places where we check for the experiment
  • Also remove one instance of unnecessary gutenberg-partial-pattern-syncing check

TIP: Hide the whitespace when looking at the diff.

Testing Instructions

  1. Register a custom field. E.g.:

    register_meta(
     'post',
     'text_custom_field',
     array(
     	'show_in_rest' => true,
     	'single'       => true,
     	'type'         => 'string',
     	'default'	   => 'Custom Text',
        )
     );
  2. Open a post or page.

  3. Insert a Paragraph block

  4. Open the Code Editor

  5. Inside the Paragraph block, add the following snippet which adds a binding:

    {"metadata":{"bindings":{"content":{"source":{"name":"post_meta","attributes":{"value":"text_custom_field"}}}}}}
  6. Open the frontend of the site and make sure that the text_custom_field is present as the content of the Paragraph.

  7. Also, make sure that the "Block bindings" experiement is no longer present on the GB Experiments page.

Copy link

github-actions bot commented Jan 22, 2024

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/blocks.php
❔ lib/experimental/blocks.php
❔ lib/experimental/editor-settings.php
❔ lib/experiments-page.php
❔ lib/load.php
❔ 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

@michalczaplinski michalczaplinski added [Feature] Block API API that allows to express the block paradigm. [Type] Code Quality Issues or PRs that relate to code quality [Type] Experimental Experimental feature or API. [Feature] Custom Fields Anything related to the custom fields project - connecting block attributes and dynamic values and removed [Type] Experimental Experimental feature or API. labels Jan 22, 2024
Copy link

github-actions bot commented Jan 22, 2024

Flaky tests detected in 0fc3262.
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/7643069301
📝 Reported issues:

@michalczaplinski
Copy link
Contributor Author

Looks like the unit test failures are due to this: https://core.trac.wordpress.org/ticket/60309

@talldan
Copy link
Contributor

talldan commented Jan 24, 2024

@michalczaplinski The PR removing the pattern overrides experiment is now merged, so there are a few conflicts to solve!

I had a go at rebasing the branch against trunk. Rather than push to your PR I've pushed it as a separate branch so that you can decide what to do - https://github.com/WordPress/gutenberg/compare/update/block-bindings-remove-experimental-flag-rebase?expand=1

If that looks good you can reset your branch to the same state.

If you decide to solve the conflicts yourself, you'll need to cherry-pick 3e611d0 onto your branch. That commit updates the e2e tests for pattern overrides so that the block bindings experiment isn't enabled as part of the test set up.

@michalczaplinski michalczaplinski force-pushed the update/block-bindings-remove-experimental-flag branch from b0b4940 to 3e611d0 Compare January 24, 2024 10:35

This comment was marked as resolved.

@michalczaplinski
Copy link
Contributor Author

Awesome, thank you @talldan ! 🙏

Your rebase looks good to me so I've just pushed the same commits to this branch.

lib/compat/wordpress-6.5/block-bindings/index.php Outdated Show resolved Hide resolved
lib/compat/wordpress-6.5/blocks.php Outdated Show resolved Hide resolved
lib/compat/wordpress-6.5/blocks.php Outdated Show resolved Hide resolved
@michalczaplinski
Copy link
Contributor Author

Thanks for the review @c4rl0sbr4v0 !

I've realized I should make the changes suggested in the Core backport here as well: WordPress/wordpress-develop#5888 (review)

@michalczaplinski
Copy link
Contributor Author

Actually, those changes would be better in a separate PR so that we keep it clean, but we should also merge them today. I ll work on it ASAP.

@cbravobernal
Copy link
Contributor

cbravobernal commented Jan 24, 2024

Hi @michalczaplinski !

I followed the instructions provided, and is not working.

Example provided:

{"metadata":{"bindings":{"text":{"source":{"name":"post_meta","attributes":{"value":"text_custom_field"}}}}}}

Part of the code that handles the custom field printing:

// If the attribute is not in the list, process next attribute.
	if ( ! in_array( $binding_attribute, $allowed_blocks[ $block_instance->name ], true ) ) {
		continue;
	}

In this case, $allowed_blocks[ $block_instance->name ] returns content, so changing the example to:

{"metadata":{"bindings":{"content":{"source":{"name":"post_meta","attributes":{"value":"text_custom_field"}}}}}}

makes it work.

Is this expected? I'm worried that I don't have enough context to confirm it, so let me know!

@artemiomorales
Copy link
Contributor

artemiomorales commented Jan 24, 2024

@c4rl0sbr4v0

Heads up that the JSON for the paragraph in the testing instructions was incorrect and was attempting to bind the text attribute rather than the content attribute, as you've noted.

So in the description, I changed it from this:

{"metadata":{"bindings":{"text":{"source":{"name":"post_meta","attributes":{"value":"text_custom_field"}}}}}}

To this:

{"metadata":{"bindings":{"content":{"source":{"name":"post_meta","attributes":{"value":"text_custom_field"}}}}}}

Copy link
Contributor

@cbravobernal cbravobernal left a comment

Choose a reason for hiding this comment

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

Work as expected.

@michalczaplinski michalczaplinski force-pushed the update/block-bindings-remove-experimental-flag branch from 1be72e2 to 5c29613 Compare January 24, 2024 15:01
@michalczaplinski michalczaplinski force-pushed the update/block-bindings-remove-experimental-flag branch from 5c29613 to a69d979 Compare January 24, 2024 15:17
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 work fine. 🙂 I tested all the blocks that are currently supported, a query loop, and the pattern overrides. Additionally, the experiments option is not there anymore.

@michalczaplinski michalczaplinski merged commit 015d8b5 into trunk Jan 24, 2024
56 checks passed
@michalczaplinski michalczaplinski deleted the update/block-bindings-remove-experimental-flag branch January 24, 2024 16:51
@github-actions github-actions bot added this to the Gutenberg 17.6 milestone Jan 24, 2024
@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 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
@michalczaplinski
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants