-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Supports: Change prefix in gutenberg_apply_colors_support to wp_ in dynamic blocks #51989
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not confident in how the prefixFunctions transforms work in this context, but this change will fix the navigation block issues when ported to a WP release.
Thank you, folks! 🚀 |
Size Change: 0 B Total Size: 1.45 MB ℹ️ View Unchanged
|
I believe that e2e tests are failing because they we use WordPress I'll quickly run them locally (with a working -- if not totally up-to-date -- WordPress) to verify. If they pass, I'm inclined to land this PR so I'll be able to publish another round of npms that includes this change, and to update |
Since tests are also failing locally (which might be inconsequential), I'm leaning towards skipping this for WP 6.3 Beta 1 and getting it into Beta 2 instead. I've now merged WordPress/wordpress-develop#4722 which should fix Core |
Flaky tests detected in fc0decb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5401763998
|
…_ in dynamic blocks
2097de7
to
fc0decb
Compare
The remaining e2e failure is present on a number of other recent commits to (It's a number of |
This PR is blocked until E2E test are passing. I am re-running now. |
Cherry-picked to |
@ramonjd @tellthemachines I guess we also might want this in a GB 16.1.x point release? |
@ockham Thanks for the ping.
As long as WordPress core is not referring to it, it will be fine. Probably not ideal from a consistency point of view however. This consistency, of course, will be there on the next release of Gutenberg (16.2) thanks to this PR's changes. It's totally feasible for PHP block code that lives only in the plugin to refer to Only during syncing Gutenberg with Core do we need to make sure these don't creep into the Core code. We all know how well that works out :) There some initiatives under way to prevent such situations, e.g., https://core.trac.wordpress.org/ticket/56794 |
…_ in dynamic blocks (WordPress#51989)
What?
In the Navigation Submenu dynamic block, rename
gutenberg_apply_colors_support
towp_apply_colors_support
, and add the latter to the list of function calls that are rewritten during block build time.Why?
For the Core merge, see WordPress/wordpress-develop#4722 (comment).
How?
Uses the existing mechanism in Webpack to rename functions during block build. See e.g.
wp_get_typography_font_size_value
/gutenberg_get_typography_font_size_value
for precedent.Testing Instructions
Verify that "Color" block supports still works.