Global Styles: update metadata and add support for padding #27099
Conversation
oandregal
requested review from
ellatrix,
spacedmonkey and
TimothyBJacobs
as code owners
oandregal
added
[Type] Code Quality
|
Size Change: +305 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
oandregal
requested review from
ajitbohra,
chrisvanpatten and
mkaz
as code owners
oandregal
changed the title
oandregal
force-pushed
the
update/global-styles-metadata
branch
from
588f8fd to
d2a3cf7
Compare
There was a problem hiding this comment.
Hi @nosolosw the changes work well on my tests 👍 I really like that the contexts for all blocks that previously we passed from the server are now computed on the client.
I'm not totally sure about passing STYLE_PROPERTIES from the server to the client.
packages/edit-site/src/components/editor/global-styles-provider.js
Outdated
Show resolved
Hide resolved
| @@ -14,7 +14,6 @@ import { | |||
| useMemo, | |||
| } from '@wordpress/element'; | |||
| import { useEntityProp } from '@wordpress/core-data'; | |||
| import { __EXPERIMENTAL_STYLE_PROPERTY as STYLE_PROPERTY } from '@wordpress/blocks'; | |||
There was a problem hiding this comment.
If global styles does not use __EXPERIMENTAL_STYLE_PROPERTY from @wordpress/blocks I guess the need to have this data in '@wordpress/blocks' is not present anymore as we now don't reuse this data between block-editor and global styles?
We still have the need to have this metadata on the client and server. What's the advantage of passing it on the server to edit-site especially but continue to use the hardcoded client information on the block-editor?
There was a problem hiding this comment.
I think we should remove that and use this setting! Didn't do it in this PR because it felt too much.
| contexts={ | ||
| settings.__experimentalGlobalStylesContexts | ||
| metadata={ | ||
| settings.__experimentalGlobalStylesMetadata |
There was a problem hiding this comment.
Previously we discussed passing this type of metadata from the server to the client to promote reusability. @youknowriad referred he did not like the idea of having metadata (like presets structure or style properties) as a setting. I'm cc @youknowriad in case he has any thoughts on this change, specifically the passing of STYLE_PROPERTY as a setting from the server.
There was a problem hiding this comment.
In the interest of agility, I've duplicated in the client the style property metadata. The conversation can continue, and if we want to pass down the metadata to the client we can address it in a follow-up PR.
There was a problem hiding this comment.
I think what's important for me is that the settings of the BlockEditorProvider make sense in isolation, in contexts without global styles, without WordPress and someone passing config there to build his own editor should be able to understand and pass the right config depending on what he wants.
So just make sure, that assumption stands :P
oandregal
force-pushed
the
update/global-styles-metadata
branch
from
d2a3cf7 to
b43a2c2
Compare
|
@jorgefilipecosta your suggestions are addressed! |
packages/edit-site/src/components/editor/global-styles-provider.js
Outdated
Show resolved
Hide resolved
oandregal
force-pushed
the
update/global-styles-metadata
branch
from
e4ee01a to
88700df
Compare
|
Follow-up that adds the padding control at #27154 |
This PR does a couple of things:
Reduces the amount of metadata we send to the client. Before this PR we sent all the metadata for every context, now the client derives the contexts from existing data. Fixes Reduce the amount of data Global Styles sends to the client #26802
While we allowed padding to be set at the block level (block attributes) we didn't do the same for the global level (theme.json). This is a bug that happened due to the distributed nature of the metadata that we had. Note that this needs a follow-up PR to add the padding control to the global styles sidebar.
How to test
Test that it works as before:
Tests that it padding works properly: