Skip to content
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: add custom CSS panel to site editor #46141

Merged
merged 39 commits into from
Dec 13, 2022

Conversation

glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Nov 29, 2022

What?

Adds a custom CSS input option to global styles in the site editor.

Why?

Because this option was available to users in customizer, but there is no option to add arbitrary CSS via the site editor.
Part of: #30142

How?

  • Adds a css property to theme.json styles - styles.css
  • Saves the global custom CSS to the wp_global_styles CPT
  • In the site editor the content of styles.css is added to a separate style sheet. Initially, it was just appended to the end of the existing global styles style sheet, but the issue with this approach was that any invalid CSS entered caused all global styles to stop working as the styles are parsed with js. Having a separate style sheet means any invalid CSS entered only breaks the custom CSS and all other global styles work as expected. It seems like just appending it to the end of the global styles style sheet works for the post editor and frontend though.

Testing Instructions

  • In the site editor open the global styles panel
  • Go to Custom CSS option at the bottom of the panel
  • Enter your favorite CSS and save
  • Check the CSS is applied in site editor, post editor and front end
  • Add a styles.css property to your theme.json file and make sure that it displays as expected. Then add some user custom CSS, and make sure the theme.json setting displays in the input box and make sure that this can be removed or overriden in site and page editor and front end
  • Add a styles.css property to the core theme.json, and make sure this is correctly overridden by the themes theme.json setting and also the user custom CSS

Known issue

Currently, any changes to the custom CSS that are not valid CSS will cause all the custom CSS to stop applying until the CSS is made valid again. Because of the way that CSS is parsed and rewritten for the site editor this is going to be difficult to fix so will be handled in a follow up.

Screenshots or screencast

customcss3.mp4

@github-actions

This comment was marked as outdated.

@Mamaduka

This comment was marked as outdated.

@glendaviesnz

This comment was marked as resolved.

@glendaviesnz

This comment was marked as outdated.

@glendaviesnz

This comment was marked as outdated.

@fabiankaegy

This comment was marked as outdated.

@glendaviesnz

This comment was marked as outdated.

@aristath

This comment was marked as outdated.

@glendaviesnz

This comment was marked as outdated.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 2, 2022

The custom CSS also needs to be added to the get_block_editor_settings, probably as one of the global styles.

@oandregal, would it make sense to add customCSS as one of the type variations for the wp_get_global_stylesheet?

@glendaviesnz
Copy link
Contributor Author

The custom CSS also needs to be added to the get_block_editor_settings, probably as one of the global styles.
@oandregal, would it make sense to add customCSS as one of the type variations for the wp_get_global_stylesheet?

Have done this by adding the extra type variation as suggested and all seems to work, but would still be good to get your view on whether this is a good approach @oandregal

@glendaviesnz glendaviesnz marked this pull request as ready for review December 4, 2022 23:53
@glendaviesnz

This comment was marked as outdated.

@annezazu annezazu mentioned this pull request Dec 5, 2022
57 tasks
@Mamaduka

This comment was marked as outdated.

@glendaviesnz
Copy link
Contributor Author

@Mamaduka I have copied across the basic CSS validation from customizer

@glendaviesnz

This comment was marked as resolved.

Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks really good so far, I'm impressed by what this PR achieves. Only very minor comments, though I haven't looked too deeply into the global styles and REST API code, which isn't my strongest area.

In using it there were a few things I noticed:

  • The textarea could be taller when initially opened
  • The textarea could use a monospaced font
  • When there are invalid styles, the entire editor styles seem to no longer be applied to the editor. It'd be nice only the custom styles are not applied.

lib/compat/wordpress-6.2/block-editor-settings.php Outdated Show resolved Hide resolved
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @glendaviesnz, it’s awesome to see this greatly anticipated addition in Global Styles coming together 🎉

Overall this tests nicely:

✅ Custom CSS is applied in the editors and frontend
✅ PHP unit tests pass
❔ Clearing custom CSS isn't immediately reflected in the editor as noted

@talldan beat me to suggesting a few UI tweaks. I think his list covers most of what I noted while testing. The only addition I had might be more a personal preference.

On the main Global Styles screen the help text for both “Blocks” and “Custom” start in an identical way which makes things feel a bit harder to scan at a glance. That probably isn’t helped by the two nearly having the same shape as well. So I was wondering if this is something we might tweak in a follow-up to potentially help users with different visual needs or dyslexia?

it is tricky to find a workaround

Indeed.

I'm not sure I have the full history on the Custom CSS work so likely have a bunch of false assumptions.

In #46255 we added the css property to the theme.json schema. Does this mean we are officially supporting that property in actual theme.json files? Or, was it only added to cover the custom CSS added by users via the Global Styles UI?

If it is the latter reasoning, would it be possible to make the css property only conditionally allowed depending on the "origin" for the theme.json? I don't think that is something currently represented in the schema so perhaps not.

The reason I'm querying if the styles.css property is only to be supported with the user origin is that we might be able to add a slightly different hack to force the user's clearing of the custom CSS to take effect.

Example tweak to `global-styles-provider.js`
diff --git a/packages/edit-site/src/components/global-styles/global-styles-provider.js b/packages/edit-site/src/components/global-styles/global-styles-provider.js
index 0ae154c11b..2e923772cc 100644
--- a/packages/edit-site/src/components/global-styles/global-styles-provider.js
+++ b/packages/edit-site/src/components/global-styles/global-styles-provider.js
@@ -44,6 +44,10 @@ const cleanEmptyObject = ( object ) => {
 	return isEmpty( cleanedNestedObjects ) ? undefined : cleanedNestedObjects;
 };
 
+const cleanObjectWithCustomCSS = ( object ) => {
+	return { ...cleanEmptyObject( object ), css: object?.css };
+};
+
 function useGlobalStylesUserConfig() {
 	const { globalStylesId, isReady, settings, styles } = useSelect(
 		( select ) => {
@@ -106,7 +110,7 @@ function useGlobalStylesUserConfig() {
 			};
 			const updatedConfig = callback( currentConfig );
 			editEntityRecord( 'root', 'globalStyles', globalStylesId, {
-				styles: cleanEmptyObject( updatedConfig.styles ) || {},
+				styles: cleanObjectWithCustomCSS( updatedConfig.styles ),
 				settings: cleanEmptyObject( updatedConfig.settings ) || {},
 			} );
 		},

The above change at least achieves the desired behaviour as I understand it. Maybe it can help spark new ideas and a better solution.

Demo video of tweaked behaviour
Screen.Recording.2022-12-07.at.4.53.09.pm.mp4

@glendaviesnz

This comment was marked as outdated.

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Dec 13, 2022

Details for Dev Note

A custom CSS input box has been added to the Global Styles settings in the site editor. This provides similar functionality to the custom CSS input available in Customizer for classic themes. This option appears under the global styles actions menu:

custom-css-location

Ideally, this input should be seen as a last resort fallback to add styling that can't yet be implemented with the other editor design tools. Some other important things to note are:

  • This custom CSS does not use the same custom_css CPT as the Customizer custom CSS, but instead is part of the existing Global Styles wp_global_styles CPT
  • As with the other user custom global styles the custom CSS is specific to the current theme
  • In order to support custom CSS as part of Global Styles a new property has been added to theme.json - styles.css
  • While the addition of the styles.css property to theme.json does allow theme authors to use it to include custom CSS strings in their themes, as with the user input for this property ideally themes should only use this as a last resort fallback to add styling that can't yet be implemented with the other editor design tools
  • Theme authors should note that users’ custom CSS can completely override or remove any theme custom CSS set in the theme.json, so in cases where theme authors do not want users to easily wipe custom CSS they should consider including it via the existing style sheet enqueuing methods
  • Because the standard global styles flow is for user data to override theme data, it would be possible for users to inadvertently override a key theme custom CSS setting, eg. add a custom image background to a group and in the process wipe a background that the theme was adding to headings. For this reason, when a user first edits the custom CSS the theme CSS is shown in the input box to allow the user to selectively and knowingly override/remove any theme custom CSS

oandregal added a commit that referenced this pull request Dec 15, 2022
@bph bph added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Jan 6, 2023
glendaviesnz added a commit to glendaviesnz/wordpress-develop that referenced this pull request Jan 23, 2023
aristath pushed a commit to aristath/wordpress-develop that referenced this pull request Jan 27, 2023
@Slals
Copy link

Slals commented Jan 29, 2023

Hi,

I'm building a website using Gutenberg. It's a very nice feature that I want to use. I'm building on top for "Twenty Twenty-Three" theme.

How can I activate this feature?

@glendaviesnz
Copy link
Contributor Author

How can I activate this feature?

@Slals currently you need to have the Gutenberg plugin installed and then enable the Custom CSS experiment under the Gutenberg Experiments menu in WP admin:

custom-css

aristath pushed a commit to aristath/wordpress-develop that referenced this pull request Feb 1, 2023
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Feb 1, 2023
aristath pushed a commit to aristath/wordpress-develop that referenced this pull request Feb 2, 2023
aristath pushed a commit to aristath/wordpress-develop that referenced this pull request Feb 3, 2023
@bph bph mentioned this pull request Feb 6, 2023
47 tasks
@bph bph added has dev note when dev note is done (for upcoming WordPress release) and removed Needs Dev Note Requires a developer note for a major WordPress release cycle labels Feb 28, 2023
@bph
Copy link
Contributor

bph commented Feb 28, 2023

@glendaviesnz could you do me a favor and review the above Dev Note?
It's been a few weeks since you wrote it and there might have been changes to the feature after that. Would you have time to combine your Dev Note with @carolinan's into a "Custom CSS for Global Styles and per block in theme.json" stand-alone post on the Make Core Blog by Thursday? I know it's short-term notice, so please notify me if that doesn't work and I will handle it.

@glendaviesnz
Copy link
Contributor Author

@bph was still accurate, but I added a note about the location of the feature under the global styles action menu.

@porg
Copy link

porg commented Mar 3, 2023

Thanks for this feature!

Usability Feedback:

function CustomCSSControl() {
const [ customCSS, setCustomCSS ] = useStyle( 'css' );
const [ themeCSS ] = useStyle( 'css', null, 'base' );
const ignoreThemeCustomCSS = '/* IgnoreThemeCustomCSS */';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there! I was trying to refactor this control to be like all the other "style" UI components and stumbled upon this specific behavior for custom CSS, where it seems that we need to be aware of "base" styles and have some custom behavior tied to it. Can you explain it a bit further and also why it has to be this way, why don't we treat the "css" theme.json property like any other property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial testing showed that it was confusing that any theme.json custom CSS was overridden completely by any custom CSS added as initially it took the same approach as other global styles in that user styles override the theme.json settings. For example, the theme.json CSS might set a heading font color, and the user may add a paragraph font family which will wipe the heading font color.

So a decision was made to include the theme.json CSS in the global styles input box to allow the user to make a conscious choice about which ones to keep.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial testing showed that it was confusing that any theme.json custom CSS was overridden completely by any custom CSS added as initially it took the same approach as other global styles in that user styles override the theme.json settings. For example, the theme.json CSS might set a heading font color, and the user may add a paragraph font family which will wipe the heading font color.

For me this is only confusing, if the CSS is not shown initially in the box.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a decision was made to include the theme.json CSS in the global styles input box to allow the user to make a conscious choice about which ones to keep.

Re-reading this, it seems like you're saying that we're already showing the theme.json CSS in the box, which is what I would expect as well, but I guess the different maybe when we edit that CSS.

For me, after editing, we should only show the edited CSS (which may or may not contain some parts of the original theme.json CSS).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, after editing, we should only show the edited CSS (which may or may not contain some parts of the original theme.json CSS)

Sorry for the confusion. This is correct, this is what happens, after editing only the edited CSS shows in the input, and the original CSS from the theme.json shows above the input in case the user wants to reinstate it:

custom-css.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json has dev note when dev note is done (for upcoming WordPress release) Needs User Documentation Needs new user documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet