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

Navigation: color consistence between front-end and editor-canvas #20017

Closed
retrofox opened this issue Feb 3, 2020 · 8 comments · Fixed by #44578
Closed

Navigation: color consistence between front-end and editor-canvas #20017

retrofox opened this issue Feb 3, 2020 · 8 comments · Fixed by #44578
Labels
[Block] Navigation Affects the Navigation Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended

Comments

@retrofox
Copy link
Contributor

retrofox commented Feb 3, 2020

Be consistent with text color in the front-end and the editor-canvas.

Twenty-twenty

front-end
image

editor-canvas
image

@retrofox retrofox changed the title Navigation Navigation: color consistence between front-end and editor-canvas Feb 3, 2020
@talldan talldan added [Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended labels Feb 6, 2020
@paaljoachim
Copy link
Contributor

paaljoachim commented Jan 18, 2021

I am doing a status update of older issues.

Text Color is sometimes working and other times not with theme Twenty Twenty.

Screen Shot 2021-01-18 at 22 28 45

I see the same problem in Twenty Twenty One.
Screen Shot 2021-01-18 at 22 32 21

Because of the color combination warnings a certain mix of colors is not being allowed to be used, but this is only in the backend. The frontend will show the colors.

Screen Shot 2021-01-18 at 22 35 04

WordPress 5.6
Gutenberg plugin 9.7.4
Local site running Desktop Server.
Chrome
Mac OSX 10.15.7

@kjellr

@jasmussen
Copy link
Contributor

I took a look today and it appears to be a specificity fight between themes, editor styles, and colors. Here's TwentyTwentyOne:

Screenshot 2021-01-27 at 12 02 37

Note how the text color doesn't work in the editor. But it works on the frontend:

Screenshot 2021-01-27 at 12 02 44

This is the offending rule:

Screenshot 2021-01-27 at 12 03 00

TwentyTwentyOne outputs this CSS in its editor style:

:not(.has-text-color).has-black-background-color[class],
:not(.has-text-color).has-gray-background-color[class],
:not(.has-text-color).has-dark-gray-background-color[class] {
	color: var(--global--color-white);
}

That has way higher specificity than what the navigation color system outputs:

.has-purple-color[class] {
    color: var(--global--color-purple);
}

Because of this added specificity, the editor style will always win this fight.

We could change the global styles to output this instead:

.has-purple-color[class][class][class] {
    color: var(--global--color-purple);
}

that would win the fight. However to me it feels like TwentyTwentyOne is doing it wrong, and is adding too high specificity. Since this was reported for TwentyTwenty as well, that either means two themes are doing it wrong, OR that what both themes are trying to accomplish is not well handled by the editor here.

I'm tempted to say both themes are doing it wrong, because it works on the frontend but not in the editor. But that could still point to a complexity in the editor markup that led to the extra specificity being added to the editor style in the first place.

@kjellr do you have any thoughts here?

@jasmussen
Copy link
Contributor

A good friend did some investigative work and uncovered that navigation li is missing the has-text-color on the editor. Seems like that could throw a wrench into the specificity wars, and that by adding this missing class in the editor, we'd solve the issue! @retrofox how hard would this be to address?

@retrofox
Copy link
Contributor Author

A good friend did some investigative work and uncovered that navigation li is missing the has-text-color on the editor. Seems like that could throw a wrench into the specificity wars, and that by adding this missing class in the editor, we'd solve the issue! @retrofox how hard would this be to address?

I think it won't be a big deal but I need to dive into the code since I've been away from a loooong time ago. Going to take a look in a few days if it's still useful. :-D

@jasmussen
Copy link
Contributor

Oh, didn't mean for you to necessarily fix it, just wanted a quick gut check! I'll see if I can find some help.

@carolinan carolinan added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Jul 14, 2022
@mrfoxtalbot
Copy link

While still not 100% consistent, this appears to be less of an issue now:

Screen Shot on 2022-10-03 at 12:22:19
Screen Shot on 2022-10-03 at 12:22:30
Screen Shot on 2022-10-03 at 12:23:00
Screen Shot on 2022-10-03 at 12:23:08

Do you have any thoughts on what to do here next @jasmussen?

@jasmussen
Copy link
Contributor

Oh nice, looks like #44578 also fixes this one.

@mrfoxtalbot
Copy link

Cool! I will add this to my list to review and close if it is no longer relevant after #44578 is merged.

Thank you @jasmussen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants