Site global styles #50102
Site global styles #50102
Conversation
|
Size Change: +1.21 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
oandregal
added
[Status] In Progress
|
I've started reviewing this and pushed a few changes, including format, tests, and behavior to make the site origin work like any other origin. I'm looking at the resolver first, and I'll move to the REST API and client-side after. I hope you don't mind that I've pushed directly here. I thought it'll be faster given our timezone differences. |
That's great! go ahead, please. |
|
I did a first pass and the main class ( |
| $is_global_styles_user_theme_json = isset( $raw_config['isGlobalStylesUserThemeJSON'] ) && true === $raw_config['isGlobalStylesUserThemeJSON']; | ||
| $config = array(); | ||
| if ( $is_global_styles_user_theme_json ) { | ||
| $origin = ( isset( $post->post_name ) && 'wp-global-styles-site' === $post->post_name ) ? 'site' : 'custom'; |
There was a problem hiding this comment.
Documenting what I learn: the only change for the prepare_item_for_response method is this line. Otherwise, it's a verbatim copy of the existing 6.2 method.
There was a problem hiding this comment.
Yes, that's correct. That line and the following one (which will be using $origin value) are the only changes.
lib/compat/wordpress-6.3/class-gutenberg-rest-global-styles-controller-6-3.php
Show resolved
Hide resolved
|
Done with the server changes for the REST endpoint. In the client, we used to have only two things: user changes + base changes. The user changes were the only ones that could be updated via the site editor, and the base ones wouldn't ( I'm going to start looking now at client-side changes. |
| () => | ||
| async ( { dispatch } ) => { | ||
| const siteGlobalStyles = await apiFetch( { | ||
| path: '/wp/v2/global-styles/', |
There was a problem hiding this comment.
So, we need a way to pass the ID of the Custom Post Type that holds the data for the site origin (user changes unbound to a theme) to the client.
I see that the ID of the CPT for the custom origin (user changes bound to a theme) is taken from the themes endpoint through the wp:user-global-styles property. It was introduced at #35801. I see a mention about how it could be extended in the future.
I wonder what our options are for this:
- Attach it to the themes' endpoint as
wp:site-global-styles, mimicking what we do withwp:user-global-styles. The data for thesiteis unrelated to the theme, so it feels wrong to do it this way. - Attach it as a link elsewhere: where?
- Have a new endpoint to expose that piece of data. The current implementation of this PR. This seems too much considering the
customorigin uses a link in an existing endpoint.
It seems 2 is more measured for what we're trying to do here, though we'd need to understand where. @youknowriad I'd like to hear your thoughts on this.
There was a problem hiding this comment.
Attach it as a link elsewhere: where?
Perhaps a link in the settings endpoint that we already expose as the site entity?
|
I'm failing to see why we need two origins "site" and "custom". What's the difference between these two? In my understanding the issue was about making "custom" cross theme, is this wrong? |
The rationale is that some user data needs to still be bound to the theme ( |
I'm afraid this will introduce too much complexities. How would you differentiate between both in the UI, how do you revert to custom site one, to theme ones... |
|
Ok, I've discussed a bit with @mtias to understand the requirements here. It seems: 1- We don't want to make global styles theme agnostic. So I guess my question now, is why are we adding so much complexity to global styles (adding a level of customization) to address this? This font library issue doesn't seem relate to global styles for me? |
I see how the user mental model is going to be a bit shaken by this change, though my understanding is that we want For example, in fonts at #45271 the ones you upload you don't want them "cleared" when you switch themes. However, for changes to styles (e.g.: background color), you want them "cleared" as they were based on the theme definition. |
There are a few use cases that support this. I tried to describe them at #47420
Does this help? |
|
I think there's a confusion that is being made between "font presets" and "font library". One is theme.json material (presets), the other is not. I think it helps to think about the font library as "media library". We can use media in blocks and theme.json (maybe not sure) but these are not theme.json material and not saved there or anything. In other words, I don't think we should make any update to the theme.json/global styles behavior to support fonts. Instead we have these three layers:
|
I've addressed this above (font library and font presets)
I don't see how this requires a "site" global style variation. Alternative user style variations are something we can support without that. (Just list them in the UI in addition to the theme variations)
That's a potential use-case but maybe too small to warrant such a big complexity (both code wise and UI wise), so we need to be certain that we want that and it's unclear at the moment to me.
This is vague, I prefer concrete use-cases like the one just above. |
How do we prevent this list from clearing upon theme switches? I'll let @matiasbenedetto share more about that use case, though what I heard was "we don't want users to have to re-introduce the curated list when they switch themes" (which I understand as "theme-agnostic presets").
It's true that we can do differently. We can develop a custom solution for any. The point I tried to make at #47420 was that having a |
I don't think we want to retain the presets upon theme switch, we want to retain our font library. It's also the same question for color presets and... and this is going to be partially addressed by the "custom global styles variations".
I don't see a need for any custom code here, we already have the CPT to store global styles variations, it's just that we want to use it for use ones as well. |
|
I'll wait until @matiasbenedetto is around and can provide feedback. His original PR was at #46332 I see it saved to the |
|
The font manager feature consists of adding fonts to the WordPress instance by downloading them from remote providers such as Google Fonts or uploading assets from the user's device. In a prototype (#46332), we implemented this by storing the font assets in a WordPress folder and adding the font families and faces to the That approach is not completely satisfying because the font definitions added by the user are lost when one switches themes. For that reason, we thought about having a way to persist this I think I initially discarded implementing the font definitions persistence outside of the global styles domain to avoid custom implementation just for the font manager. Now @youknowriad has mentioned it I think we could explore a solution like that if adding a new origin for global styles is something that adds too much complexity and it is not potentially useful for other settings/styles apart from the font families definition. |
|
If maintaining data upon theme switch is a requisite, I still think the @youknowriad do you have thoughts on the way forward? |
Think of it like "media library". Do we use "global styles"
This feels like the right approach to me. In other words, can we make "fonts" just another type of media? |
|
Closing this old experimental PR |
Implements #47420 to add support for #45271.
What ?
This PR tries to add a site-level settings/styles config to persist data across theme switches.
Why ?
We need site-level data persistence to store configs such as user-added typographic fonts.
How ?
wp_global_styles.wp-global-styles-site.default > blocks > theme > site > custom. Note thatcustomoverrides thesite. This allows for users to have preferences stored that can be overridden if a specific theme is in use.siteto the allowed origins inWP_Theme_JSON_Gutenberg.get_merged_datato work with the newsiteorigin.Questions: