WordPress.org

Make WordPress Core

Opened 11 months ago

Last modified 2 months ago

#50120 assigned defect (bug)

[Multiple Bundled Themes] Custom color palette styles need to be added to the editor stylesheet

Reported by: kjellr Owned by: notlaura
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: needs-patch dev-feedback
Focuses: css Cc:

Description

Recent changes in Gutenberg 7.9 require themes to bundle their custom color styles in the editor. Previously, these styles were only needed on the front end:

https://github.com/WordPress/gutenberg/pull/21428#issuecomment-610874932

If themes do not include their custom colors in the editor styles, the color swatches will have no effect:

https://cldup.com/m5GNXkhznA.png


The following default themes need their custom color palette rules included in their editor styles by the time 7.9 gets into core (or sooner, as they won't hurt anything if they're in there beforehand):

  • Twenty Nineteen
  • Twenty Sixteen
  • Twenty Fifteen
  • Twenty Fourteen
  • Twenty Thirteen
  • Twenty Twelve
  • Twenty Eleven

Attachments (5)

50120.patch (16.1 KB) - added by sabernhardt 10 months ago.
Twenty Ten to Twenty Sixteen, same :root .editor-styles-wrapper .has-x-color pattern for each
50120.1.patch (12.8 KB) - added by sabernhardt 10 months ago.
Twenty Ten has .editor-styles-wrapper .has-x-color; Twenty Eleven to Twenty Sixteen only need .has-x-color
50120.2020-theme.patch (5.3 KB) - added by sabernhardt 9 months ago.
Twenty Twenty: setting customized colors in block editor
text-color-inline.jpg (39.0 KB) - added by sabernhardt 5 months ago.
Inline color in a paragraph block (highlight text and find "Text color" in the dropdown)
editor-css-specificty.png (60.4 KB) - added by notlaura 5 months ago.
The specificity of the default color causes the user set color to be overridden

Download all attachments as: .zip

Change History (42)

#1 @SergeyBiryukov
11 months ago

  • Milestone changed from Awaiting Review to 5.5

@sabernhardt
10 months ago

Twenty Ten to Twenty Sixteen, same :root .editor-styles-wrapper .has-x-color pattern for each

@sabernhardt
10 months ago

Twenty Ten has .editor-styles-wrapper .has-x-color; Twenty Eleven to Twenty Sixteen only need .has-x-color

#2 @sabernhardt
10 months ago

  • Focuses css added

50120.patch gives the same :root .editor-styles-wrapper .has-x-color pattern in each theme from Twenty Ten to Twenty Sixteen.

50120.1.patch uses selectors with lower specificity, depending on what is necessary, for Twenty Ten to Twenty Sixteen. This would not override the default editor stylesheet's colors, but currently only the black and white have the same class names (and the colors match anyway).

Additional work is needed for (at least) the two most recent themes:

  1. Twenty Twenty - The theme's default colors correctly show in the editor, but the user-customized colors do not. (Editor styles use twentytwenty_generate_css.)
  2. Twenty Nineteen - I noticed that choosing background colors can set the block's text color but no background color in the editor. Both theme default and user-customized colors require attention.

(Twenty Seventeen does not have its own color palette.)

Last edited 10 months ago by sabernhardt (previous) (diff)

This ticket was mentioned in Slack in #core by sabernhardt. View the logs.


10 months ago

#4 @whyisjake
10 months ago

  • Owner set to whyisjake
  • Status changed from new to accepted

#5 @whyisjake
9 months ago

Would love to get @ianbelanger to weigh in on this ticket too.

#6 @ianbelanger
9 months ago

  • Keywords has-patch commit added
  • Owner changed from whyisjake to ianbelanger
  • Status changed from accepted to assigned

I tested the patch in 2010 - 2016 and it works well. I think we are good to commit this one.

#7 @whyisjake
9 months ago

In 48459:

Bundled Themes: Add custom color pallettes for block editor.

Gutenberg 7.9 added support for themes to bundle their own custom styles in to the editor. These changes have been added to 2010 - 2016.

See #50120.

Props kjellr, sabernhardt, ianbelanger.

#8 @whyisjake
9 months ago

Thanks for the patch @sabernhardt. I left the ticket open to account for:

Additional work is needed for (at least) the two most recent themes:

  • Twenty Twenty - The theme's default colors correctly show in the editor, but the user-customized colors do not. (Editor styles use twentytwenty_generate_css.)
  • Twenty Nineteen - I noticed that choosing background colors can set the block's text color but no background color in the editor. Both theme default and user-customized colors require attention.

#9 @whyisjake
9 months ago

  • Keywords needs-patch dev-feedback added; has-patch commit removed

#10 @davidbaumwald
9 months ago

@ianbelanger @whyisjake Is this still in the cards for 5.5?

#11 @whyisjake
9 months ago

@sabernhardt, can you work on a patch for 2019/2020?

#12 @sabernhardt
9 months ago

I should be able to create a patch for Twenty Twenty. If no one else makes a patch for Twenty Nineteen by the time the other is done, I could give it a try.

#13 @ianbelanger
9 months ago

Sorry all, I am in the process of moving so I'm afraid I can't be much help right now, other than testing and committing patches.

@sabernhardt
9 months ago

Twenty Twenty: setting customized colors in block editor

#14 @sabernhardt
9 months ago

50120.2020-theme.patch adds block editor support for any user-customized colors in Twenty Twenty (Accent, Primary, Secondary, Background, Subtle Background).

Last edited 9 months ago by sabernhardt (previous) (diff)

#15 @sabernhardt
9 months ago

I'm having trouble creating a patch for Twenty Nineteen.

For standard theme colors, this is what worked for me in the style-editor.scss file. I put the following code in the Base Typography section, between the .has-white-background-color settings and the figcaption styles, and there may a better place for it.

.has-primary-color {
	color: $color__link;
}

.has-secondary-color {
	color: $color__border-link-hover;
}

.has-dark-gray-color {
	color: $color__text-main;
}

.has-light-gray-color {
	color: $color__text-light;
}

.has-white-color {
	color: #FFF;
}

I couldn't figure out how to set the user-customized colors for inline text, but Twenty Nineteen apparently does not yet support those on the front end either.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


9 months ago

#17 @SergeyBiryukov
9 months ago

  • Milestone changed from 5.5 to 5.6

Per the latest bug scrub:

youknowriad
So for this one, we included a fix in Gutenberg to avoid having themes include these stylesheets by default.

sabernhardt
2019 still needs a patch, and both 2019 and 2020 need testing/commit

youknowriad
The recommendation is that folks need to include these stylesheets but since in 5.4 it was not enforced, we kept it as is.

davidb
So, is this a close as resolved upstream?

youknowriad
I’d say it would be still good to do but it’s not a blocker for 5.5

williampatton
I'm with riad here, this is not a hard requirement in 5.5
It would be nice but can be added in future release.

This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.


8 months ago

#19 @kburgoine
8 months ago

  • Owner changed from ianbelanger to notlaura

This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.


6 months ago

#22 @notlaura
6 months ago

I'm getting started on this later than intended, but progress is underway! I am working on testing the Twenty Twenty patch, then investigating the update for Twenty Nineteen.

#23 @helen
5 months ago

@notlaura Have you had a chance to look at this yet? We're a bit past the ideal window to get this committed but I think it's still okay for the next couple days.

#24 @notlaura
5 months ago

@helen Somewhat - for TwentyTwenty, the user-selected styles (set in Customizer) seem to apply as expected in both the front-end and block editor with and without 50120.2020-theme.patch. I have not made progress on the TwentyNineteen piece.

My process for testing TwentyTwenty was:

  1. Add a block and set the text and background colors
  2. Update the colors selected in the Customizer
  3. See if the color updated in both the editor and front-end

@sabernhardt does that sound like the right testing approach? I know there was mention of some issue being fixed upstream - could that be the case here?

#25 @sabernhardt
5 months ago

@notlaura Your testing process sounds good, but please check one more thing.

When I checked this in Twenty Twenty (without a patch) during the past week, I apparently forgot to try the inline text color option in addition to the block's text and background colors. With the Gutenberg fix for WordPress 5.5, the editor supplies the block with inline CSS again, though the span within a (paragraph) block does not specify color in the style attribute.

Inline text color seems to work with Twenty Twenty's standard palette yet not a custom palette. Twenty Nineteen still does not honor the inline color at all.

@sabernhardt
5 months ago

Inline color in a paragraph block (highlight text and find "Text color" in the dropdown)

@notlaura
5 months ago

The specificity of the default color causes the user set color to be overridden

#26 @notlaura
5 months ago

Ah, indeed - the inline text color is updated to the user-selected one on the front-end, but not in the editor (I did not know the text color option existed there! Nice).

In TwentyTwenty this looks to be a specificity battle - in twentytwenty/assets/css/editor-style-block.css, the styles that apply the default color palette are overriding those added in the twentytwenty_generate_css method (see above).

If we can remove :root from the selector in this block of styles:

/* CUSTOM COLORS */


:root .has-accent-color {
	color: #cd2653;
}

:root .has-accent-background-color {
	background-color: #cd2653;
	color: #fff;
}

/* etc */

...then the user-selected colors were applied as expected when I tested. Does anyone know if this specificity added by :root required?

I haven't created a patch before, so I am figuring that out, and I can push up this change tomorrow if someone else hasn't gotten to it yet!

#27 @sabernhardt
5 months ago

The :root was added in [46714]
See also https://github.com/WordPress/twentytwenty/issues/964

I think I've seen the explanation for that somewhere else (probably for a different theme), but I'm not finding it today.

This ticket was mentioned in Slack in #core-themes by hellofromtonya. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-themes by hellofromtonya. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


5 months ago

#31 @helen
5 months ago

  • Milestone changed from 5.6 to 5.7

#32 @helen
5 months ago

Punting due to impending RC, this is not new to block editor in 5.6, since it's in bundled themes I think technically it can even go into a maintenance release but for now I've put it in 5.7.

This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.


5 months ago

#34 @notlaura
5 months ago

I haven't gotten my patch together yet for the final bit of the 2020 theme piece yet, but for 2019, there is ticket 49787 that outlines the issue affecting both the editor and front end. We discussed in #core-css triage that the 2019 bit can be handled as part of that ticket since it adds complexity here.

Last edited 5 months ago by notlaura (previous) (diff)

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


2 months ago

#36 @hellofromTonya
2 months ago

@notlaura Is this one on your radar to fix in 5.7? What's the next for it?

#37 @lukecarbis
2 months ago

  • Milestone changed from 5.7 to 5.8

We're too late in the release cycle for 5.7 to get this one through. Moving to 5.8.

Note: See TracTickets for help on using tickets.