From 66d0dbbaf726989a6b731356e74d7a619e3c895c Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 12:22:53 +0100 Subject: [PATCH 01/25] Expose state related to navigations --- packages/interactivity-router/src/index.js | 41 +++++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 7396c21e9638d1..d11aadfa3d879e 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -60,9 +60,6 @@ const renderRegions = ( page ) => { } }; -// Variable to store the current navigation. -let navigatingTo = ''; - // Listen to the back and forward buttons and restore the page if it's in the // cache. window.addEventListener( 'popstate', async () => { @@ -82,6 +79,15 @@ pages.set( ); export const { state, actions } = store( 'core/router', { + state: { + url: '', + navigation: { + target: '', + hasStarted: false, + hasFinished: false, + texts: {}, + }, + }, actions: { /** * Navigates to the specified page. @@ -108,7 +114,8 @@ export const { state, actions } = store( 'core/router', { */ *navigate( href, options = {} ) { const url = cleanUrl( href ); - navigatingTo = href; + const { navigation } = state; + navigation.target = href; actions.prefetch( url, options ); // Create a promise that resolves when the specified timeout ends. @@ -117,21 +124,45 @@ export const { state, actions } = store( 'core/router', { setTimeout( resolve, options.timeout ?? 10000 ) ); + // Don't update the navigation status immediately, wait 400 ms. + const timeout = setTimeout( () => { + if ( navigation.target === href ) { + navigation.hasStarted = true; + navigation.hasFinished = false; + navigation.message = navigation.texts.loading; + } + }, 400 ); + const page = yield Promise.race( [ pages.get( url ), timeoutPromise, ] ); + // Dismiss loading message if it hasn't been added yet. + clearTimeout( timeout ); + // Once the page is fetched, the destination URL could have changed // (e.g., by clicking another link in the meantime). If so, bail // out, and let the newer execution to update the HTML. - if ( navigatingTo !== href ) return; + if ( navigation.target !== href ) return; + + navigation.hasStarted = false; + navigation.hasFinished = true; + // Announce that the page has been loaded. If the message is the + // same, we use a no-break space similar to the @wordpress/a11y + // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 + navigation.message = + navigation.texts.loaded + + ( navigation.message === navigation.texts.loaded + ? '\u00A0' + : '' ); if ( page ) { renderRegions( page ); window.history[ options.replace ? 'replaceState' : 'pushState' ]( {}, '', href ); + state.url = href; } else { window.location.assign( href ); yield new Promise( () => {} ); From 07bbaadda9876bf2e7eeb76615a80cb3c18dd1d0 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 12:23:29 +0100 Subject: [PATCH 02/25] Remove loading bar and aria region from query block --- packages/block-library/src/query/index.php | 37 +--------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/packages/block-library/src/query/index.php b/packages/block-library/src/query/index.php index e21601b1711872..6ef132385c7731 100644 --- a/packages/block-library/src/query/index.php +++ b/packages/block-library/src/query/index.php @@ -30,43 +30,8 @@ function render_block_core_query( $attributes, $content, $block ) { $p->set_attribute( 'data-wp-interactive', '{"namespace":"core/query"}' ); $p->set_attribute( 'data-wp-router-region', 'query-' . $attributes['queryId'] ); $p->set_attribute( 'data-wp-init', 'callbacks.setQueryRef' ); - // Use context to send translated strings. - $p->set_attribute( - 'data-wp-context', - wp_json_encode( - array( - 'loadingText' => __( 'Loading page, please wait.' ), - 'loadedText' => __( 'Page Loaded.' ), - ), - JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP - ) - ); + $p->set_attribute( 'data-wp-context', '{}' ); $content = $p->get_updated_html(); - - // Mark the block as interactive. - $block->block_type->supports['interactivity'] = true; - - // Add a div to announce messages using `aria-live`. - $html_tag = 'div'; - if ( ! empty( $attributes['tagName'] ) ) { - $html_tag = esc_attr( $attributes['tagName'] ); - } - $last_tag_position = strripos( $content, '' ); - $content = substr_replace( - $content, - '
-
', - $last_tag_position, - 0 - ); } } From e56191df89bcc4c44d26e3aabec742c4b2944717 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 12:23:53 +0100 Subject: [PATCH 03/25] Implement wp-router-region processor --- .../class-wp-interactivity-api.php | 69 +++++++++++++++++-- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php index ad9e5d7c439533..012968a51bfb46 100644 --- a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php +++ b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php @@ -18,12 +18,13 @@ class WP_Interactivity_API { * @var array */ private static $directive_processors = array( - 'data-wp-interactive' => 'data_wp_interactive_processor', - 'data-wp-context' => 'data_wp_context_processor', - 'data-wp-bind' => 'data_wp_bind_processor', - 'data-wp-class' => 'data_wp_class_processor', - 'data-wp-style' => 'data_wp_style_processor', - 'data-wp-text' => 'data_wp_text_processor', + 'data-wp-interactive' => 'data_wp_interactive_processor', + 'data-wp-router-region' => 'data_wp_router_region_processor', + 'data-wp-context' => 'data_wp_context_processor', + 'data-wp-bind' => 'data_wp_bind_processor', + 'data-wp-class' => 'data_wp_class_processor', + 'data-wp-style' => 'data_wp_style_processor', + 'data-wp-text' => 'data_wp_text_processor', ); /** @@ -673,6 +674,62 @@ private function data_wp_text_processor( WP_Interactivity_API_Directives_Process } } } + + /** + * Processes the `data-wp-router-region` directive. + * + * It renders in the footer a set of HTML elements to notify users about + * client-side navigations. More concretely, the elements added are 1) a + * top loading bar to visually inform that a navigation is in progress + * and 2) an `aria-live` region for accessible navigation announcements. + * + * The same action is registered everytime the directive is processed to + * prevent element duplication. + * + * @since 6.5.0 + * + * @param WP_Interactivity_API_Directives_Processor $p The directives processor instance. + */ + private function data_wp_router_region_processor( WP_Interactivity_API_Directives_Processor $p ) { + if ( ! $p->is_tag_closer() ) { + /* + * The state could be declared multiple times here but is + * doesn't matter as the values are always the same. + */ + wp_interactivity_state( + 'core/router', + array( + 'navigation' => array( + 'message' => '', + 'hasStarted' => false, + 'hasFinished' => false, + 'texts' => array( + 'loading' => __( 'Loading page, please wait.' ), + 'loaded' => __( 'Page Loaded.' ), + ), + ), + ) + ); + + $callback = static function () { + echo << +
+HTML; + }; + add_action( 'wp_footer', $callback ); + } + } } } From c2ab4d2b6c64891d5551ef24cb50768ac21f7a40 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 12:24:15 +0100 Subject: [PATCH 04/25] Remove aria regions and loading bar logic from query block --- packages/block-library/src/query/view.js | 27 ------------------------ 1 file changed, 27 deletions(-) diff --git a/packages/block-library/src/query/view.js b/packages/block-library/src/query/view.js index 85cd16d9e14225..c9282f3af0e490 100644 --- a/packages/block-library/src/query/view.js +++ b/packages/block-library/src/query/view.js @@ -19,14 +19,6 @@ const isValidEvent = ( event ) => ! event.defaultPrevented; store( 'core/query', { - state: { - get startAnimation() { - return getContext().animation === 'start'; - }, - get finishAnimation() { - return getContext().animation === 'finish'; - }, - }, actions: { *navigate( event ) { const ctx = getContext(); @@ -37,30 +29,11 @@ store( 'core/query', { if ( isValidLink( ref ) && isValidEvent( event ) && ! isDisabled ) { event.preventDefault(); - // Don't announce the navigation immediately, wait 400 ms. - const timeout = setTimeout( () => { - ctx.message = ctx.loadingText; - ctx.animation = 'start'; - }, 400 ); - const { actions } = yield import( '@wordpress/interactivity-router' ); yield actions.navigate( ref.href ); - // Dismiss loading message if it hasn't been added yet. - clearTimeout( timeout ); - - // Announce that the page has been loaded. If the message is the - // same, we use a no-break space similar to the @wordpress/a11y - // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 - ctx.message = - ctx.loadedText + - ( ctx.message === ctx.loadedText ? '\u00A0' : '' ); - - ctx.animation = 'finish'; - ctx.url = ref.href; - // Focus the first anchor of the Query block. const firstAnchor = `.wp-block-post-template a[href]`; queryRef.querySelector( firstAnchor )?.focus(); From e0fecdc9e2e8da2e270f50a046ae9916801b42c0 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 12:48:34 +0100 Subject: [PATCH 05/25] Move loading bar CSS from query to router region processor --- .../class-wp-interactivity-api.php | 54 ++++++++++++++----- packages/block-library/src/query/block.json | 3 +- packages/block-library/src/query/style.scss | 52 ------------------ 3 files changed, 43 insertions(+), 66 deletions(-) delete mode 100644 packages/block-library/src/query/style.scss diff --git a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php index 012968a51bfb46..e6bdb32cb405e9 100644 --- a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php +++ b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php @@ -713,18 +713,48 @@ private function data_wp_router_region_processor( WP_Interactivity_API_Directive $callback = static function () { echo << -
+ +
+
HTML; }; add_action( 'wp_footer', $callback ); diff --git a/packages/block-library/src/query/block.json b/packages/block-library/src/query/block.json index 26d194dcd1f23b..8ea8d818433828 100644 --- a/packages/block-library/src/query/block.json +++ b/packages/block-library/src/query/block.json @@ -52,6 +52,5 @@ "html": false, "layout": true }, - "editorStyle": "wp-block-query-editor", - "style": "wp-block-query" + "editorStyle": "wp-block-query-editor" } diff --git a/packages/block-library/src/query/style.scss b/packages/block-library/src/query/style.scss deleted file mode 100644 index 4e9f4741beaed4..00000000000000 --- a/packages/block-library/src/query/style.scss +++ /dev/null @@ -1,52 +0,0 @@ -.wp-block-query__enhanced-pagination-animation { - position: fixed; - top: 0; - left: 0; - margin: 0; - padding: 0; - width: 100vw; - max-width: 100vw !important; - height: 4px; - background-color: var(--wp--preset--color--primary, #000); - opacity: 0; - - &.start-animation { - animation: - wp-block-query__enhanced-pagination-start-animation - 30s - cubic-bezier(0.03, 0.5, 0, 1) - forwards; - } - - &.finish-animation { - animation: - wp-block-query__enhanced-pagination-finish-animation - 300ms - ease-in; - } -} - -@keyframes wp-block-query__enhanced-pagination-start-animation { - 0% { - transform: scaleX(0); - transform-origin: 0% 0%; - opacity: 1; - } - 100% { - transform: scaleX(1); - transform-origin: 0% 0%; - opacity: 1; - } -} - -@keyframes wp-block-query__enhanced-pagination-finish-animation { - 0% { - opacity: 1; - } - 50% { - opacity: 1; - } - 100% { - opacity: 0; - } -} From 020e9c2527ca61b075026da59bc9828516313bd8 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 16:37:24 +0100 Subject: [PATCH 06/25] Recover `navigatingTo` variable --- packages/interactivity-router/src/index.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index d11aadfa3d879e..d22fb7ef2dc8f9 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -78,11 +78,13 @@ pages.set( Promise.resolve( regionsToVdom( document ) ) ); +// Variable to store the current navigation. +let navigatingTo = ''; + export const { state, actions } = store( 'core/router', { state: { url: '', navigation: { - target: '', hasStarted: false, hasFinished: false, texts: {}, @@ -115,7 +117,7 @@ export const { state, actions } = store( 'core/router', { *navigate( href, options = {} ) { const url = cleanUrl( href ); const { navigation } = state; - navigation.target = href; + navigatingTo = href; actions.prefetch( url, options ); // Create a promise that resolves when the specified timeout ends. @@ -126,7 +128,7 @@ export const { state, actions } = store( 'core/router', { // Don't update the navigation status immediately, wait 400 ms. const timeout = setTimeout( () => { - if ( navigation.target === href ) { + if ( navigatingTo === href ) { navigation.hasStarted = true; navigation.hasFinished = false; navigation.message = navigation.texts.loading; @@ -144,7 +146,7 @@ export const { state, actions } = store( 'core/router', { // Once the page is fetched, the destination URL could have changed // (e.g., by clicking another link in the meantime). If so, bail // out, and let the newer execution to update the HTML. - if ( navigation.target !== href ) return; + if ( navigatingTo !== href ) return; navigation.hasStarted = false; navigation.hasFinished = true; From f45282b9ab7c01e58e67705ab573d1087c482ebe Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 16:42:03 +0100 Subject: [PATCH 07/25] Fix flaky test --- test/e2e/specs/interactivity/tovdom-islands.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/specs/interactivity/tovdom-islands.spec.ts b/test/e2e/specs/interactivity/tovdom-islands.spec.ts index 257b0a0fc94b8c..849001274cfd53 100644 --- a/test/e2e/specs/interactivity/tovdom-islands.spec.ts +++ b/test/e2e/specs/interactivity/tovdom-islands.spec.ts @@ -44,7 +44,7 @@ test.describe( 'toVdom - islands', () => { } ) => { const el = page.getByTestId( 'island inside another island' ); const templates = el.locator( 'template' ); - expect( await templates.count() ).toEqual( 1 ); + await expect( templates ).toHaveCount( 1 ); } ); test( 'islands inside inner blocks of isolated islands should be hydrated', async ( { From 2f95ca7dbfe8352a5b14359d2e35694a91be1097 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 29 Jan 2024 17:13:15 +0100 Subject: [PATCH 08/25] Remove unnecessary PHPUnit checks --- phpunit/blocks/render-query-test.php | 53 +--------------------------- 1 file changed, 1 insertion(+), 52 deletions(-) diff --git a/phpunit/blocks/render-query-test.php b/phpunit/blocks/render-query-test.php index 76dbf537e78684..c75174c7417855 100644 --- a/phpunit/blocks/render-query-test.php +++ b/phpunit/blocks/render-query-test.php @@ -68,7 +68,7 @@ public function test_rendering_query_with_enhanced_pagination() { $p = new WP_HTML_Tag_Processor( $output ); $p->next_tag( array( 'class_name' => 'wp-block-query' ) ); - $this->assertSame( '{"loadingText":"Loading page, please wait.","loadedText":"Page Loaded."}', $p->get_attribute( 'data-wp-context' ) ); + $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' ) ); @@ -86,14 +86,6 @@ public function test_rendering_query_with_enhanced_pagination() { $this->assertSame( 'core/query::actions.navigate', $p->get_attribute( 'data-wp-on--click' ) ); $this->assertSame( 'core/query::actions.prefetch', $p->get_attribute( 'data-wp-on--mouseenter' ) ); $this->assertSame( 'core/query::callbacks.prefetch', $p->get_attribute( 'data-wp-watch' ) ); - - $p->next_tag( array( 'class_name' => 'screen-reader-text' ) ); - $this->assertSame( 'polite', $p->get_attribute( 'aria-live' ) ); - $this->assertSame( 'context.message', $p->get_attribute( 'data-wp-text' ) ); - - $p->next_tag( array( 'class_name' => 'wp-block-query__enhanced-pagination-animation' ) ); - $this->assertSame( 'state.startAnimation', $p->get_attribute( 'data-wp-class--start-animation' ) ); - $this->assertSame( 'state.finishAnimation', $p->get_attribute( 'data-wp-class--finish-animation' ) ); } /** @@ -131,49 +123,6 @@ public function test_rendering_query_with_enhanced_pagination_auto_disabled_when $this->assertSame( 'true', $p->get_attribute( 'data-wp-navigation-disabled' ) ); } - - /** - * Tests that the `core/query` last tag is rendered with the tagName attribute - * if is defined, having a div as default. - */ - public function test_enhanced_query_markup_rendering_at_bottom_on_custom_html_element_tags() { - global $wp_query, $wp_the_query; - - $content = << - - - -HTML; - - // Set main query to single post. - $wp_query = new WP_Query( - array( - 'posts_per_page' => 1, - ) - ); - - $wp_the_query = $wp_query; - - $output = do_blocks( $content ); - - $p = new WP_HTML_Tag_Processor( $output ); - - $p->next_tag( 'span' ); - - // Test that there is a div added just after the last tag inside the aside. - $this->assertSame( $p->next_tag(), true ); - // Test that that div is the accesibility one. - $this->assertSame( 'screen-reader-text', $p->get_attribute( 'class' ) ); - $this->assertSame( 'context.message', $p->get_attribute( 'data-wp-text' ) ); - $this->assertSame( 'polite', $p->get_attribute( 'aria-live' ) ); - } - /** * Tests that the `core/query` block adds an extra attribute to disable the * enhanced pagination in the browser when a post content block is found inside. From eb65d2b3144c09466d5647ed2955dda4ec88a968 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 1 Feb 2024 16:07:34 +0100 Subject: [PATCH 09/25] Ensure the callback is executed once --- .../interactivity-api/class-wp-interactivity-api.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php index e6bdb32cb405e9..1fa63ba254e767 100644 --- a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php +++ b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php @@ -691,7 +691,10 @@ private function data_wp_text_processor( WP_Interactivity_API_Directives_Process * @param WP_Interactivity_API_Directives_Processor $p The directives processor instance. */ private function data_wp_router_region_processor( WP_Interactivity_API_Directives_Processor $p ) { - if ( ! $p->is_tag_closer() ) { + static $has_added_markup = false; + + if ( ! $p->is_tag_closer() && ! $has_added_markup ) { + $has_added_markup = true; /* * The state could be declared multiple times here but is * doesn't matter as the values are always the same. From 6ab84a11f44a8341558e531620990bdb86545ccc Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 1 Feb 2024 16:21:11 +0100 Subject: [PATCH 10/25] Update boolean flags and message only if page exists --- packages/interactivity-router/src/index.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index d22fb7ef2dc8f9..77bcc1301b6eb3 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -148,18 +148,17 @@ export const { state, actions } = store( 'core/router', { // out, and let the newer execution to update the HTML. if ( navigatingTo !== href ) return; - navigation.hasStarted = false; - navigation.hasFinished = true; - // Announce that the page has been loaded. If the message is the - // same, we use a no-break space similar to the @wordpress/a11y - // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 - navigation.message = - navigation.texts.loaded + - ( navigation.message === navigation.texts.loaded - ? '\u00A0' - : '' ); - if ( page ) { + navigation.hasStarted = false; + navigation.hasFinished = true; + // Announce that the page has been loaded. If the message is the + // same, we use a no-break space similar to the @wordpress/a11y + // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 + navigation.message = + navigation.texts.loaded + + ( navigation.message === navigation.texts.loaded + ? '\u00A0' + : '' ); renderRegions( page ); window.history[ options.replace ? 'replaceState' : 'pushState' From 1c4445fdc4c0069571aa866c1a4797549fe7cb48 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 1 Feb 2024 16:21:56 +0100 Subject: [PATCH 11/25] Clarify usage of unresolved promise --- packages/interactivity-router/src/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 77bcc1301b6eb3..25ce02b606ddb6 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -166,6 +166,9 @@ export const { state, actions } = store( 'core/router', { state.url = href; } else { window.location.assign( href ); + // We await here a promise that won't resolve to prevent any + // potential feedback indicating that the navigation has + // finished while the new page is being loaded. yield new Promise( () => {} ); } }, From f207a7998034bf15c012202e9137111d75e17adc Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 1 Feb 2024 16:33:00 +0100 Subject: [PATCH 12/25] More code reordering --- packages/interactivity-router/src/index.js | 23 +++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 25ce02b606ddb6..c4202fe7aae775 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -83,7 +83,7 @@ let navigatingTo = ''; export const { state, actions } = store( 'core/router', { state: { - url: '', + url: window.location.href, navigation: { hasStarted: false, hasFinished: false, @@ -149,8 +149,18 @@ export const { state, actions } = store( 'core/router', { if ( navigatingTo !== href ) return; if ( page ) { + renderRegions( page ); + window.history[ + options.replace ? 'replaceState' : 'pushState' + ]( {}, '', href ); + + state.url = href; + + // Update the navigation status once the render of the new page + // has been completed. navigation.hasStarted = false; navigation.hasFinished = true; + // Announce that the page has been loaded. If the message is the // same, we use a no-break space similar to the @wordpress/a11y // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 @@ -159,16 +169,11 @@ export const { state, actions } = store( 'core/router', { ( navigation.message === navigation.texts.loaded ? '\u00A0' : '' ); - renderRegions( page ); - window.history[ - options.replace ? 'replaceState' : 'pushState' - ]( {}, '', href ); - state.url = href; } else { window.location.assign( href ); - // We await here a promise that won't resolve to prevent any - // potential feedback indicating that the navigation has - // finished while the new page is being loaded. + // Await a promise that won't resolve to prevent any potential + // feedback indicating that the navigation has finished while + // the new page is being loaded. yield new Promise( () => {} ); } }, From 56225f5c9f5c4a3658533d86279d257121e37635 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 1 Feb 2024 16:40:18 +0100 Subject: [PATCH 13/25] Save current link URL after navigating --- packages/block-library/src/query/view.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-library/src/query/view.js b/packages/block-library/src/query/view.js index c9282f3af0e490..dc82d7968dad45 100644 --- a/packages/block-library/src/query/view.js +++ b/packages/block-library/src/query/view.js @@ -33,6 +33,7 @@ store( 'core/query', { '@wordpress/interactivity-router' ); yield actions.navigate( ref.href ); + ctx.url = ref.href; // Focus the first anchor of the Query block. const firstAnchor = `.wp-block-post-template a[href]`; From 04ce0ddeecdbee9c0b8b9f3dba29fda296650ec3 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 11:00:37 +0100 Subject: [PATCH 14/25] Add topLoadingBar and screenReaderAnnounce options --- packages/interactivity-router/src/index.js | 69 ++++++++++++---------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index c4202fe7aae775..2eb8f6fd47ff47 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -98,39 +98,44 @@ export const { state, actions } = store( 'core/router', { * needed, and updates any interactive regions whose contents have * changed. It also creates a new entry in the browser session history. * - * @param {string} href The page href. - * @param {Object} [options] Options object. - * @param {boolean} [options.force] If true, it forces re-fetching the - * URL. - * @param {string} [options.html] HTML string to be used instead of - * fetching the requested URL. - * @param {boolean} [options.replace] If true, it replaces the current - * entry in the browser session - * history. - * @param {number} [options.timeout] Time until the navigation is - * aborted, in milliseconds. Default - * is 10000. + * @param {string} href The page href. + * @param {Object} [options] Options object. + * @param {boolean} [options.force] If true, it forces re-fetching the URL. + * @param {string} [options.html] HTML string to be used instead of fetching the requested URL. + * @param {boolean} [options.replace] If true, it replaces the current entry in the browser session history. + * @param {number} [options.timeout] Time until the navigation is aborted, in milliseconds. Default is 10000. + * @param {boolean} [options.topLoadingBar] Whether the top loading bar should be shown while navigating. Default to `true`. + * @param {boolean} [options.screenReaderAnnounce] Whether a message for screen readers should be announced while navigating. Default to `true`. * - * @return {Promise} Promise that resolves once the navigation is - * completed or aborted. + * @return {Promise} Promise that resolves once the navigation is completed or aborted. */ *navigate( href, options = {} ) { const url = cleanUrl( href ); const { navigation } = state; + const { + topLoadingBar = true, + screenReaderAnnounce = true, + timeout = 10000, + } = options; + navigatingTo = href; actions.prefetch( url, options ); // Create a promise that resolves when the specified timeout ends. // The timeout value is 10 seconds by default. const timeoutPromise = new Promise( ( resolve ) => - setTimeout( resolve, options.timeout ?? 10000 ) + setTimeout( resolve, timeout ) ); // Don't update the navigation status immediately, wait 400 ms. - const timeout = setTimeout( () => { - if ( navigatingTo === href ) { + const loadingTimeout = setTimeout( () => { + if ( navigatingTo !== href ) return; + + if ( topLoadingBar ) { navigation.hasStarted = true; navigation.hasFinished = false; + } + if ( screenReaderAnnounce ) { navigation.message = navigation.texts.loading; } }, 400 ); @@ -141,7 +146,7 @@ export const { state, actions } = store( 'core/router', { ] ); // Dismiss loading message if it hasn't been added yet. - clearTimeout( timeout ); + clearTimeout( loadingTimeout ); // Once the page is fetched, the destination URL could have changed // (e.g., by clicking another link in the meantime). If so, bail @@ -156,19 +161,23 @@ export const { state, actions } = store( 'core/router', { state.url = href; - // Update the navigation status once the render of the new page + // Update the navigation status once the the new page rendering // has been completed. - navigation.hasStarted = false; - navigation.hasFinished = true; - - // Announce that the page has been loaded. If the message is the - // same, we use a no-break space similar to the @wordpress/a11y - // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 - navigation.message = - navigation.texts.loaded + - ( navigation.message === navigation.texts.loaded - ? '\u00A0' - : '' ); + if ( topLoadingBar ) { + navigation.hasStarted = false; + navigation.hasFinished = true; + } + + if ( screenReaderAnnounce ) { + // Announce that the page has been loaded. If the message is the + // same, we use a no-break space similar to the @wordpress/a11y + // package: https://github.com/WordPress/gutenberg/blob/c395242b8e6ee20f8b06c199e4fc2920d7018af1/packages/a11y/src/filter-message.js#L20-L26 + navigation.message = + navigation.texts.loaded + + ( navigation.message === navigation.texts.loaded + ? '\u00A0' + : '' ); + } } else { window.location.assign( href ); // Await a promise that won't resolve to prevent any potential From 8217089efc4eb33015966c2aaf246b037dae2ae7 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 11:01:53 +0100 Subject: [PATCH 15/25] Fix url updating after navigating back or forward --- packages/interactivity-router/src/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 2eb8f6fd47ff47..6de959c1e9de19 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -67,6 +67,8 @@ window.addEventListener( 'popstate', async () => { const page = pages.has( url ) && ( await pages.get( url ) ); if ( page ) { renderRegions( page ); + // Update the URL in the state. + state.url = window.location; } else { window.location.reload(); } @@ -159,6 +161,7 @@ export const { state, actions } = store( 'core/router', { options.replace ? 'replaceState' : 'pushState' ]( {}, '', href ); + // Update the URL in the state. state.url = href; // Update the navigation status once the the new page rendering From 196634cfd50bfbdfd050b5cb168258ceaabe0792 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 11:04:29 +0100 Subject: [PATCH 16/25] Rename internal `url` variable to `pagePath` --- packages/interactivity-router/src/index.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 6de959c1e9de19..e565c5d4dbb8f0 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -14,7 +14,7 @@ const pages = new Map(); // Helper to remove domain and hash from the URL. We are only interesting in // caching the path and the query. -const cleanUrl = ( url ) => { +const getPagePath = ( url ) => { const u = new URL( url, window.location ); return u.pathname + u.search; }; @@ -63,8 +63,8 @@ const renderRegions = ( page ) => { // Listen to the back and forward buttons and restore the page if it's in the // cache. window.addEventListener( 'popstate', async () => { - const url = cleanUrl( window.location ); // Remove hash. - const page = pages.has( url ) && ( await pages.get( url ) ); + const pagePath = getPagePath( window.location ); // Remove hash. + const page = pages.has( pagePath ) && ( await pages.get( pagePath ) ); if ( page ) { renderRegions( page ); // Update the URL in the state. @@ -76,7 +76,7 @@ window.addEventListener( 'popstate', async () => { // Cache the current regions. pages.set( - cleanUrl( window.location ), + getPagePath( window.location ), Promise.resolve( regionsToVdom( document ) ) ); @@ -112,7 +112,7 @@ export const { state, actions } = store( 'core/router', { * @return {Promise} Promise that resolves once the navigation is completed or aborted. */ *navigate( href, options = {} ) { - const url = cleanUrl( href ); + const pagePath = getPagePath( href ); const { navigation } = state; const { topLoadingBar = true, @@ -121,7 +121,7 @@ export const { state, actions } = store( 'core/router', { } = options; navigatingTo = href; - actions.prefetch( url, options ); + actions.prefetch( pagePath, options ); // Create a promise that resolves when the specified timeout ends. // The timeout value is 10 seconds by default. @@ -143,7 +143,7 @@ export const { state, actions } = store( 'core/router', { }, 400 ); const page = yield Promise.race( [ - pages.get( url ), + pages.get( pagePath ), timeoutPromise, ] ); @@ -203,9 +203,9 @@ export const { state, actions } = store( 'core/router', { * fetching the requested URL. */ prefetch( url, options = {} ) { - url = cleanUrl( url ); - if ( options.force || ! pages.has( url ) ) { - pages.set( url, fetchPage( url, options ) ); + const pagePath = getPagePath( url ); + if ( options.force || ! pages.has( pagePath ) ) { + pages.set( pagePath, fetchPage( pagePath, options ) ); } }, }, From 4e7660f162e80f63eb462388645ae85145ca1645 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 11:07:07 +0100 Subject: [PATCH 17/25] Always set a string in `state.url` --- packages/interactivity-router/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index e565c5d4dbb8f0..0afc57a7c76984 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -68,7 +68,7 @@ window.addEventListener( 'popstate', async () => { if ( page ) { renderRegions( page ); // Update the URL in the state. - state.url = window.location; + state.url = window.location.href; } else { window.location.reload(); } From ab00ba8548986f0b3d975fbcb0c941cacdc0cce6 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 15:05:32 +0100 Subject: [PATCH 18/25] Remove confusing comment --- .../interactivity-api/class-wp-interactivity-api.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php index 1fa63ba254e767..33a733d0db21e0 100644 --- a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php +++ b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php @@ -683,9 +683,6 @@ private function data_wp_text_processor( WP_Interactivity_API_Directives_Process * top loading bar to visually inform that a navigation is in progress * and 2) an `aria-live` region for accessible navigation announcements. * - * The same action is registered everytime the directive is processed to - * prevent element duplication. - * * @since 6.5.0 * * @param WP_Interactivity_API_Directives_Processor $p The directives processor instance. From 196020395d3c4aba3d3534af1fc575cd32918327 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 15:06:02 +0100 Subject: [PATCH 19/25] Use internal state instance instead of `wp_interactivity_state` --- .../interactivity-api/class-wp-interactivity-api.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php index 33a733d0db21e0..6f14babe7f2027 100644 --- a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php +++ b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php @@ -692,11 +692,9 @@ private function data_wp_router_region_processor( WP_Interactivity_API_Directive if ( ! $p->is_tag_closer() && ! $has_added_markup ) { $has_added_markup = true; - /* - * The state could be declared multiple times here but is - * doesn't matter as the values are always the same. - */ - wp_interactivity_state( + + // Initialize the `core/router` store. + $this->state( 'core/router', array( 'navigation' => array( From a13b08c6e3c8e09b382b1b652ce078adc5d15db3 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 2 Feb 2024 15:06:35 +0100 Subject: [PATCH 20/25] Add id to the router animations style tag --- .../interactivity-api/class-wp-interactivity-api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php index 6f14babe7f2027..81e492ee937c00 100644 --- a/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php +++ b/lib/compat/wordpress-6.5/interactivity-api/class-wp-interactivity-api.php @@ -711,7 +711,7 @@ private function data_wp_router_region_processor( WP_Interactivity_API_Directive $callback = static function () { echo << +