-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Gallery Block: Use wp_enqueue_block_support_styles()
#43859
Conversation
wp_enqueue_block_support_styles()
Size Change: 0 B Total Size: 1.25 MB ℹ️ View Unchanged
|
The code looks good, but it would be great to get confirmation that it works as expected. |
( match ) => | ||
`${ prefix }${ match.replace( | ||
/^wp_/, | ||
'' | ||
) }` |
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.
I'm not super familiar with those files but isn't this already done in the same file below? 🤔
gutenberg/tools/webpack/blocks.js
Lines 162 to 169 in e21975b
// Prepend the Gutenberg prefix, substituting any | |
// other core prefix (e.g. "wp_"). | |
return result.replace( | |
new RegExp( functionName, 'g' ), | |
( match ) => | |
prefix + | |
match.replace( /^wp_/, '' ) | |
); |
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.
Oh, wait forget it! The snippet I linked is replacing inside of the function definitions. You were doing it in the function calls 🤦 .
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.
I've tested it and seems to work just fine:
2022-09-05_15-30-48.mp4
This approach LGTM. Thanks for pursuing it @ockham |
Tested and everything still working as in the original implementation, ie. styles added to very end of header for block themes, and footer for non block themes 🎉 |
Thanks a lot for testing and reviewing @gziolo @michalczaplinski @ramonjd @glendaviesnz! |
What?
In the Gallery Block, use
wp_enqueue_block_support_styles()
.Alternative approach to #43779, based on @gziolo's suggestion: #43779 (comment)
Why?
To enable us to use the two-argument version of
wp_enqueue_block_support_styles()
(which has been backported to Core here), and eventually remove thegutenberg_enqueue_block_support_styles()
shim.How?
When used within the Gutenberg plugin, we have some code in the Webpack config that will change any occurrence of
wp_enqueue_block_support_styles()
togutenberg_enqueue_block_support_styles()
to use the version of the function that supports the second argument ($priority
).Testing Instructions
Verify that the Gallery block still works.
Furthermore, run
npm run build
. Inspectbuild/block-library/blocks/gallery.php
, and verify that it has a call togutenberg_enqueue_block_support_styles()
(instead ofwp_enqueue_block_support_styles()
).