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.

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

Styles: top level styles does not open on mobile for sites using themes without style variations #67532

Closed
annezazu opened this issue Dec 3, 2024 · 9 comments · Fixed by #67537
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

annezazu commented Dec 3, 2024

When a block theme doesn't have style variations, the newer experience for top level styles in the Site Editor does not open on mobile when using GB 19.7 and GB nightly.

Steps to replicate

  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. Notice it doesn't respond.
  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.

Videos

With GB 19.7 and OnyxPulse:

ScreenRecording_12-03-2024.07-59-05_1.MP4

With GB nightly and OnyxPulse:

ScreenRecording_12-03-2024.08-03-22_1.MP4

cc @tellthemachines @aaroncampbell @ramonjd @talldan as you all have been working on this!

@annezazu annezazu added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Dec 3, 2024
@annezazu
Copy link
Contributor Author

annezazu commented Dec 3, 2024

Props to @supernovia for originally finding and flagging this!

@annezazu
Copy link
Contributor Author

annezazu commented Dec 3, 2024

If we're able to get a PR in place, be sure to add Backport to Gutenberg RC to it so we can try to get it in as a fix in GB 19.8!

@Mayank-Tripathi32
Copy link
Contributor

I ran git bisect, and it seems that 2a18aae3768da80f94e437c16e8d56e5a9a45e03 is the bad commit. I’ll investigate further and aim to open a PR to address it soon.

@Mayank-Tripathi32
Copy link
Contributor

Mayank-Tripathi32 commented Dec 3, 2024

Hi, It seems that the following code is causing issues with the style editor not navigating properly:

export function SidebarNavigationItemGlobalStyles( props ) {
	const { name } = useLocation();
	const hasGlobalStyleVariations = useSelect(
		( select ) =>
			!! select(
				coreStore
			).__experimentalGetCurrentThemeGlobalStylesVariations()?.length,
		[]
	);
	if ( hasGlobalStyleVariations ) {
		return (
			<SidebarNavigationItem
				{ ...props }
				to="/styles"
				uid="global-styles-navigation-item"
				aria-current={ name === 'styles' }
			/>
		);
	}
	return <SidebarNavigationItem { ...props } />;
}

Is there a specific reason for checking hasGlobalStyleVariations? It doesn’t appear to be serving any purpose at the moment. Removing it might resolve the issue.

Additionally, the following onClick function was removed:

<SidebarNavigationItem
			{ ...props }
			onClick={ () => {
				// Switch to edit mode.
				history.push(
					{
						...params,
						canvas: 'edit',
					},
					undefined,
					{
						transition: 'canvas-mode-edit-transition',
					}
				);
				// Open global styles sidebar.
				openGeneralSidebar( 'edit-site/global-styles' );
			} }
		/>
	);

This removal seems to have broken the functionality of the "Styles" tab for themes where hasGlobalStyleVariations is false.

Patch:

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' } />;

Will be opening PR shortly with the changes. I kept hasGlobalStyleVariations to conditionally apply uid as it was done previously.

@ramonjd
Copy link
Member

ramonjd commented Dec 4, 2024

If we're able to get a PR in place, be sure to add Backport to Gutenberg RC to it so we can try to get it in as a fix in GB 19.8!

It looks like the PR that affected the nav here isn't in 19.8 either, so I think we're safe to wait for 19.9

@annezazu
Copy link
Contributor Author

annezazu commented Dec 4, 2024

Hmm I can replicate in 19.7 though and I’d prefer a fix is out for 19.8 rather than waiting two more weeks for 19.9. What am I missing? 😄

@ramonjd
Copy link
Member

ramonjd commented Dec 4, 2024

Apologies, @annezazu. I had a brain short circuit. I've compared 19.8 with trunk and I think a version of the latest clean up PR merged today is the right one to backport:

However #67199 has changed a lot since then so I think it will need a special 19.8 PR instead (not to be merged in trunk).

Most notably the route params. Trunk uses name but 19.8 still refers to params.path

Here's the PR:

cc @bph

@annezazu
Copy link
Contributor Author

annezazu commented Dec 4, 2024

Thank you for the fast and lovely team work here, everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
3 participants