Experimental: Navigation block with the Interactivity API #50041
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: David Arenas <david.arenas@automattic.com>
This is still pretty basic, just to check that it works. Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
SantosGuillamot
changed the title
SantosGuillamot
added
[Type] Experimental
|
Size Change: 0 B Total Size: 1.39 MB ℹ️ View Unchanged
|
SantosGuillamot
changed the title
gziolo
reviewed
Apr 25, 2023
There was a problem hiding this comment.
I took it for a spin, and the version built on top of Interactivity API seems to work the same as the old one. That's impressive, given that the focus is managed manually 😄
I was a bit surprised that the existing implementation of the overlay doesn't handle clicking outside of the menu as closing the dialog. Another missing feature was the using arrow up/down to move the focus between items in the submenu. Anyway, taking all that into account, it seems like both implementations have the same set of features.
See the recorded screencast with the existing behavior in the block editor when using menus and dialogs:
Screen.Recording.2023-04-25.at.13.21.56.mov
I'm not saying we need to match that on the front end, but it is something I found surprising.
gziolo
reviewed
Apr 25, 2023
|
Flaky tests detected in 4367b4d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4935185137
|
|
I tested this PR, and everything works as advertised. I can enable/disable the experiment with the setting: 
I see only 3 scripts loaded for the Navigation block (view script, interactivity runtime, and vendors). There is one issue to resolve for the view script, as 
The menu icon looks like before (without the experiment enabled): 
The menu overlay behaves as before (without the experiment enabled): 
|
|
I discovered an issue with the case where you didn't opt for the modal overlay to show the menu: 
In that case, the code that modifies the HTML of the Navigation block won't ever find the button that opens the menu, and will finish processing HTML without any further updates. We could fix it with something close to: diff --git a/lib/experimental/interactivity-api/navigation-block-interactivity.php b/lib/experimental/interactivity-api/navigation-block-interactivity.php
index 0453c8ed1a..3d363db4df 100644
--- a/lib/experimental/interactivity-api/navigation-block-interactivity.php
+++ b/lib/experimental/interactivity-api/navigation-block-interactivity.php
@@ -58,6 +58,7 @@ function gutenberg_block_core_navigation_add_directives_to_markup( $block_conten
if ( $w->next_tag( 'nav' ) ) {
$w->set_attribute( 'data-wp-island', '' );
$w->set_attribute( 'data-wp-context', '{ "core": { "navigation": { "isMenuOpen": false, "overlay": true, "roleAttribute": "" } } }' );
+ $w->set_bookmark( 'nav-element' );
};
// Add directives to the open menu button.
@@ -70,6 +71,28 @@ function gutenberg_block_core_navigation_add_directives_to_markup( $block_conten
$w->set_attribute( 'data-wp-on.click', 'actions.core.navigation.openMenu' );
$w->set_attribute( 'data-wp-on.keydown', 'actions.core.navigation.handleMenuKeydown' );
$w->remove_attribute( 'data-micromodal-trigger' );
+ } else {
+ // If the open modal button not found, we handle submenus immediately.
+ $w->seek( 'nav-element' );
+
+ // Add directives to the menu container.
+ if ( $w->next_tag(
+ array(
+ 'tag_name' => 'UL',
+ 'class_name' => 'wp-block-navigation__container',
+ )
+ ) ) {
+ $w->set_attribute( 'data-wp-class.is-menu-open', 'context.core.navigation.isMenuOpen' );
+ $w->set_attribute( 'data-wp-bind.aria-hidden', '!context.core.navigation.isMenuOpen' );
+ $w->set_attribute( 'data-wp-effect', 'effects.core.navigation.initModal' );
+ $w->set_attribute( 'data-wp-on.keydown', 'actions.core.navigation.handleMenuKeydown' );
+ $w->set_attribute( 'data-wp-on.focusout', 'actions.core.navigation.handleMenuFocusout' );
+ $w->set_attribute( 'tabindex', '-1' );
+ };
+
+ gutenberg_block_core_navigation_add_directives_to_submenu( $w );
+
+ return $w->get_updated_html();
}
// Add directives to the menu container.
@@ -124,7 +147,7 @@ function gutenberg_block_core_navigation_add_directives_to_markup( $block_conten
// Submenus.
gutenberg_block_core_navigation_add_directives_to_submenu( $w );
- return (string) $w;
+ return $w->get_updated_html();
};
/**
@@ -209,7 +232,7 @@ add_filter(
gutenberg_url( 'build/block-library/interactive-blocks/navigation.min.js' ),
array( 'wp-interactivity-runtime' )
);
- $metadata['viewScript'] = "array('wp-block-navigation-view')";
+ $metadata['viewScript'] = array( 'wp-block-navigation-view' );
}
return $metadata;
},However, it doesn't work correctly, as the view script code most likely assumes there is always an overlay. The printed HTML with the diff applied: 
|
|
I think I resolved both issues reported earlier:
|
gziolo
approved these changes
May 10, 2023
|
Test failures from CI jobs for checking PHP unit tests fail now because of the regression introduced in |
Taking a look at them, @gziolo. |
cbravobernal
approved these changes
May 10, 2023
|
@gziolo, I've tested the changes you mentioned in #50041 (comment), and the issues seem to be solved now. 👍 |
…h-interactivity-api
gziolo
reviewed
May 10, 2023
| ) ) { | ||
| $w->set_attribute( 'data-wp-class.is-menu-open', 'context.core.navigation.isMenuOpen' ); | ||
| $w->set_attribute( 'data-wp-bind.aria-hidden', '!context.core.navigation.isMenuOpen' ); | ||
| $w->set_attribute( 'data-wp-effect', 'effects.core.navigation.initModal' ); |
There was a problem hiding this comment.
@SantosGuillamot, would it make sense to rename the effect to reflect the fact it works with the menu when there is no modal, too?
There was a problem hiding this comment.
I renamed it to initMenu in this commit.
|
Version 15.9.20230511 returns 404 for the following files: |
|
It's very likely that those files aren't whitelisted for inclusion in the zip file attached to the release: gutenberg/bin/build-plugin-zip.sh Lines 81 to 89 in 37d9839 |
|
I opened #50598 to fix the issue with missing scripts. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This pull request adds an experimental option to toggle between the Navigation block using the Interactivity API and the old scripts that use
micromodal:I recorded this video explaining in detail the purpose of the pull request and how it has been implemented:
https://www.loom.com/share/c07afacb4e464de8a8e7b253930dcd1b
Why?
This pull request is part of the proposal to use the Interactivity API as a standard for the blocks frontend. The main goal is to start testing real blocks with this API, which could also serve as an example, and see what they would look like.
In terms of performance, it is true that, if you only use the Interactivity API for the navigation block, it sends a few more kBs to the client than the current approach (12kBs vs 4kBs). However, if more blocks use the Interactivity API, they would reuse those 12kBs and would result in fewer kBs overall. There are already ongoing pull requests that aim to use this code like the Lightbox in images. And there could be more core blocks in the future using it.
Additionally, this would prevent the block from depending on external libraries like
micromodal.How?
micromodalis doing.There are several options in the navigation block. These are the use cases taken into account to replicate the current behavior:
I tried to cover all the use cases, but let me know if you are missing anything.
Testing
Basically, we have to ensure that the Navigation block, the navigation submenu, and the page list behave exactly the same when the option is activated/deactivated.
As part of this pull request, I added e2e tests for the navigation to check all those user experiences mentioned previously. With both options tests are passing, but maybe there are more use cases not contemplated there.