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

Colors assigned as 'preset' colors do not render correctly in the editor: Are assigned both STYLE values and CLASSes. #26506

Open
pbking opened this issue Oct 27, 2020 · 3 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Comments

@pbking
Copy link
Contributor

pbking commented Oct 27, 2020

When a 'preset' color is selected as the background color for an element BOTH of these things happen in the editor:

  • the background-color is set as a style attribute to the color value selected (i.e. style="background-color:rgb(0,0,0)")
  • the appropriate class is applied to the element (i.e. class="has-PRESET-background-color")

While the user's preferences are set to "light mode" this renders fine; the color values are the same.

The 'dark mode' of a theme changes the preset values to support that dark mode and the value are no longer the same as defined by the theme for the editor's consumption. Thus, while the user's preferences are set to "dark mode" this does NOT render fine as there is a mismatch between what the expected "preset" color is as represented in the editor and what will actually render as defined by the theme.

What is rendered in the VIEW is correct, as only those elements that have a custom color that is NOT set to a 'predefined' color have the style attribute added.

To reproduce
Steps to reproduce the behavior:

  1. Set your system appearance to 'Light' mode.
  2. Activate Spearhead (a theme that has dark mode support).
  3. Add a

    block to a page and add some text.

  4. Change the background color to the preset 'Foreground' which is represented as a BLACK color option.
  5. Note that the Background Color of the

    block is correctly rendered as BLACK (and foreground text white).

  6. Change your system appearance to 'Dark' mode.
  7. Note that the Background Color of the

    block is still rendered as BLACK (however the foreground text is now also black).

  8. Publish the page and observe it in VIEW mode.
  9. Note that the

    tag is correctly rendered with a "white-ish" background (the FOREGROUND color of the theme while in dark mode).

Expected behavior
An element that has a BACKGROUND COLOR or TEXT COLOR applied that is NOT a 'custom color' but instead a color defined as a 'preset' color should NOT have the color values applied via a style attribute, but only by way of the 'preset' color classes. (This is the same color attribute assignment behavior found in the rendered view.)

Screenshots
<p> block in editor view with 'Foreground' preset color applied as the background (light mode):
image
The same block in view mode (light mode):
image

<p> block in editor view with 'Foreground' preset color applied as the background (dark mode):
image
The same block in view mode (dark mode):
image

Editor version (please complete the following information):

  • WordPress version: 5.5.1
  • Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? "gutenberg"
  • If the Gutenberg plugin is installed, which version is it? 9.2.1

Desktop (please complete the following information):

  • OS: Mac
  • Browser: FireFox

Additional context

@scruffian
Copy link
Contributor

One idea @kjellr had for how we could deal with this is to add a theme_support option that turns off the inline colors in the editor and uses class names instead. That way the colors would be based on whatever the stylesheet dictates.

@annezazu annezazu added [a11y] Color Contrast [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended labels Nov 4, 2020
@pbking
Copy link
Contributor Author

pbking commented Jan 19, 2021

I was able to track down that this change introduced adding the style attributes to the block. That was done to fix this issue.

It was also noted in the original bug that this type of behavior might occur and is a good reason NOT to have done that original fix.

This fix was originally done because of how TwentyNineteen defined its palette. I haven't yet confirmed if that has been addressed.

@kjellr suggested adding a theme_support option to disable this bit of code. An alternative might be to instead test the palette to determine if it was created in a state where this logic won't be necessary instead. (My expectation is that the code will rarely be ran in that case.) Or, perhaps the code can be simply removed if 2019 has been brought up-to-speed.

@pbking pbking changed the title Background Colors assigned as 'preset' colors do not render correctly in the editor while in dark mode. Colors assigned as 'preset' colors do not render correctly in the editor: Are assigned both STYLE values and CLASSes. Jan 19, 2021
@stefanfisk
Copy link
Contributor

This workaround solved the issue for me:

import { removeFilter } from '@wordpress/hooks';

removeFilter('editor.BlockListBlock', 'core/color/with-color-palette-styles');

@priethor priethor added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed [a11y] Color Contrast labels Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants