twentytwentythree icon indicating copy to clipboard operation
twentytwentythree copied to clipboard

Use "background" and "foreground" color slugs instead of "base" and "contrast" for consistency and compatibility

Open dianeco opened this issue 3 years ago • 16 comments

Currently, the theme.json declares base and contrast colors that match the background and text colors of the site. To be consistent with Twenty Twenty-Two, the Colors inside the Figma Styles page, and a majority of themes (Poe, Blockbase, Vivre, Pendant, Extendable, Lyna, Gutenify, eStory, Lawson...), we should consider using the background and foreground slugs along with the "Background" and "Foreground" names.

Moreover, having the same color slugs as Twenty Twenty-Two would allow users to easily switch from Twenty Twenty-Two to Twenty Twenty-Three.

dianeco avatar Aug 11 '22 14:08 dianeco

Unfortunately, these slugs have become prevalent. I do agree with @dianeco we mine as well keep it consistent.

colorful-tones avatar Aug 15 '22 10:08 colorful-tones

I generally agree with maintaining compat between themes, but background and foreground were poor choices early on. They generated slugs like .has-background-background-color and .has-foreground-background-color. Or, in cases where the text colors are flipped, those become .has-background-color (for a foreground) and .has-foreground-color (for a background).

I'd much rather suffer a little growing/learning pain now instead of having years of weird and confusing slugs and continuing to propagate their use throughout the theming community.

justintadlock avatar Aug 15 '22 19:08 justintadlock

I'm just worried we might go down this rabbit hole 🕳️ 🐇

colorful-tones avatar Aug 15 '22 19:08 colorful-tones

"background" is where is goes That's my brain saying "your background color is your background color"

"contrast" describes purpose for the color and a relationship with the palette That's my brain saying "your background color is your contrast color"

"primary" is a lot more like "contrast" than it is "background" I usually use "primary" for my button color, but I don't call the property "button".

I have used "foreground" and "background" for nearly every Theme I've taken part in. Using anything else would feel weird. But I agree that "base" and "contrast" are words that better suit the purpose.

I would really really really like those values to be standardized though.

pbking avatar Aug 15 '22 21:08 pbking

I don't mind "base" and "contrast". They're a bit abstracted from the elements they're declaring, but I think its fine.

Though I'd rather see WordPress using the values within styles.color.background and styles.color.text to create value programmatically that we could use throughout the theme (perhaps it is base/contrast) — just like it does for spacing now.

richtabor avatar Aug 15 '22 22:08 richtabor

I'm not a fan of .has-background-background-color either.

Moreover, having the same color slugs as Twenty Twenty-Two would allow users to easily switch from Twenty Twenty-Two to Twenty Twenty-Three.

However, this is my bigger concern.

I'd much rather suffer a little growing/learning pain now instead of having years of weird and confusing slugs and continuing to propagate their use throughout the theming community.

Ultimately, I'm swayed by @justintadlock 's reasoning and think this may be a great point to rip the band-aid off 😫 and switch to base and contrast.

So, I vote to stick with base and contrast.

colorful-tones avatar Aug 17 '22 11:08 colorful-tones

I noticed that the new core site header and footer patterns use the foreground and background color slugs.

carolinan avatar Aug 26 '22 07:08 carolinan

Thanks for highlighting that! Hmm, I'm not sure what the best way to handle the color names is now if the core patterns are using background and foreground.

I really like avoiding the use of background and foreground as slugs for the reasons described above, but I think the default theme and core patterns should agree on color naming.

It would be great if the core patterns could reference styles.color.background / styles.color.text as variables. Does anyone know if there's an existing Gutenberg issue or discussion around this? I'm struggling to find one.

mikachan avatar Aug 26 '22 09:08 mikachan

It would be great if the core patterns could reference styles.color.background / styles.color.text as variables. Does anyone know if there's an existing Gutenberg issue or discussion around this? I'm struggling to find one.

We can do it in theme.json, seems logical that the next step is that patterns can do it as well!

MaggieCabrera avatar Aug 26 '22 09:08 MaggieCabrera

I've created an issue! https://github.com/WordPress/gutenberg/issues/43627

mikachan avatar Aug 26 '22 09:08 mikachan

Just dropping back here to note that WooCommerce 6.9 Beta has new CSS to try and make the assimilation of block themes a bit more seamless, and there are several references to the following:

background-color: var(--wp--preset--color--foreground, $highlight);
color: var(--wp--preset--color--background, $highlightext);

You can see more of the changes in the newly added plugins/woocommerce/client/legacy/css/woocommerce-blocktheme.scss

It is nice that there are fallback values (e.g. $highlight) too, which is useful for theme developers to reference.

Also, I know that traditionally a Twenty Twenty-something custom stylesheet is often added to the WooCommerce project for new themes. So, we might want to keep some of this on the radar.

Here is a list of the past Twenty Twenty-something stylesheets in WooCommerce:

colorful-tones avatar Sep 08 '22 14:09 colorful-tones

Thank you for noting this, @colorful-tones! It's super helpful. 🙇

I'm still unsure of the best color naming for TT3. I prefer our current setup, but I don't want to make things difficult or too different if other prominent areas are using the background and foreground names.

mikachan avatar Sep 08 '22 17:09 mikachan

Just my two cents, as I fully agree with what @justintadlock says here:

I'd much rather suffer a little growing/learning pain now instead of having years of weird and confusing slugs and continuing to propagate their use throughout the theming community.

bgardner avatar Sep 13 '22 21:09 bgardner

~~I have little experience with the vertical bar syntax, but I'm wondering if it might act as a nice compromise for these naming discussions? There is some good context and history around its introduction here: https://github.com/WordPress/gutenberg/issues/42204 and I recommend chiming in if there is interest~~

I misunderstood the implications of using the vertical bar syntax and this is not relevant.

colorful-tones avatar Sep 19 '22 10:09 colorful-tones

I'm not sure the vertical bar syntax would help in this context, as we'd still only be able to reference the colour palette variable, e.g. var:preset|color|base rather than var(--wp--preset--color--base). It doesn't allow us to reference settings like styles.color.background and styles.color.text.

Apologies if I've misunderstood here - were you thinking of another way we could use it?

mikachan avatar Sep 19 '22 13:09 mikachan

I'm not sure the vertical bar syntax would help in this context, as we'd still only be able to reference the colour palette variable, e.g. var:preset|color|base rather than var(--wp--preset--color--base). It doesn't allow us to reference settings like styles.color.background and styles.color.text.

Ah yes, I figured I was missing some of the understanding around the limitations and usage.

Apologies if I've misunderstood here - were you thinking of another way we could use it?

No, and thanks for clarifying. Please disregard my commentary on the possibility of using the vertical bar syntax as it is not relevant to this issue. 👍

colorful-tones avatar Sep 19 '22 15:09 colorful-tones

I think that it's essential to have consistency between themes. Most of the latest block themes are not using base and contrast, including the ones made by Automattic.

If the new default theme uses base and contrast, IMO this naming convention should be written in the theme handbook to promote its use throughout the theming community. And from now on, the default themes (2024, 2025...) should stick to it. It would also be helpful for plugins. As @colorful-tones said, plugins like WooCommerce currently use background and foreground.

dianeco avatar Oct 25 '22 11:10 dianeco

If the new default theme uses base and contrast, IMO this naming convention should be written in the theme handbook to promote its use throughout the theming community.

Agreed.

richtabor avatar Oct 25 '22 14:10 richtabor

I really do hope that base and contrast are chosen. As a reminder:

I'd much rather suffer a little growing/learning pain now instead of having years of weird and confusing slugs and continuing to propagate their use throughout the theming community.

bgardner avatar Oct 25 '22 14:10 bgardner

If the new default theme uses base and contrast, IMO this naming convention should be written in the theme handbook to promote its use throughout the theming community.

Agree. Also, embedded in stone and shouted from all of the hilltops! 😃 📢

colorful-tones avatar Oct 25 '22 20:10 colorful-tones

Thanks, all. I appreciate the continued discussion around this. I would also like to stick with the 'base' and 'contrast' names for the same reasons listed above.

I really like the idea of encouraging a standardized color naming convention. I've opened an issue here for the theme handbook: https://github.com/WordPress/Documentation-Issue-Tracker/issues/563. Please feel free to edit / let me know if I've missed anything here.

mikachan avatar Oct 27 '22 18:10 mikachan

Closing as the theme has been merged into the WordPress Core SVN repository and is no longer maintained on GitHub.

mikachan avatar Oct 28 '22 14:10 mikachan