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

Use data_wp_context helper in core blocks and remove data-wp-interactive object #58943

Merged

Conversation

SantosGuillamot
Copy link
Contributor

What?

I'm using some of the latest changes in the Interactivity API in the core blocks:

  • Use data_wp_context helper. Although it doesn't seem possible to use it in the blocks using the tag processor.
  • Use 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

  • Add an image -> click the link button -> enable expand on click. Check it works in the frontend.
  • Add a navigation block with the overlay menu set to always. Check it is interactive in the frontend.
  • Add a search block -> Select the option button only. Check it is interactive in the frontend.
  • Add a query block -> Disable force reload option. Check it is interactive in the frontend.
  • Add a file block -> Enable the show inline embed option. Check the embed is shown in desktop and hidden in mobile.

Copy link

github-actions bot commented Feb 12, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: SantosGuillamot <[email protected]>
Co-authored-by: c4rl0sbr4v0 <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@SantosGuillamot SantosGuillamot added [Type] Code Quality Issues or PRs that relate to code quality Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Feature] Interactivity API API to add frontend interactivity to blocks. labels Feb 12, 2024
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.

Some tests need to be updated, in example:

Tests_Blocks_RenderQueryBlock::test_rendering_query_with_enhanced_pagination

expected -'{"namespace":"core/query"}'
returned  +'core/query'

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
❔ phpunit/blocks/render-query-test.php

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.

@cbravobernal cbravobernal enabled auto-merge (squash) February 12, 2024 19:21
@SantosGuillamot
Copy link
Contributor Author

We should update the api reference too:

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 data-wp-interactive format and the data_wp_context function?

@cbravobernal cbravobernal merged commit 535252e into trunk Feb 12, 2024
57 checks passed
@cbravobernal cbravobernal deleted the update/use-latest-interactivity-apis-in-core-blocks branch February 12, 2024 19:46
@github-actions github-actions bot added this to the Gutenberg 17.8 milestone Feb 12, 2024
@cbravobernal
Copy link
Contributor

You're right! Should it be part of this PR or should we add a follow-up task?

A follow-up task.

By the way, are there tests for the new data-wp-interactive format and the data_wp_context function?

Yep.

aristath added a commit that referenced this pull request Feb 13, 2024
* 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)
  ...
youknowriad pushed a commit that referenced this pull request Feb 13, 2024
…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 <[email protected]>
Co-authored-by: c4rl0sbr4v0 <[email protected]>
@youknowriad
Copy link
Contributor

I just cherry-picked this PR to the backports/beta1 branch to get it included in the next release: 87c120a

@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 13, 2024
youknowriad pushed a commit that referenced this pull request Feb 13, 2024
…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 <[email protected]>
Co-authored-by: c4rl0sbr4v0 <[email protected]>
youknowriad pushed a commit that referenced this pull request Feb 13, 2024
…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 <[email protected]>
Co-authored-by: c4rl0sbr4v0 <[email protected]>
@gziolo
Copy link
Member

gziolo commented Feb 16, 2024

Use data_wp_context helper. Although it doesn't seem possible to use it in the blocks using the tag processor.

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.

@SantosGuillamot
Copy link
Contributor Author

What alternatives have you considered to make it compatible with the tag processor?

I believe it was discussed at some point to create a method for that. Something like $p->set_context().

@@ -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(
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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

Copy link
Member

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Interactivity API API to add frontend interactivity to blocks. [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants