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

Add a way to identify which .JSON style variation that is in use #43405

Open
carolinan opened this issue Aug 19, 2022 · 22 comments · May be fixed by #51512
Open

Add a way to identify which .JSON style variation that is in use #43405

carolinan opened this issue Aug 19, 2022 · 22 comments · May be fixed by #51512
Assignees
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Theme Style Variations Related to style variations provided by block themes [Focus] Blocks Adoption For issues that directly impact the ability to adopt features of Gutenberg. Needs Dev Ready for, and needs developer efforts [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@carolinan
Copy link
Contributor

carolinan commented Aug 19, 2022

There should be a way to identify which theme.json style variation that is in use:

  • A unique identifier as a CSS class on the body, so that custom CSS can be applied to a variation. is-variation-name, has-variation-name or similar.
  • A PHP function that returns true or false for variations, (similar to for example is_page_template ) so that patterns, custom styles and filters can be used conditionally.
@carolinan carolinan added [Feature] Theme Style Variations Related to style variations provided by block themes [Type] Enhancement A suggestion for improvement. labels Aug 19, 2022
@colorful-tones
Copy link
Member

+1000 for this. Thanks for opening an Issue. 👏

@carolinan
Copy link
Contributor Author

carolinan commented Sep 2, 2022

If I understand correctly, the resulting styles from the variation.json is stored as a revision of the original wp_global_styles database entry, but the data does not include the variation file name, which we would need to determine which variation is in use.

@carolinan carolinan added the Needs Dev Ready for, and needs developer efforts label Nov 10, 2022
@annezazu annezazu added the [Focus] Blocks Adoption For issues that directly impact the ability to adopt features of Gutenberg. label Jun 5, 2023
@ndiego ndiego added the [Feature] Extensibility The ability to extend blocks or the editing experience label Jun 5, 2023
@pbking pbking linked a pull request Jun 14, 2023 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jun 14, 2023
@colorful-tones
Copy link
Member

I'm going to test out @pbking PR this week and drop any feedback. I wonder if this may be considered for WP 6.4 release?

@colorful-tones
Copy link
Member

I would love to hear input from @mikachan @annezazu @richtabor @carolinan on whether this might be considered for the WP 6.4 release.

It seems like a simple enhancement for extenders, but sometimes, the tiny things offer nuance and complexity 😄

@mikachan
Copy link
Member

The last Gutenberg release that's scheduled to be included with WP 6.4 is 16.7, and the RC for this is being released tomorrow, so I don't believe we have enough time to get this into WP 6.4 I'm afraid.

Please correct me if I'm wrong! This will be a great enhancement when it lands.

@colorful-tones
Copy link
Member

The last Gutenberg release that's scheduled to be included with WP 6.4 is 16.7, and the RC for this is being released tomorrow, so I don't believe we have enough time to get this into WP 6.4 I'm afraid.

That makes sense 👍

@mikachan
Copy link
Member

I've noticed that #51512 has just been approved, so when this is merged I'm happy for it to go into GB 16.7 for WP 6.4 🎉

I'm sorry that I didn't spot this before I ran the 16.7 RC process, but happy to cherry-pick this into the 16.7 release when it's merged.

@colorful-tones
Copy link
Member

I'm sorry that I didn't spot this before I ran the 16.7 RC process

@mikachan no worries! There are a lot of moving pieces and we never want to rush things. I appreciate the response and clarification. 🤘

@richtabor
Copy link
Member

Why is this necessary again? I’m not following.

@carolinan
Copy link
Contributor Author

carolinan commented Sep 20, 2023

  • For adding CSS when a style variation is active, and you have too much CSS than what works well in the theme.json CSS field
  • For filtering (anything) when a style variation is active, for example, block output, scripts, stylesheets.

@richtabor
Copy link
Member

richtabor commented Sep 20, 2023

You shouldn't need that much custom CSS (ideally none, but we're getting there), especially for a variation of a theme. And I don't think most folks need the variation title in the body class name.

I think it's fine to have an easy way to get the active variation title, but not append it to the body class. Then if the developer wants to do add it to the body, they can easily enough.

@carolinan
Copy link
Contributor Author

if the developer wants to do add it to the body, they can easily enough.

Not without the change to the variation setting in the PR.

@carolinan
Copy link
Contributor Author

In what way is the body class damaging?

It already outputs data that only some people will use:
class="home page-template-default page page-id-16 wp-custom-logo wp-embed-responsive cookies-not-set"
class="search search-results wp-custom-logo wp-embed-responsive cookies-not-set"
home page-template-default page page-id-39806 customizer-styles-applied

@colorful-tones
Copy link
Member

@richtabor I'm not clear on the hesitation for introducing this functionality. Even after reading this and this.

For adding CSS when a style variation is active, and you have too much CSS than what works well in the theme.json CSS field

I would add that JavaScript is a possibility here too and not just CSS. JavaScript that targets the additional body class that is being proposed.

It's quite a specific need for a theme (variation)

Yes.

and not useful for the vast majority—or really any active block themes—as far as I'm aware.

This is anecdotal. I've been asked several times by extenders about this functionality and this is why I wrote a post for a workaround.

@justintadlock
Copy link
Contributor

justintadlock commented Sep 20, 2023

FWIW, access to the variation name/slug is one of the most-requested features by theme authors I've talked to who ship style variations. It's one of the reasons I still push child themes, even in cases where they wouldn't be necessary.

They're generally less concerned about the <body> class (though this is useful). The data is what's important for various scenarios beyond just CSS.

I mostly agree that you shouldn't need much custom CSS for variations. Most CSS overrides in variations can happen via the settings.custom property.

@richtabor
Copy link
Member

I would add that JavaScript is a possibility here too and not just CSS. JavaScript that targets the additional body class that is being proposed.

That's very specific to a functionality not available in WordPress core, but extended upon. I'm all for making it easier for developers to add a variation class to the body tag — but not a wholesale approach that's not needed for most folks.

They're generally less concerned about the class (though this is useful). The data is what's important for various scenarios beyond just CSS.

FWIW, access to the variation name/slug is one of the most-requested features by theme authors I've talked to who ship style variations.

Yes, let's make it easy to get the variation slug/filename/title (whichever makes most sense; can't change/break things when edited is a big consideration). But not auto-apply it to the body tag. Make room for developers to do that easily.

@colorful-tones
Copy link
Member

There are two pieces of functionality at play here:

  1. surfacing the style variation name/slug
  2. assigning it as a body class

I do agree that the first is more important and would allow extenders something to hook onto. The second item is less crucial. I would be fine with just the slug being surfaced.

@colorful-tones
Copy link
Member

I'm all for making it easier for developers to add a variation class to the body tag — but not a wholesale approach that's not needed for most folks.

This additional explanation helps land some of the nuance here and I appreciate @richtabor offering additional clarity. ❤️

@landwire
Copy link

So is there a way yet to access the variation slug/filename/title? And if not is there any timeline for this?

@deryckoe
Copy link

So is there a way yet to access the variation slug/filename/title? And if not is there any timeline for this?

Something like this?

class Styles {
	public function __construct() {
		add_action( 'admin_init', [ $this, 'variation_styles' ] );
		add_filter( 'body_class', [ $this, 'body_class' ] );
	}

	public function variation_styles() {
		$variation = wp_get_global_settings( [ 'custom', 'variation' ] );

		if ( empty( $variation ) || is_array( $variation ) ) {
			return;
		}

		$file   = '/styles/css/' . $variation . '.css';
		$styles = file_get_contents( get_theme_file_path( $file ) );

		if ( $styles === false ) {
			return;
		}

		wp_add_inline_style( 'core-block-supports', $styles );
		add_editor_style( $file );
	}

	public function body_class( $classes ) {
		$variation_class = wp_get_global_settings( [ 'custom', 'variation' ] );

		if ( empty( $variation_class ) || is_array( $variation_class ) ) {
			return $classes;
		}

		$classes[] = 'is-variation-' . $variation_class;

		return $classes;
	}
}

new Styles();

@Imran92
Copy link
Contributor

Imran92 commented Oct 31, 2023

Hey folks 👋 We needed a quick workaround for it in our Course theme. This is how we ended up solving it for us for now Automattic/themes#7432. Just wanted to share our approach in case you wanted to take a look <3

@oandregal
Copy link
Member

Related #62686

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Theme Style Variations Related to style variations provided by block themes [Focus] Blocks Adoption For issues that directly impact the ability to adopt features of Gutenberg. Needs Dev Ready for, and needs developer efforts [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.