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

Button default colours (and possibly others) do not work when using editor styles #22482

Closed
BinaryMoon opened this issue May 19, 2020 · 13 comments · Fixed by #22526
Closed

Button default colours (and possibly others) do not work when using editor styles #22482

BinaryMoon opened this issue May 19, 2020 · 13 comments · Fixed by #22526
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@BinaryMoon
Copy link

BinaryMoon commented May 19, 2020

If you use editor styles then the custom background and text colours do not work in the editor. They will (generally) display on the front end.

To reproduce
Steps to reproduce the behavior:

  1. Activate Twenty Twenty.
  2. Add a button.
  3. Change the background gradient.
  4. The button background gradient will not change.

Expected behavior
The background gradient should change.

Screenshots
Screenshot 2020-05-19 at 23 13 29

Additional context
This happens because inline styles are not used in the editor. Classes are used and they are applied to the root element. For example the gradient in the screenshot above uses :root .has-blush-bordeaux-gradient-background. When you add editor styles they get prefixed with .editor-styles-wrapper which has a higher priority than root and so the editor styles take precendence.

Note: custom colours added by a theme will work, as long as the colours are loaded into the editor.

The reason the default styles work is they are enqueued like a normal stylesheet and not loaded as editor styles, so don't have the .editor-styles-wrapper prefix.

I have added a list of suggested workarounds/ fixes below:
#22482 (comment)

@youknowriad
Copy link
Contributor

duplicate of #21931

@BinaryMoon
Copy link
Author

I had not seen that bug, but it sounds like you're not going to fix it. This is not a theme issue, it's a problem with Gutenberg.

If you read my report you will see this does not work in any theme because of CSS specificity issues. Those are not being addressed in the linked ticket.

@youknowriad
Copy link
Contributor

I had not seen that bug, but it sounds like you're not going to fix it. This is not a theme issue, it's a problem with Gutenberg

Where did you make that assumption? In fact, there's a fix here #22356

@BinaryMoon
Copy link
Author

BinaryMoon commented May 19, 2020

Where did you make that assumption?

I read your comments on the linked ticket about writing a blog post to explain how to fix this.

That ticket does sound like it will address the issue but it doesn't appear to understand what I reported.

The problem I am (badly) trying to report is not about custom colours not being loaded in the editor. It is also broken for themes that do not have custom colours set and use the default palette.

@youknowriad
Copy link
Contributor

The reason the default styles work is they are enqueued like a normal stylesheet and not loaded as editor styles, so don't have the .editor-styles-wrapper prefix.

As you mention, the styles are not loaded using the recommended editor styles mechanism which will definitely cause specificity issues.

@youknowriad
Copy link
Contributor

It is also broken for themes that do not have custom colours set and use the default palette.

this part needs testing though. I'm happy to open the issue again to investigate it.

@youknowriad youknowriad added the Needs Testing Needs further testing to be confirmed. label May 19, 2020
@youknowriad youknowriad reopened this May 19, 2020
@BinaryMoon
Copy link
Author

As you mention, the styles are not loaded using the recommended editor styles mechanism which will definitely cause specificity issues.

Yes. The core styles are loaded differently to theme styles. Is changing how they are loaded on the cards? That will help to show these problems up quicker.

@BinaryMoon
Copy link
Author

And thanks for reopening this. I've been debugging this for about 3 hours this evening trying to work out why the colours aren't applying to buttons. It didn't help that the test page I was using had buttons on it with inline styles so they were behaving differently.

@youknowriad
Copy link
Contributor

youknowriad commented May 19, 2020

it's a bit late here so I can't debug right now, I'm making a note to take a look tomorrow.

@BinaryMoon
Copy link
Author

Hi - now that I've slept on this I have three suggestions.

1. Load the colour classes on .editor-styles-wrapper instead of :root

This should fix the problem without inline styles

2. Load the default styles using add_editor_styles

This will surface bugs like this more quickly.

3. Load foreground, and background colours (and font sizes and gradients) automatically in the admin and on the front end.

Use PHP to stick it in the head. You have the details required in the add_theme_support function. This will:

  1. Simplify theme development.
  2. Create a single source of truth for the colour/ size data.
  3. Allow you to load the colour everywhere and not worry about the theme including it properly.

@BinaryMoon BinaryMoon changed the title Button custom colours (and possibly others) do not work when using editor styles Button default colours (and possibly others) do not work when using editor styles May 20, 2020
@youknowriad
Copy link
Contributor

Ok, I did some debugging and I agree that there's a problem for themes with editor styles but without a custom palette. This seems related to the "font-sizes" specifity changes done recently too right @nosolosw

And yeah it seems like using editor-styles-wrapper is the way to go similar to font sizes.

@youknowriad youknowriad added [Type] Bug An existing feature does not function as intended Good First Issue An issue that's suitable for someone looking to contribute for the first time and removed Needs Testing Needs further testing to be confirmed. labels May 20, 2020
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 21, 2020
@oandregal
Copy link
Member

oandregal commented May 21, 2020

@youknowriad yeah, the root cause is the same (bumped specificity for theme styles but not for core defaults) and should have been started happening since the removal of inline styles in the editor (related thread for reference).

PR that fixes the issue #22526

@oandregal
Copy link
Member

oandregal commented May 21, 2020

the gradient in the screenshot above uses :root .has-blush-bordeaux-gradient-background. When you add editor styles they get prefixed with .editor-styles-wrapper which has a higher priority than root and so the editor styles take precendence.

I want to clarify this bit. Note that classes and pseudo-classes have the same specificity. What was happening here was that both the theme selector and the core selector had the same specificity (020). Because of that, the last declaration found according to the source order (the theme's) won. The fix at #22526 increases the specificity of the core classes by adding the editor wrapper, so it becomes 030, hence, source order doesn't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants