-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Global Styles: Allow references to values in other locations in the tree #41696
Conversation
* @param array $properties Properties metadata. | ||
* @return array Returns the modified $declarations. | ||
*/ | ||
protected static function compute_style_properties( $styles, $settings = array(), $properties = null, $theme_json = null ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No change to this function except that we're passing in theme_json
.
} | ||
|
||
foreach ( $properties as $css_property => $value_path ) { | ||
$value = static::get_property_value( $styles, $value_path, $theme_json ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then passing $theme_json
through here...
Size Change: -64 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool idea @scruffian! I like the idea of the styles/settings lookup, and I can imagine it being quite powerful, and it seems a bit more direct than some cases of trying to implement this via CSS variables instead like in your earlier PR (#39432)
I've just left a couple of comments — while I think the feature itself is a great idea, I was wondering if it's worth breaking up the PR slightly to propose the new feature in one PR, and propose setting the default colors in another PR?
Cool stuff 👍🏻 I think it's awesome to lean in the power of our chosen format! I wonder if this will introduce a lot of complexity in themes as it is so inviting to do this. Will it lower the expectation for defined palettes for instance? E.g. instead of "primary-color" just use "styles.color.background" ... |
I think this is great to have. The defined palette is important just on its own because it's user-facing. And in your example, the background color is going to be defined as primary in any case, this approach just allows a different way of referencing colors and adds an extra tool for plugins to be styled in line with the rest of the theme. |
I've addressed the feedback here. If y'all are happy with this approach, I'll remove the changes to theme.json so that this is a lot simpler. I'll leave them there for now to make it easy to test :) |
cc @ramonjd |
$value_path = explode( '.', $value['source'] ); | ||
$source_value = _wp_array_get( $theme_json, $value_path ); | ||
// Only use the source value if we find anything. | ||
if ( ! empty( $source_value ) && is_string( $source_value ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If $source_value is empty, would this return just { source: "..." }
? Is this intended? Also, could an empty value be actually valid and desired?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking is that if nothing is found then we should be non-destructive. This could be useful in the case where multiple theme.json files are combined - we might not have a value on the first processing but we may do later...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this lead to a situation where the final value after all the processing ends up being { source: "..." }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, do you think that could be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, depends on how it gets rendered as CSS – I haven't tried running it yet 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the value is an array it get skipped inside compute_style_properties
(line 348).
🙌 I tried the following using Curator and it also worked, the background color of button elements was set to the same value as the button block. I also tested changing the button block background color via the GS panel (blocks > button > color > background), and the button elements still matched.
This feels pretty powerful. It creates possibilities for themes to make some strange connections / "links" in addition to very sensible ones. How would this work if I wanted to access a color from |
We already have a mechanism for doing that, using vars ( |
I like this idea. Thanks @scruffian It's testing pretty well for me so far. I was toying with some edge cases, and I wasn't sure if they were important or not. One was: should a ref to another ref be able to resolve (I can't see that it does)? E.g., see "margin" here: "spacing": {
"blockGap": "200px",
"padding": {
"source": "styles.spacing.blockGap"
},
"margin": {
"source": "styles.spacing.padding"
}
}, Just asking. I could imagine that would get pretty recursive. |
@ramonjd 's example is my main fear with this enhancement, it's a tool for complexity creation, which is solving the same problem that css variables solves but has the potential to bring over a different flavor of callback hell. |
Yeah at the moment it doesn't work. I feel like this is probably the best option to avoid any kind of recursive loops etc. If you want to use a property which references another property then you'll have to update the reference to the parent property. |
Adding a |
7b9f7ae
to
370e049
Compare
Done! |
I've made a few changes here:
Ready for another review... |
Thanks for the updates @scruffian!
It looks like the PR still refers to Also, since this introduces some potentially complex logic, would it be worth adding a couple of PHP tests with example |
Ooops sorry yes, try now! Good idea, I'll add some tests... |
* Elements: Add an api make it easier to get class names * Add some PHP unit tests * Add a unit tests for elements * lint fix * fix PHP tests * Update lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php Co-authored-by: George Mamadashvili <[email protected]> * Add @SInCE * update form.js * fix PHP test Co-authored-by: George Mamadashvili <[email protected]> Theme JSON: Add dynamic properties Theme JSON: Make it possible to use styles and settings from elsewhere in the tree also make button text dynamic fix type remove unconnected changes move the value replacement earlier and add validation use an object rather than a string to fetch other values adding a doing it wrong message remove color changes for buttons add an explanatory comment change source to ref Add unit tests Update lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php Co-authored-by: Adam Zielinski <[email protected]> lint fixes fix fixtures remove incorrect change lint fixes
4e17798
to
35e42ed
Compare
While getting familiar with how this works I've found a bug (this doesn't work in the site editor) #42890 |
Thanks @oandregal I noticed that too: #42884 It's on my list of things to look at this week. |
For posterity, YAML supports these kind of aliases out of the box. It also supports comments. |
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54118 602fd350-edb4-49c9-b593-d223f7449a82
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. Built from https://develop.svn.wordpress.org/trunk@54118 git-svn-id: http://core.svn.wordpress.org/trunk@53677 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. Built from https://develop.svn.wordpress.org/trunk@54118 git-svn-id: https://core.svn.wordpress.org/trunk@53677 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. Built from https://develop.svn.wordpress.org/trunk@54118
Dev Note draft: WhatTheme designs often require consistency in the styles applied to blocks. Often this can be achieved by setting Global Styles properties which are inherited by blocks using the inheritance of the CSS cascade. In some cases blocks want to apply settings from Global Styles to a different property of the block - for example the button element might want to use the global text color for its background color and the global background for it's text color. WhyTo solve this problem we need to allow themes to share Global Styles settings with blocks. This will make it easier for users to update these shared properties across all their blocks. Again take the example of a button element that wants to use the text color for its background. If the button element is set up as in the example above, then when a user edits the global text color, the background color of their buttons will also be updated. HowTo achieve this, we have added new
Global Styles will convert { ref: "styles.color.background" } the value at: LimitationsIt is currently only possible to use |
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54118 602fd350-edb4-49c9-b593-d223f7449a82
What?
There are many times when a block would like to access properties from the global styles context but is unable to do so. Sometimes that isn't a problem as the CSS cascade takes care of it for us, but in the case where blocks want to apply a setting to a different property, there's no mechanism to do so. This PR proposes that we add a new convention to theme.json which allows one property to set itself to the value of another, for example buttons could do this:
Global Styles will convert
{ ref: "styles.color.background" }
to whatever is the value atstyles > color > background
in theme.json.Why?
This will allow blocks to be a lot less opinionated in their styles and simply inherit from theme settings.
How?
The main part of this change is in
get_property_value
, which adds some lines of code to detect whether the value starts withstyles
orsettings
. If it does then we replace it with the result of _wp_array_get. The rest of the changes are necessary to pass $theme_json down to this function, since it's static. This was first attempted in #39432.Testing Instructions
Screenshots or screencast
Note
This change has a few side effects that we'll need to be careful with
cc @WordPress/block-themers