Fix CSS Custom Properties for presets in the site editor #36851
Merged
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
[Feature] Site Editor
This was referenced Nov 25, 2021
oandregal
commented
Nov 25, 2021
| @@ -71,34 +71,22 @@ function_exists( 'gutenberg_is_edit_site_page' ) && | |||
| } | |||
|
|
|||
| $new_global_styles = array(); | |||
| $new_presets = array( | |||
| array( | |||
| 'css' => 'variables', | |||
There was a problem hiding this comment.
We no longer pass the CSS variables via the block editor settings and tell the editor not to wrap them. Instead, we enqueue them via the enqueue_block_editor_assets filter.
oandregal
commented
Nov 25, 2021
| @@ -23,23 +23,20 @@ import wrap from './transforms/wrap'; | |||
| * @return {Array} converted rules. | |||
| */ | |||
| const transformStyles = ( styles, wrapperClassName = '' ) => { | |||
| return map( | |||
| styles, | |||
| ( { css, baseURL, __experimentalNoWrapper = false } ) => { | |||
There was a problem hiding this comment.
We no longer need the __experimentalNoWrapper so this is an extra win.
|
Size Change: -28 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Nov 25, 2021
| @@ -304,3 +302,5 @@ function gutenberg_global_styles_include_support_for_wp_variables( $allow_css, $ | |||
| add_filter( 'force_filtered_html_on_import', 'gutenberg_global_styles_force_filtered_html_on_import_filter', 999 ); | |||
| add_filter( 'safecss_filter_attr_allow_css', 'gutenberg_global_styles_include_support_for_wp_variables', 10, 2 ); | |||
| // This filter needs to be executed last. | |||
|
|
|||
| add_filter( 'enqueue_block_editor_assets', 'gutenberg_load_css_custom_properties' ); | |||
There was a problem hiding this comment.
I think this was of loading styles is kind of deprecated to the iframing. cc @ellatrix
There was a problem hiding this comment.
For them to load in the iframe, the styles need to be enqueued here:
gutenberg/lib/client-assets.php
Lines 710 to 780 in e523441
There was a problem hiding this comment.
Hey @ellatrix I've pushed the changes to use the function you mention, but it looks like it enqueues the given styles within the iframe. Or perhaps I need to do more things?
In this PR I'm actually trying to do the opposite: enqueue the styles outside the iframe, so they're available for the global styles sidebar.
This is the setup:
- Styles for the content, within the iframe: CSS vars + block styles. This is working fine and it's done at
gutenberg_experimental_global_styles_settings(adds them to editor settings' styles key). - Styles for the sidebar: only the CSS vars. So far, we've been using the same method than the content's styles but with an additional flag (
__experimentalNoWrapper) so they weren't wrapped by.editor-styles-wrapperand so targeted the sidebar. This is what I'm struggling to see how to do in an iframed editor.
There was a problem hiding this comment.
Talked with ella in private and can confirm that using enqueue_block_editor_assets is fine. So, switching this back to the original direction.
There was a problem hiding this comment.
enqueue_block_editor_assets is for the editor UI only, not the iframe. If you need it to load in both, you'll need to use both.
|
For me this issue is larger than just "loading styles". Basically we render these "previews" (the small circles) for gradients/colors in a lot of places, and we always relied on the fact that the previous use the actual value and are not dependent of a global CSS variable. So while the current PR solves the issue for the case where the custom CSS variable originates from theme.json, it doesn't solve if if the value of the gradient relied on a variable that is loaded in an editor styles for instance. So this makes me wonder whether a potential fix could be to actually consider these small circles as "block previews". Meaning render a circle inside an iframe and use the same mechanism used for the block canvas. Also, it's not just about the circles, there's a lot of places where we want a "preview": the global styles preview on the global styles sidebar, the typography previews as well. |
|
A similar but maybe simpler workaround to the proposed solution on this PR could be to "flag" (add some flag to the style object) the style and enqueue it outside the iframe as well. This would also only solve the issue if the custom CSS variable comes from theme.json. |
|
Oh, interesting. I'm not super familiar with how other parts of the editor interface work (previews, etc), although I see this approach is working and seems natural (3rd parties can use it as well). But I'll let others comment on the best direction to fix this. |
oandregal
force-pushed
the
fix/css-vars-site-editor-sidebar
branch
from
dcafb08 to
e5d91a6
Compare
oandregal
force-pushed
the
fix/css-vars-site-editor-sidebar
branch
from
e5d91a6 to
50e1019
Compare
|
This is ready for another review and/or 👍 |
|
For the record, I see |
This reverts commit e5d91a66932576f0699ab0df2e2893594734b34a.
oandregal
force-pushed
the
fix/css-vars-site-editor-sidebar
branch
from
50e1019 to
b481928
Compare
|
@youknowriad @ellatrix is there anything left here that I should implement? It's working as expected for me:
|
youknowriad
approved these changes
Dec 7, 2021
|
It appears this PR introduced a regression #37292 |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
noisysocks
pushed a commit
that referenced
this pull request
Dec 13, 2021
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.
Fixes #30642
This PR enqueues the CSS Custom Properties coming from the
theme.jsonin a different way.Before
We enqueued them via the
styleskey in the block editor settings. By doing this the styles are wrapped with the.editor-styles-wrapperclass, for them to be scoped to the content canvas. This was problematic for the CSS Custom Properties, as they wouldn't be available in the sidebar, hence, we added__experimentalNoWrapperto the styles transformation, a scape hatch for certain styles not to be wrapped via.editor-styles-wrapper.After
We enqueue the vars via the
enqueue_block_editor_assetsthat is unaffected by the.editor-styles-wrappermechanism.How to test
See testing instructions in the issue.
In addition to testing that the gradients are visible, I've also tested that, in the site editor, modifications to the solid colors are reflected in the own component (sidebar) and in the content canvas. I've also applied this PR on top of #36820 to make sure gradients work as expected as well.