Do not render global styles coming from the server in the site editor #40185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oandregal
added
[Type] Bug
oandregal
changed the title
|
In addition to this:
|
7 tasks
|
Relevant PR #35736 the changes we do need to respect the fact that an user may want to remove all theme styles. |
youknowriad
reviewed
Apr 8, 2022
36 tasks
oandregal
force-pushed
the
fix/global-styles-in-site-editor
branch
from
ac2e0fd to
3a4afb1
Compare
oandregal
changed the title
oandregal
commented
Apr 12, 2022
oandregal
commented
Apr 12, 2022
oandregal
force-pushed
the
fix/global-styles-in-site-editor
branch
from
e460ed9 to
e555884
Compare
|
@youknowriad This now works with 5.8, 5.9, and 6.0 (beta1). I've also updated the test instructions in the issue description and backported the relevant changes to the wordpress-develop PR at WordPress/wordpress-develop#2542 (cc @gziolo that PR is enough to backport the changes in this one). |
youknowriad
approved these changes
Apr 13, 2022
|
@oandregal don't merge just yet, there's something I want to test. Give me some minutes |
|
Ok @oandregal so here's what I found: The theory is that the CSS loaded from the server should be strictly equivalent to the one generated using the client side global styles engine. (I wonder if we can have an e2e test or unit test for that to avoid regressions). With the current PR, this is even more important because now we don't enqueue the styles that come from the server. In my testing with the empty theme, I found the following differences: 1- Duotone variables are not being generated on the client Global styles engine. but the client outputs something a bit different 4- The server outputs this But the client doesn't. These differences mean for instance that floats might not work on the site editor at the root level (while they work on the frontend) ...etc. |
youknowriad
added
the
Backport to WP 6.7 Beta/RC
This is being fixed at #37727 For the rest of issues: it comes down to a mismatch between the rules provided in the server and in the client. I can fix this by updating the client assets here as well. |
|
Something I've been thinking about that is related to this and also to the style partials for sections at #39281 is that these alignment/layout styles should be considered side-effects of the theme.json. It's problematic that we have them where they are today: it prevent us from using the |
|
I'm noticed a weird |
|
Size Change: +125 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
oandregal
force-pushed
the
fix/global-styles-in-site-editor
branch
from
63fdce8 to
6002452
Compare
Everything good in this front. @youknowriad I made the client styles be the same as the server styles. |
|
Awesome thank you |
|
@gziolo @adamziel The PHP changes this PR introduces are backported at WordPress/wordpress-develop#2542 and can land independently from the JS changes. |
|
Do we need these changes in Gutenberg 13.0 or can it go in 13.1? |
|
The bug is a bit old I think, so we can leave it in 13.1 if needed but it's still a bug. So I guess I don't care which gutenberg version it goes in, the important is that it goes in the new WP version. |
gziolo
removed
the
Backport to WP 6.7 Beta/RC
|
This PR was backported to WordPress core with https://core.trac.wordpress.org/changeset/53208. |
37 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR makes the global styles coming from the server not to be rendered in the site editor.
Why?
The site editor, unlike other editors, generates the styles coming from global styles (presets, styles) dynamically in the client. This is because the user can modify any value that we need to consolidate (merge core, theme, and user style preferences). We don't want to have the global styles (coming from core or the plugin) twice, as it introduces subtle bugs.
How?
This PR modifies how things work:
isGlobalStyles: trueThe styles sent by the server to the client end up in the
block-editorstore, to be found atsettings.styles. By doing this, we leverage the existing mechanism in the site editor to recalculate the global styles upon user changes.These are the styles before this PR (in WordPress 5.9 and WordPress trunk):
And after this PR:
Note how the first three styles are no longer in the store.
Testing Instructions
Check the block editor store:
Do some random manual test: