Skip to content
This repository has been archived by the owner on Oct 28, 2022. It is now read-only.

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

Closed
dianeco opened this issue Aug 11, 2022 · 22 comments

Comments

@dianeco
Copy link

dianeco commented Aug 11, 2022

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.

@colorful-tones
Copy link
Member

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

@justintadlock
Copy link

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.

@colorful-tones
Copy link
Member

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

@pbking
Copy link
Collaborator

pbking commented Aug 15, 2022

"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.

@richtabor
Copy link
Member

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.

@colorful-tones
Copy link
Member

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.

@carolinan
Copy link
Collaborator

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

@mikachan
Copy link
Member

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.

@MaggieCabrera
Copy link
Collaborator

MaggieCabrera commented Aug 26, 2022

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!

@mikachan
Copy link
Member

I've created an issue! WordPress/gutenberg#43627

@colorful-tones
Copy link
Member

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:

@mikachan
Copy link
Member

mikachan commented Sep 8, 2022

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.

@bgardner
Copy link

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.

@colorful-tones
Copy link
Member

colorful-tones commented Sep 19, 2022

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: WordPress/gutenberg#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.

@mikachan
Copy link
Member

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?

@colorful-tones
Copy link
Member

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. 👍

@dianeco
Copy link
Author

dianeco commented Oct 25, 2022

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.

@richtabor
Copy link
Member

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.

@bgardner
Copy link

bgardner commented Oct 25, 2022

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.

@colorful-tones
Copy link
Member

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! 😃 📢

@mikachan
Copy link
Member

mikachan commented Oct 27, 2022

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: WordPress/Documentation-Issue-Tracker#563. Please feel free to edit / let me know if I've missed anything here.

@mikachan
Copy link
Member

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants