Desktop Navigation flashes briefly on load before Mobile Navigation JS kicks in

[tomxygen]

Description

If the Gutenberg plugin is installed, when loading any page of a website on mobile, the mobile menu is briefly shown ad it is on desktop, and then collapsed in the hamburger menu.
See the attached screen recording.

Step-by-step reproduction instructions

1. add a navigation menu
2. open any page on mobile, the menu should look fine in its hamburger menu
3. install the Gutenberg plugin
4. reopen any page on mobile.

Screenshots, screen recording, code snippet

Screen.Recording.2024-02-06.at.12.39.07.PM.mov

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[tomxygen]

@jordesign I saw the "needs testing" flag.
is there any way I can help with that?

[tomxygen]

the issue is now present in WordPress 6.5 beta 1, even without the Gutenberg plugin installed.
cc. @jordesign @fabiankaegy

[fabiankaegy]

I am able to replicate this in Beta 1 also. Thaks for the ping!

You can see it happen if you enable network throteling. My assumption is that it happes because the JS only gets loaded initialized after the page loads here.

CleanShot.2024-02-16.at.08.49.50.mp4

Pinging some of the Interactivity API folks: @luisherranz @juanmaguitar @gziolo @DAreRodz @c4rl0sbr4v0 @michalczaplinski

[fabiankaegy]

As far as I can tell the issue here is that the logic that determines whether the mobile menu should be shown now happens entirely in JS. It is invoked by the data-wp-init directive which calls this callback here:

gutenberg/packages/block-library/src/navigation/view.js

Lines 204 to 223 in 01708f1

	initNav() {
	const context = getContext();
	const mediaQuery = window.matchMedia(
	`(max-width: ${ NAVIGATION_MOBILE_COLLAPSE })`
	);
	// Run once to set the initial state.
	context.isCollapsed = mediaQuery.matches;
	function handleCollapse( event ) {
	context.isCollapsed = event.matches;
	}
	// Run on resize to update the state.
	mediaQuery.addEventListener( 'change', handleCollapse );
	// Remove the listener when the component is unmounted.
	return () => {
	mediaQuery.removeEventListener( 'change', handleCollapse );
	};

I don't think we can pre set the isCollapsed context on the server since we don't know the actual viewport there.

But I wonder if there isn't a way thtough a script in the head of the document to set the propper context not after the compoent renders but before it renders.

[DAreRodz]

Thanks for the ping, @fabiankaegy. 😊

I'm also able to reproduce the issue. Regarding what you mentioned about setting the context in advance, it wouldn't work either because the context is consumed during the interactive blocks hydration. You would need to wait for the browser to download and execute the Interactivity API runtime to see the Navigation block collapse.

However, I guess all this logic could be implemented using only CSS... 🤔

Pinging @scruffian as he was involved in the implementation.

[cbravobernal]

I can confirm that this error is not happening in 6.4.
I'm taking a look at it right now.

[cbravobernal]

It seems that this issue is caused by this PR.
#57520

Should we revert it or try to set the context before it renders?

[fabiankaegy]

@c4rl0sbr4v0 awesome find :) If we can revert that without too much hasstle I think we should. Unless anyone has an alternative fix for the issue that we can get tested and reployed in the next few days so that we can get it into 6.5 during the beta cycle :)

[DAreRodz]

A potential fix would be to add the .is-collapsed class to the <nav> element with an inline script. Such script would have to compute the mediaQuery, locate the nav.wp-block-navigation element, and add the .is-collapsed class to it, so the visitor doesn't have to wait for the Interactivity API runtime to be downloaded and executed.

In this case, though, I feel returning to a CSS-only implementation would be more appropriate than using directives to add/remove the .is-collapsed class. 🤔

[cbravobernal]

I feel returning to a CSS-only implementation would be more appropriate than using directives to add/remove the .is-collapsed class. 🤔

I've been investigating and it seems that people want the breakpoint to be an editable field. So for that case the CSS should be added in the render_callback rather than a file.

[huubl]

How about implementing something similar as the x-cloak directive in AlpineJS? This directive hides elements until the framework is fully loaded.

[cbirdsong]

Scripts can still fail to run for a lot of reasons even if they're inline. It would be best to handle as much of this as possible in CSS.