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.

Sign up for GitHub

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
merged 9 commits into from
Feb 12, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d80395e
Update file block
SantosGuillamot Feb 12, 2024
ee0c679
Update image block
SantosGuillamot Feb 12, 2024
4707658
Update navigation block
SantosGuillamot Feb 12, 2024
1178a0b
Update query block
SantosGuillamot Feb 12, 2024
3d8cecc
WIP: Update form block
SantosGuillamot Feb 12, 2024
70742f0
Use boolean instead of string in `$open_by_default` variable
SantosGuillamot Feb 12, 2024
ab8ac4d
Don't use `data-wp-interactive` object in search block
SantosGuillamot Feb 12, 2024
4a95840
Remove unnecessary quotes
SantosGuillamot Feb 12, 2024
86ffce0
Adapt query block unit test
SantosGuillamot Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/block-library/src/file/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static function ( $matches ) {

$processor = new WP_HTML_Tag_Processor( $content );
$processor->next_tag();
$processor->set_attribute( 'data-wp-interactive', '{"namespace":"core/file"}' );
$processor->set_attribute( 'data-wp-interactive', 'core/file' );
$processor->next_tag( 'object' );
$processor->set_attribute( 'data-wp-bind--hidden', '!state.hasPdfPreview' );
$processor->set_attribute( 'hidden', true );
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/image/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function block_core_image_render_lightbox( $block_content, $block ) {
$figure_class_names = $p->get_attribute( 'class' );
$figure_styles = $p->get_attribute( 'style' );
$p->add_class( 'wp-lightbox-container' );
$p->set_attribute( 'data-wp-interactive', '{"namespace":"core/image"}' );
$p->set_attribute( 'data-wp-interactive', 'core/image' );
$p->set_attribute(
'data-wp-context',
wp_json_encode(
Expand Down Expand Up @@ -240,7 +240,7 @@ function block_core_image_print_lightbox_overlay() {
echo <<<HTML
<div
class="wp-lightbox-overlay zoom"
data-wp-interactive='{"namespace":"core/image"}'
data-wp-interactive="core/image"
data-wp-context='{}'
data-wp-bind--role="state.roleAttribute"
data-wp-bind--aria-label="state.currentImage.ariaLabel"
Expand Down
12 changes: 5 additions & 7 deletions packages/block-library/src/navigation/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,19 +557,17 @@ private static function get_nav_element_directives( $is_interactive, $attributes
return '';
}
// When adding to this array be mindful of security concerns.
$nav_element_context = wp_json_encode(
$nav_element_context = data_wp_context(
array(
'overlayOpenedBy' => array(),
'type' => 'overlay',
'roleAttribute' => '',
'ariaLabel' => __( 'Menu' ),
),
JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP
)
);
$nav_element_directives = '
data-wp-interactive=\'{"namespace":"core/navigation"}\'
data-wp-context=\'' . $nav_element_context . '\'
';
data-wp-interactive="core/navigation"'
. $nav_element_context;

/*
* When the navigation's 'overlayMenu' attribute is set to 'always', JavaScript
Expand Down Expand Up @@ -780,7 +778,7 @@ function block_core_navigation_add_directives_to_submenu( $tags, $block_attribut
)
) ) {
// Add directives to the parent `<li>`.
$tags->set_attribute( 'data-wp-interactive', '{ "namespace": "core/navigation" }' );
$tags->set_attribute( 'data-wp-interactive', 'core/navigation' );
$tags->set_attribute( 'data-wp-context', '{ "submenuOpenedBy": {}, "type": "submenu" }' );
$tags->set_attribute( 'data-wp-watch', 'callbacks.initMenu' );
$tags->set_attribute( 'data-wp-on--focusout', 'actions.handleMenuFocusout' );
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/query/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function render_block_core_query( $attributes, $content, $block ) {
$p = new WP_HTML_Tag_Processor( $content );
if ( $p->next_tag() ) {
// Add the necessary directives.
$p->set_attribute( 'data-wp-interactive', '{"namespace":"core/query"}' );
$p->set_attribute( 'data-wp-interactive', 'core/query' );
$p->set_attribute( 'data-wp-router-region', 'query-' . $attributes['queryId'] );
$p->set_attribute( 'data-wp-init', 'callbacks.setQueryRef' );
$p->set_attribute( 'data-wp-context', '{}' );
Expand Down
20 changes: 14 additions & 6 deletions packages/block-library/src/search/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function render_block_core_search( $attributes ) {
$border_color_classes = get_border_color_classes_for_block_core_search( $attributes );
// This variable is a constant and its value is always false at this moment.
// It is defined this way because some values depend on it, in case it changes in the future.
$open_by_default = 'false';
$open_by_default = false;

$label_inner_html = empty( $attributes['label'] ) ? __( 'Search' ) : wp_kses_post( $attributes['label'] );
$label = new WP_HTML_Tag_Processor( sprintf( '<label %1$s>%2$s</label>', $inline_styles['label'], $label_inner_html ) );
Expand Down Expand Up @@ -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.

Ticket created: https://core.trac.wordpress.org/ticket/60575#ticket

array(
'isSearchInputVisible' => $open_by_default,
'inputId' => $input_id,
'ariaLabelExpanded' => $aria_label_expanded,
'ariaLabelCollapsed' => $aria_label_collapsed,
)
);
$form_directives = '
data-wp-interactive=\'{ "namespace": "core/search" }\'
data-wp-context=\'{ "isSearchInputVisible": ' . $open_by_default . ', "inputId": "' . $input_id . '", "ariaLabelExpanded": "' . $aria_label_expanded . '", "ariaLabelCollapsed": "' . $aria_label_collapsed . '" }\'
data-wp-class--wp-block-search__searchfield-hidden="!context.isSearchInputVisible"
data-wp-on--keydown="actions.handleSearchKeydown"
data-wp-on--focusout="actions.handleSearchFocusout"
data-wp-interactive=\'"core/search"\''
. $form_context .
'data-wp-class--wp-block-search__searchfield-hidden="!context.isSearchInputVisible"
data-wp-on--keydown="actions.handleSearchKeydown"
data-wp-on--focusout="actions.handleSearchFocusout"
';
}

Expand Down
2 changes: 1 addition & 1 deletion phpunit/blocks/render-query-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public function test_rendering_query_with_enhanced_pagination() {
$p->next_tag( array( 'class_name' => 'wp-block-query' ) );
$this->assertSame( '{}', $p->get_attribute( 'data-wp-context' ) );
$this->assertSame( 'query-0', $p->get_attribute( 'data-wp-router-region' ) );
$this->assertSame( '{"namespace":"core/query"}', $p->get_attribute( 'data-wp-interactive' ) );
$this->assertSame( 'core/query', $p->get_attribute( 'data-wp-interactive' ) );

$p->next_tag( array( 'class_name' => 'wp-block-post' ) );
$this->assertSame( 'post-template-item-' . self::$posts[1], $p->get_attribute( 'data-wp-key' ) );
Expand Down
Loading