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

Update the color of text-secondary #5151

Closed
fcoveram opened this issue Nov 7, 2024 · 6 comments · Fixed by #5148
Closed

Update the color of text-secondary #5151

fcoveram opened this issue Nov 7, 2024 · 6 comments · Fixed by #5148
Labels
💻 aspect: code Concerns the software code in the repository ✨ 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

fcoveram commented Nov 7, 2024

Problem

During the last monthly call (Nov 6th), we talked about the color feeling of text-secondary in content-heavy pages in light theme and how the lighter shade feels too soft for reading, as part of #5148

We decided to make the text-secondary color darker in light theme while keeping the lighter version in dark theme.

Description

Change text-secondary from gray-9 to gray-12 in light theme. The outcome uses the same gray shade in both text and text-secondary.

The Figma's Design Library variables were updated with this change.

Screenshot of Figma variables of Openverse Design Library

@fcoveram fcoveram added ✨ goal: improvement Improvement to an existing user-facing feature 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Nov 7, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Nov 7, 2024
@dhruvkb
Copy link
Member

dhruvkb commented Nov 7, 2024

@fcoveram this change would make the secondary text as dark as the primary in the light theme. Is that intentional? I thought we would be picking something darker but not as much as the primary.

@openverse-bot openverse-bot moved this from 📋 Backlog to 🏗 In Progress in Openverse Backlog Nov 7, 2024
@fcoveram
Copy link
Contributor Author

fcoveram commented Nov 7, 2024

It is intentional but open to change.

I can try gray-10 and gray-11 in my browser inspector, and perhaps you both @obulat and @dhruvkb do the same and share your preferences here. What do you think?

@dhruvkb
Copy link
Member

dhruvkb commented Nov 7, 2024

I tried your suggestion @fcoveram and to be honest, I did not observe major differences in contrast between gray-11 and gray-12, (and gray-10 felt a bit lighter and harder to read) so I'm inclined to accept your first suggestion and use gray-12.

@fcoveram
Copy link
Contributor Author

fcoveram commented Nov 7, 2024

I tried both colors and gray-12 keeps feeling better than lighter versions.

@obulat
Copy link
Contributor

obulat commented Nov 7, 2024

I agree, @fcoveram, gray-12 feels more readable than the lighter colors.

@fcoveram
Copy link
Contributor Author

fcoveram commented Nov 8, 2024

Let's go with gray-12 then. We can always change it in the future if needed.

@krysal krysal added 🟨 priority: medium Not blocking but should be addressed soon 💻 aspect: code Concerns the software code in the repository 🧱 stack: frontend Related to the Nuxt frontend and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Nov 11, 2024
@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Nov 25, 2024
@openverse-bot openverse-bot moved this from ✅ Done to 🏗 In Progress in Openverse Backlog Nov 25, 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 ✨ 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: 🏗 In Progress
Development

Successfully merging a pull request may close this issue.

4 participants