-
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
Use data_wp_context
helper in core blocks and remove data-wp-interactive
object
#58943
Use data_wp_context
helper in core blocks and remove data-wp-interactive
object
#58943
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
Some tests need to be updated, in example:
Tests_Blocks_RenderQueryBlock::test_rendering_query_with_enhanced_pagination
expected -'{"namespace":"core/query"}'
returned +'core/query'
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❔ phpunit/blocks/render-query-test.php |
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.
Thanks for the refactor Mario!
We should update the api reference too:
https://github.com/WordPress/gutenberg/blob/trunk/packages/interactivity/docs/2-api-reference.md#wp-interactive
You're right! Should it be part of this PR or should we add a follow-up task? By the way, are there tests for the new |
A follow-up task.
Yep. |
* trunk: (273 commits) Remove preffered style variations legacy support (#58930) Style theme variations: add property extraction and merge utils (#58803) Migrate `change-detection` to Playwright (#58767) Update Changelog for 17.6.6 Docs: Clarify the status of the wp-block-styles theme support, and its intent (#58915) Use `data_wp_context` helper in core blocks and remove `data-wp-interactive` object (#58943) Try double enter for details block. (#58903) Template revisions API: move from experimental to compat/6.4 (#58920) Editor: Remove inline toolbar preference (#58945) Clean up link control CSS. (#58934) Font Library: Show error message when no fonts found to install (#58914) Block Bindings: lock editing of blocks by default (#58787) Editor: Remove the 'all' rendering mode (#58935) Pagination Numbers: Add `data-wp-key` to pagination numbers if enhanced pagination is enabled (#58189) Close link preview if collapsed selection when creating link (#58896) Fix incorrect useAnchor positioning when switching from virtual to rich text elements (#58900) Upgrade Floating UI packages, fix nested iframe positioning bug (#58932) Site editor: fix start patterns store selector (#58813) Revert "Rich text: pad multiple spaces through en/em replacement (#56341)" (#58792) Documentation: Clarify the performance reference commit and how to pick it (#58927) ...
…active` object (#58943) * Update file block * Update image block * Update navigation block * Update query block * WIP: Update form block * Use boolean instead of string in `$open_by_default` variable * Don't use `data-wp-interactive` object in search block * Remove unnecessary quotes * Adapt query block unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: c4rl0sbr4v0 <cbravobernal@git.wordpress.org>
I just cherry-picked this PR to the backports/beta1 branch to get it included in the next release: 87c120a |
…active` object (#58943) * Update file block * Update image block * Update navigation block * Update query block * WIP: Update form block * Use boolean instead of string in `$open_by_default` variable * Don't use `data-wp-interactive` object in search block * Remove unnecessary quotes * Adapt query block unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: c4rl0sbr4v0 <cbravobernal@git.wordpress.org>
…active` object (#58943) * Update file block * Update image block * Update navigation block * Update query block * WIP: Update form block * Use boolean instead of string in `$open_by_default` variable * Don't use `data-wp-interactive` object in search block * Remove unnecessary quotes * Adapt query block unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: c4rl0sbr4v0 <cbravobernal@git.wordpress.org>
What alternatives have you considered to make it compatible with the tag processor? It would be useful to be able to use it with the tag processor, too. |
I believe it was discussed at some point to create a method for that. Something like |
@@ -179,12 +179,20 @@ function render_block_core_search( $attributes ) { | |||
if ( $is_expandable_searchfield ) { | |||
$aria_label_expanded = __( 'Submit Search' ); | |||
$aria_label_collapsed = __( 'Expand search field' ); | |||
$form_context = data_wp_context( |
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.
What is this function? Are we sure the naming is correct? It's not clear to me what it does based on its name.
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.
From what I understood, it was included as part of the Interactivity API server directive processing: link.
@DAreRodz @c4rl0sbr4v0 Do you know if the name was discussed at some point? Should it be prefixed by wp_interactivity_
as the other functions?
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.
There was a discussion, but rather for using a function to generate a context that naming it:
#53102 (reply in thread)
If it is not clear, we can refactor it to wp_interactivitity_create_data_wp_context
. Or wp_interactivity_create_context_directive
or wp_interactivity_print_context_directive
.
What do you think?
Let's do some pings to join the discussion, sorry for that 😅
@ryanwelcher , @swissspidy , @westonruter , @youknowriad , @gziolo
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.
An inline thread on a closed Gutenberg pull request is really not the best place to discuss naming of WP core functions that are already in core... Can we move this elsewhere with more visibility, like a Trac ticket?
I agree that the name is not the best. Unfortunately when reviewing WordPress/wordpress-develop#5953 this wasn't more obvious to me, so thanks for raising this. It's definitely an outlier as the only unprefixed function in this file.
Something like wp_interactivity_get_context_attribute
, wp_interactivity_context_attr
would be more indicative of what the function does for sure.
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.
Ticket created: https://core.trac.wordpress.org/ticket/60575#ticket
What?
I'm using some of the latest changes in the Interactivity API in the core blocks:
data_wp_context
helper. Although it doesn't seem possible to use it in the blocks using the tag processor.data-wp-interactive="core/image"
instead of the whole object.Why?
These changes simplify the code and they serve as a potential example.
How?
I just modified the directives of the existing blocks.
Testing Instructions