Fix: Fixed styling tab not opening on themes without style variations on mobile & desktop

[Mayank-Tripathi32]

resolves #67532

What?

This PR adds to="/styles" and aria-current={ name === 'styles' } to SidebarNavigationItem to ensure the Styles tab works correctly on mobile devices.

Why?

When a block theme lacks global style variations, the top-level Styles panel in the Site Editor does not open on mobile when using Gutenberg 19.7 or the nightly build. This PR resolves the issue to ensure consistent behavior across all themes.

How?

The conditional rendering for SidebarNavigationItem was updated. The onClick logic was removed with the introduction of the Styles panel in 2a18aae3768da80f94e437c16e8d56e5a9a45e03 , but it made it work only for themes with global variations. This patch ensures the SidebarNavigationItem opens the Styles panel for all themes, regardless of variations.

if ( hasGlobalStyleVariations ) {
   	return (
   		<SidebarNavigationItem
   			{ ...props }
   			to="/styles"
   			uid="global-styles-navigation-item"
   			aria-current={ name === 'styles' }
   		/>
   	);
   }
   return <SidebarNavigationItem { ...props } to="/styles" aria-current={ name === 'styles' } />;

Testing Instructions

1. Use a block theme without style variations, like https://wordpress.org/themes/onyxpulse/
2. Ensure you're using at least GB 19.7.
3. Open this on your phone or use browser tools to pull up on mobile.
4. Open Appearance > Editor
5. Try to select Styles and notice it properly opens up the Styles panel.
6. Switch to a theme with style variations, like https://wordpress.org/themes/twentytwentyfive/
7. Open Appearance > Editor
8. Try to select Styles and notice it properly opens up the Styles panel.

Screenshots or screencast

Screen.Recording.2024-12-03.at.11.54.32.PM.mov

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mayank-Tripathi32 <[email protected]>
Co-authored-by: annezazu <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Mayank-Tripathi32]

@annezazu @supernovia Please have a look at PR. Thank you

[annezazu]

Thanks so much for working on this! It works great for me in my testing. I can't speak to the code though.

[annezazu]

Video for good measure:

works.mov

[ramonjd]

Thanks for getting this PR up! I'll test further later today.

I'm wondering if we need the hasGlobalStyleVariations condition at all. 🤔

E.g.,

export function SidebarNavigationItemGlobalStyles( props ) {
	const { name } = useLocation();
	return (
		<SidebarNavigationItem
			{ ...props }
			to="/styles"
			uid="global-styles-navigation-item"
			aria-current={ name === 'styles' }
		/>
	);
}

I'll investigate later.

Also, not related to this PR, but there's a persistent JS error for sidebar items on mobile: #67467 (comment)

Just mentioning in case you see it and are wondering 👍🏻

[Mayank-Tripathi32]

I thought about removing the condition too. But after looking at the original code, I saw that the condition was there and the uid was missing in the default return. To keep the code consistent and working as expected, I decided to leave it as is.

[ramonjd]

Ah, thanks for the context. That's a good point. Anyway, I'll test as soon as I can today and we'll aim to get the fix in ASAP.

[ramonjd]

I still think a lot of this code is superfluous and doubles up on things.

For example, most of the props could be passed to SidebarNavigationItemGlobalStyles in MainSidebarNavigationContent.

			<SidebarNavigationItemGlobalStyles
				to="/styles"
				uid="global-styles-navigation-item"
				icon={ styles }
			>
				{ __( 'Styles' ) }
			</SidebarNavigationItemGlobalStyles>

With the same effect.

Removing the condition doesn't have any side effects as far as I can see, but I agree that we can take the cautious route to fix the bug.

I can get a follow up to clean up the code and we can get folks who have commented on recent changes to routing here to chime in.

cc @youknowriad and @t-hamano for visibility

[ramonjd]

Fixes the issue for me, thanks a lot @Mayank-Tripathi32

I'll add a clean up PR as a follow up as I suspect that the props can be tidied up and we don't need the hasGlobalStyleVariations condition.

[ramonjd]

I still think a lot of this code is superfluous and doubles up on things.

For example, most of the props could be passed to SidebarNavigationItemGlobalStyles in MainSidebarNavigationContent.

			<SidebarNavigationItemGlobalStyles
				to="/styles"
				uid="global-styles-navigation-item"
				icon={ styles }
			>
				{ __( 'Styles' ) }
			</SidebarNavigationItemGlobalStyles>

With the same effect.

Removing the condition doesn't have any side effects as far as I can see, but I agree that we can take the cautious route to fix the bug.

I can get a follow up to clean up the code and we can get folks who have commented on recent changes to routing here to chime in.

cc @youknowriad and @t-hamano for visibility

[ramonjd]

Now I remember 😄 The style panel used to only contain variations.

See: #50573

Since #65619 it's no longer the case.

I'll throw up a clean up PR

[ramonjd]

I'll throw up a clean up PR

#67545