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

Semantics for the new color system #4325

Open
Tracked by #3592
fcoveram opened this issue May 14, 2024 · 3 comments
Open
Tracked by #3592

Semantics for the new color system #4325

fcoveram opened this issue May 14, 2024 · 3 comments
Labels
💻 aspect: code Concerns the software code in the repository 🕹 aspect: interface Concerns end-users' experience with the software ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend

Comments

@fcoveram
Copy link
Contributor

Part of #3592

Description

As part of the new color system introduced with the dark mode designs #3959, the color scales asked to be implemented in #4268 have a semantic naming to describe how they are applied in Openverse UI.

This ticket details the alias for light and dark themes as CSS variables.

Light theme

:root {
	bg: var(white);
	bg-surface: var(gray-1);
	bg-overlay: var(white);
	bg-fill-primary: var(pink-8);
	bg-fill-primary-hover: var(pink-9);
	bg-fill-secondary: var(gray-2);
	bg-fill-secondary-hover: var(gray-12);
	bg-fill-terciary: var(gray-12);
	bg-fill-terciary-hover: var(gray-11);
	bg-fill-transparent-hover: var(gray-12-10);
	bg-fill-complementary: var(yellow-3);
	bg-fill-warning: var(warning-2);
	bg-fill-info: var(info-2);
	bg-fill-success: var(success-2);
	bg-fill-error: var(error-2);
	bg-fill-disabled: var(gray-5);
	bg-zero: var(white-0);
	border: var(gray-3);
	border-hover: var(gray-12);
	border-secondary: var(gray-12-20);
	border-secondary-hover: var(gray-12);
	border-tertiary: var(gray-12);
	border-transparent-hover: var(gray-3);
	border-focus: var(pink-8);
	border-bg-ring: var(white);
	border-disabled: var(gray-5);
	text: var(gray-12);
	text-secondary: var(gray-8);
	text-disabled: var(gray-5);
	text-link: var(pink-8);
	text-over-dark: var(white);
	text-secondary-over-dark: var(gray-5);
	icon-warning: var(warning-8);
	icon-info: var(info-8);
	icon-success: var(success-8);
	icon-error: var(error-8);
	wave-active: var(yellow-9);
	wave-inactive: var(gray-12-20);
	modal-layer: var(gray-12-80);
}

Dark theme

:root {
	bg: var(gray-13);
	bg-surface: var(gray-12);
	bg-overlay: var(gray-11);
	bg-fill-primary: var(yellow-4);
	bg-fill-primary-hover: var(yellow-3);
	bg-fill-secondary: var(gray-11);
	bg-fill-secondary-hover: var(gray-1);
	bg-fill-terciary: var(gray-1);
	bg-fill-terciary-hover: var(gray-2);
	bg-fill-transparent-hover: var(gray-1-10);
	bg-fill-complementary: var(pink-9);
	bg-fill-warning: var(warning-11);
	bg-fill-info: var(info-11);
	bg-fill-success: var(success-11);
	bg-fill-error: var(error-11);
	bg-fill-disabled: var(gray-8);
	bg-zero: var(gray-13-0);
	border: var(gray-11);
	border-hover: var(gray-1);
	border-secondary: var(gray-1-20);
	border-secondary-hover: var(gray-1);
	border-tertiary: var(gray-1);
	border-transparent-hover: var(gray-11);
	border-focus: var(yellow-4);
	border-bg-ring: var(gray-13);
	border-disabled: var(gray-8);
	text: var(gray-1);
	text-secondary: var(gray-5);
	text-disabled: var(gray-8);
	text-link: var(yellow-4);
	text-over-dark: var(gray-13);
	text-secondary-over-dark: var(gray-8);
	icon-warning: var(warning-5);
	icon-info: var(info-5);
	icon-success: var(success-5);
	icon-error: var(error-5);
	wave-active: var(pink-4);
	wave-inactive: var(gray-1-30);
	modal-layer: var(gray-12-60);
}
@fcoveram fcoveram added 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work ✨ goal: improvement Improvement to an existing user-facing feature 🕹 aspect: interface Concerns end-users' experience with the software 🧱 stack: frontend Related to the Nuxt frontend labels May 14, 2024
@fcoveram fcoveram mentioned this issue May 14, 2024
9 tasks
@zackkrida
Copy link
Member

@fcoveram this is excellent, thank you so much! I thought of something relating to this today. We may need to modify these names for the frontend:

These names have some redundancy with the CSS classes tailwind generates. For example, if we have a color called "primary" tailwind creates classes like "bg-primary", "text-primary", "border-primary", and so on. So if we used these names exactly as-written we would get generated classes like "bg-bg-fill-success" and so on. This isn't necessarily a huge problem but does seem a little inelegant.

Perhaps we could remove the bg-, and bg-fill- prefixes from some of the colors and keep all of the other names the same. We would end up with classes like text-text and border-border but it isn't terrible. @WordPress/openverse-frontend what do you think about this problem?

@sarayourfriend
Copy link
Contributor

Considering tailwind creates these classes for all colours, regardless of their intention, I think it's better to keep the bg and bg-fill prefixes. bg-bg-fill-success is ugly, but dropping the prefix makes classnames like text-surface, which should never be used. If we keep the prefixes, on the other hand, it would be text-bg-surface, which at least gives some indication that it's incorrect usage.

We'd still want to be careful here though because even the possibility of text-bg-surface is important to prevent if we can, through means other than relying on PR reviews catching it. Is it possibly to bypass tailwind's class generation altogether by disabling all tailwind-built colours (looks like colors: {} would do the trick) and then define these classes ourselves? We can probably pretty easily write a script to generate the list programmatically to reduce the burden of making changes in the future 🤔

Generally I think it would be a good idea to discourage misuse of these variables as much as possible.

@fcoveram
Copy link
Contributor Author

I agree with @sarayourfriend about the importance of adding information to prevent misuse.

Regarding naming, the difference between bg and bg-fill is that the latter works for individual elements of the UI, while the former is a section on the page. Because of this, there are more bg-fill elements in the collection.

The fill version could be renamed as fill without problem. In that vein, it seems better that Figma variables follow what's best in frontend. The Figma design library doesn't risk breaking the files attached to it.

@dhruvkb dhruvkb added 🟨 priority: medium Not blocking but should be addressed soon 💻 aspect: code Concerns the software code in the repository and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🕹 aspect: interface Concerns end-users' experience with the software ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend
Projects
Status: 📋 Backlog
Development

No branches or pull requests

4 participants