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

Jump to bottom

Interactivity Router: Update for latest GB changes. #7304

Closed
wants to merge 7 commits into from
+34 −25
Closed

Interactivity Router: Update for latest GB changes. #7304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c137e90
Add interactivity-router translations via script module data
sirreal Sep 6, 2024
0d310f3
Remove iAPI-specific screen reader text
sirreal Sep 20, 2024
252648f
Remove localized strings from state
sirreal Sep 20, 2024
c03c18b
Remove interactivity router screen reader test
sirreal Sep 24, 2024
531dea9
Revert "Remove localized strings from state"
sirreal Sep 24, 2024
f2ba5de
Restore some server-side init
sirreal Sep 24, 2024
e7c6cfe
Rename method to reflect its new behavior
sirreal Sep 26, 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
53 changes: 33 additions & 20 deletions src/wp-includes/interactivity-api/class-wp-interactivity-api.php
View file
Open in desktop
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 @@ -973,29 +994,21 @@ private function get_router_animation_styles(): string {
}

/**
* Outputs the markup for the top loading indicator and the screen reader
* notifications during client-side navigations.
* Outputs markup for the @wordpress/interactivity-router script module.
*
* This method prints a div element representing a loading bar visible during
* navigation, as well as an aria-live region that can be read by screen
* readers to announce navigation status.
* navigation.
*
* @since 6.5.0
* @since 6.7.0
*/
public function print_router_loading_and_screen_reader_markup() {
public function print_router_markup() {
echo <<<HTML
<div
class="wp-interactivity-router-loading-bar"
data-wp-interactive="core/router"
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:

wordpress-develop/src/wp-includes/class-wp-scripts.php

Lines 479 to 505 in 9e29083

/**
* 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 +1029,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 All @@ -1035,7 +1048,7 @@ private function data_wp_router_region_processor( WP_Interactivity_API_Directive
wp_enqueue_style( 'wp-interactivity-router-animations' );

// Adds the necessary markup to the footer.
add_action( 'wp_footer', array( $this, 'print_router_loading_and_screen_reader_markup' ) );
add_action( 'wp_footer', array( $this, 'print_router_markup' ) );
}
}

Expand Down
6 changes: 1 addition & 5 deletions tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-router-region.php
View file
Open in desktop
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