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

Interactivity Router: Update for latest GB changes. #7304

Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
41 changes: 28 additions & 13 deletions src/wp-includes/interactivity-api/class-wp-interactivity-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,26 @@ public function print_client_interactivity_data() {
_deprecated_function( __METHOD__, '6.7.0' );
}

/**
* Set client-side interactivity-router data.
*
* Once in the browser, the state will be parsed and used to hydrate the client-side
* interactivity stores and the configuration will be available using a `getConfig` utility.
*
* @since 6.7.0
*
* @param array $data Data to filter.
* @return array Data for the Interactivity Router script module.
*/
public function filter_script_module_interactivity_router_data( array $data ): array {
sirreal marked this conversation as resolved.
Show resolved Hide resolved
if ( ! isset( $data['i18n'] ) ) {
$data['i18n'] = array();
}
$data['i18n']['loading'] = __( 'Loading page, please wait.' );
$data['i18n']['loaded'] = __( 'Page Loaded.' );
return $data;
}

/**
* Set client-side interactivity data.
*
Expand Down Expand Up @@ -296,6 +316,7 @@ public function register_script_modules() {
*/
public function add_hooks() {
add_filter( 'script_module_data_@wordpress/interactivity', array( $this, 'filter_script_module_interactivity_data' ) );
add_filter( 'script_module_data_@wordpress/interactivity-router', array( $this, 'filter_script_module_interactivity_router_data' ) );
}

/**
Expand Down Expand Up @@ -990,12 +1011,6 @@ class="wp-interactivity-router-loading-bar"
data-wp-class--start-animation="state.navigation.hasStarted"
data-wp-class--finish-animation="state.navigation.hasFinished"
></div>
<div
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to bring this up after having already approved!

I've just noticed that we should rename the function to print_router_loading_markup(), update its docstring and update the reference where we pass this function to the filter over here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a public method, extenders could be calling it or modifying its behavior in filters. Renaming the method could be disruptive. It's true the method name isn't a perfect description now. What do you think of adding a note in the method documentation instead of changing the name?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could do this:

  • Create a new print_router_markup function
  • Deprecate the “old” method, and have it log a deprecation.
  • The “old” method calls the new print_router_markup function.

I think that might be the way to go.

I’m looking at some existing functions/methods that have the @deprecated docstring. For example this function is deprecated, calls _deprecated_function() and then calls the updated version:

/**
* Prints inline scripts registered for a specific handle.
*
* @since 4.5.0
* @deprecated 6.3.0 Use methods get_inline_script_tag() or get_inline_script_data() instead.
*
* @param string $handle Name of the script to print inline scripts for.
* Must be lowercase.
* @param string $position Optional. Whether to add the inline script
* before the handle or after. Default 'after'.
* @param bool $display Optional. Whether to print the script tag
* instead of just returning the script data. Default true.
* @return string|false Script data on success, false otherwise.
*/
public function print_inline_script( $handle, $position = 'after', $display = true ) {
_deprecated_function( __METHOD__, '6.3.0', 'WP_Scripts::get_inline_script_data() or WP_Scripts::get_inline_script_tag()' );
$output = $this->get_inline_script_data( $handle, $position );
if ( empty( $output ) ) {
return false;
}
if ( $display ) {
echo $this->get_inline_script_tag( $handle, $position );
}
return $output;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I've searched for the method name and it only appears in Gutenberg across searchable plugins and themes.

Let's just rename it.

Copy link
Contributor

Choose a reason for hiding this comment

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

All right!

class="screen-reader-text"
aria-live="polite"
data-wp-interactive="core/router"
data-wp-text="state.navigation.message"
></div>
HTML;
}

Expand All @@ -1016,16 +1031,16 @@ private function data_wp_router_region_processor( WP_Interactivity_API_Directive
if ( 'enter' === $mode && ! $this->has_processed_router_region ) {
$this->has_processed_router_region = true;

// Initialize the `core/router` store.
/*
* Initialize the `core/router` store.
* If the store is not initialized like this with minimal
* navigation object, the interactivity-router script module
* errors.
*/
$this->state(
'core/router',
array(
'navigation' => array(
'texts' => array(
'loading' => __( 'Loading page, please wait.' ),
'loaded' => __( 'Page Loaded.' ),
),
),
'navigation' => new stdClass(),
sirreal marked this conversation as resolved.
Show resolved Hide resolved
)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public function test_wp_router_region_missing() {
*
* @covers ::process_directives
*/
public function test_wp_router_region_adds_loading_bar_aria_live_region_only_once() {
public function test_wp_router_region_adds_loading_bar_region_only_once() {
$html = '
<div data-wp-router-region="region A">Interactive region</div>
<div data-wp-router-region="region B">Another interactive region</div>
Expand All @@ -125,9 +125,5 @@ public function test_wp_router_region_adds_loading_bar_aria_live_region_only_onc
$p = new WP_HTML_Tag_Processor( $footer );
$this->assertTrue( $p->next_tag( $query ) );
$this->assertFalse( $p->next_tag( $query ) );
$query = array( 'class_name' => 'screen-reader-text' );
$p = new WP_HTML_Tag_Processor( $footer );
$this->assertTrue( $p->next_tag( $query ) );
$this->assertFalse( $p->next_tag( $query ) );
}
}
Loading