WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.49k stars 4.19k forks source link

Pullquote colors conflict when only one swatch is registered #12024

Closed La-Geek closed 1 year ago

La-Geek commented 5 years ago

No idea, how to descirbe these bugs, since I have no idea, how it should work (when it works), but I am sure, as they are atm., this can't be okay.

Windows 10, chrome browser (all recent), theme twentynighteen, WP (5.0-beta5-43907)

I made an animated Gif, please see yourself:

pullquote_confusion

earnjam commented 5 years ago

Ok, so this baffled me when I first looked at it too. After playing with it for a while I finally figure out what everything does.

A few observations:

earnjam commented 5 years ago

Screenshot of the radio button look-a-like

image

La-Geek commented 5 years ago

There's more confusing than that. Once you click on the left circle under Text Color it has an effect on both (quote and citation), once it has an effect only on citation. Similar when you click on the left circle under Main Color in combination with the Clear Button, it has different inconsistent effects. Imho the best solution would be a third row with circles for "citation" text and all should work in a simlar way to the circles in block paragraph= First row = background color Second row = (quote) text color Third row = (citation) text color

The first circle on the left are the default color(s) supplied by the theme. There could be more than one there

The first circle of the first row (Main Color) shows blue and has effects on the background color (box) and on the (quote) text color. Test= Change the background color with the rainbow circle (this will change the background color to your chosen color and the text to blue), then click once on the first circle (it will change the background color to blue and the quote text to white), then click again, it will change the (quote) text color to blue

quote01

In the second row (Text Color) the first circle shows blue, but the default Text Color is not blue. .wp-block-pullquote has the color #40464d

But now, it becomes really confusing. Change the Text Color with the rainbow circle, both text colors are changed. Now click on the first circle under Main Color, the (quote) text changes to white, and at least click on the clear button, and the (quote) text changes back to the chosen text color.

quote02

La-Geek commented 5 years ago

And here is, how Genesis Framework handles this (imho the better name for Main Color would be Background Color), there is no difference in text color between quote and citation text:

genesis_pullquote

jasmussen commented 5 years ago

Thanks for the ticket. I would like to re-label to something like "Pullquote colors conflict when only one swatch is registered", or something like that.

Let me show a few GIFs and then elaborate on what's going on. Vanilla stylesheet:

vanilla

TwentyNineteen:

2019

So, there are a number of issues here. Notably with the "Solid Color".

  1. Note how in the Regular style, if you set the main color, you only set the main color. But in the Solid style, when you set the main color, you also set the text color. This is a problem because a theme, as in the case of TwentyNineteen, might not have registered more than a single color. Which means it's a buggy experience. We should fix this so you only ever set a single color.
  2. TwentyNineteen, as mentioned above, only sets a single color swatch. Not only that, but it's the same color as the solid pullquote style has by default. Suggested fix: expand the TwentyNineteen palette to have more than one color.
  3. TwentyNineteen does not make use of the main color in the "Solid" style at all. It should. Main color should always be the background, or the decoratives, and the text color should always be the text above.

3 is worth expanding on, because there's a matrix of bugginess here. This refers only to the "solid" style:

A: When no color options are set, you get a blue quote with white text:

screenshot 2018-11-19 at 13 23 24

B: When you set a blue background main color, not only is the text color option also set to blue, but weirdly only the citation text color is changed:

screenshot 2018-11-19 at 13 24 01

C: If you then clear the text color, so there's a blue main color, you get basically the same as the default pullquote:

screenshot 2018-11-19 at 13 24 26

D: If you set only the text color, you get this:

screenshot 2018-11-19 at 13 25 20

C and D technically appear to be behaving correctly, as the main color is the same as the background, and the text color appears to apply to the whole text. But when you compare D with B, you realize that if you set the Main color to blue, then the pullquote text is white.

Suggested next steps:

  1. Fix so setting the Main color of the pullquote doesn't also set the text-color. While the intention is probably to ensure contrast, this stresscase proves that doesn't work. I would also argue that this is lossy behavior, as it makes a change to an option you didn't touch yourself. CC: @tofumatt @jorgefilipecosta I think you two worked on the pullquote, do you have insights here?

  2. Make the TwentyNineteen pullquote customization slightly less opinionated. By default it's fine that the pullquote is white text on a blue background, but that's where the customization should stop, and both the main color and text color should behave predictably like it does in the vanilla style. Additionally the color palette should be expanded. I will take a stab at this but CC: @kjellr for insights.

tofumatt commented 5 years ago

I hope I followed everything here but I'd say the main issue is that the limited numbers of swatches make it tough to ensure contrast. The code put in place to ensure text stays visible should work with slightly less opinionated pull quote colours, I think.

Sorry if I'm missing something or you wanted more specific feedback, there's a lot to follow in this issue but I think the main thing is that the theme should tweak its colour options rather than us remove the code we have to ensure contrast. 🤷‍♂️

kjellr commented 5 years ago

Just circling back to say that Twenty Nineteen now includes the two recommendations @jasmussen made above:

... both the main color and text color should behave predictably like it does in the vanilla style.

As of https://github.com/WordPress/twentynineteen/pull/643, the "Main" color now works reliably in both of the Pullquote variations. The display of the default pullquote is has no top or bottom border, but if someone specifies the "Main" color, borders using that color will be added.

Additionally the color palette should be expanded. I will take a stab at this but CC: @kjellr for insights.

The palette was expanded from 1 color to 5 colors in https://github.com/WordPress/twentynineteen/pull/642

jasmussen commented 5 years ago

I hope I followed everything here but I'd say the main issue is that the limited numbers of swatches make it tough to ensure contrast. The code put in place to ensure text stays visible should work with slightly less opinionated pull quote colours, I think.

Yeah sorry that got very cryptic.

However that's actually not the biggest issue. The biggest issue as I can tell is that setting the "main" color also sets the text color. This is the cause of most of the issues up there, and is only slightly mitigated by expanding the color palette.

jasmussen commented 5 years ago

To elaborate on this, there are two problems with automatically setting the text color when you set the main color:

  1. It overrides any choice the user might have made, i.e. picked a custom text color first. It's also unpredictable, as one option usually only sets that one option.
  2. It is actually as bad and sometimes worse for contrast, because as was the case initially, there was only one swatch registered for TwentyNineteen, blue, and when it was the only one to choose it was blue on blue with zero contrast. At least by keeping the text color the default black, the user would've seen the contrast warning, and been able to act on it.
dreamwhisper commented 5 years ago

From what I've experienced, if there are 2 custom colors set by the theme, when selecting one, the alternate color is being selected as the text color. My expectation is that if I have not specified a text color, the color would be set with acceptable contrast.

This seems to work well with buttons, but not with the pullquotes.

tomdevisser commented 1 year ago

Since it's been inactive a long time, is this still considered an issue or has it been resolved in the meantime? What can we do to move this forward? @jasmussen @dreamwhisper @La-Geek

jasmussen commented 1 year ago

Thanks for the ping. The UI has advanced substantially since this issue, with both a better color picker system, as well as the retirement of the pullquote style variation:

Screenshot 2023-03-13 at 10 53 44

In that light, I'll close this one. If the issue remains in some form, let me know and we can open a new issue describing what remains to do.