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

docs: improve badges #2704

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

realguse
Copy link
Contributor

Originally in #2674

What is the purpose of this pull request?

  • Documentation update

Description

This PR aims to update the badges to a more modern style (flat-square and for-the-badge).

Before Submitting

@realguse realguse mentioned this pull request Dec 21, 2024
3 tasks
@github-actions github-actions bot added 🧳 lucide package About the lucide package 🅰️ angular package About the Angular package ⚛️ preact package About the Preact Package ⚛️ react package Lucide React Package ⚛️ react native package About the React Native package 🪝 solid package About the Solid package 🪨 static package About the static package 🧣 svelte package About the Svelte package 💎 vue package Lucide Vue package labels Dec 21, 2024
@realguse realguse marked this pull request as ready for review December 21, 2024 20:50
@realguse
Copy link
Contributor Author

@jguddas, based on you contrast "issue", i have not found any solution. There doesn't seem to be a option to change the foreground/textcolor, sorry

@jguddas
Copy link
Member

jguddas commented Dec 21, 2024

I did some testing, the dynamic color switch logic seams to only trigger when using hex values like by for example adding &color=febcda to the end of the url, weird.

Maybe worth doing A bug report for this, needs more testing tho, can't right now, I'm only on the phone.

What I suggest we do is find the hex value of the current color and try if adding it to the url helps.

@realguse
Copy link
Contributor Author

Yeah, look likes that's the problem. I'll only add the color param to the ones that actually switches color automatically to keep things simple

@realguse
Copy link
Contributor Author

After testing the color param on the badges, none of them changed their foreground color. So that means the contrast is sufficient enough according to shields.io

README.md Outdated
<a href="https://www.figma.com/community/plugin/939567362549682242/Lucide-Icons"><img src="https://img.shields.io/badge/Figma-F24E1E?logo=figma&logoColor=white" alt="figma installs"></a>
<a href="https://github.com/lucide-icons/lucide/actions/workflows/release.yml"><img src="https://github.com/lucide-icons/lucide/actions/workflows/release.yml/badge.svg" alt="build status"></a>
<a href="https://discord.gg/EH6nSts"><img src="https://img.shields.io/discord/723074157486800936?label=chat&logo=discord&logoColor=%23ffffff&colorB=%237289DA" alt="discord chat"></a>
<a href="https://github.com/lucide-icons/lucide/blob/main/LICENSE"><img src="https://img.shields.io/npm/l/lucide?style=for-the-badge" alt="license"></a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<a href="https://github.com/lucide-icons/lucide/blob/main/LICENSE"><img src="https://img.shields.io/npm/l/lucide?style=for-the-badge" alt="license"></a>
<a href="https://github.com/lucide-icons/lucide/blob/main/LICENSE"><img src="https://img.shields.io/npm/l/lucide?style=for-the-badge&color=008c14" alt="license"></a>

I like @JoshuaKGoldberg s suggestion of changing the background colors to be a bit darker.
badges/shields#5497

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that feels more right. I'll do it now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like #008c14 gets too dark, another option is to use brightgreen which looks like this:
image

Copy link
Member

@jguddas jguddas Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimum contrast to get me to be happy is 4.5.

We can try other shades and colors tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1A871B

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible colors:
image

image

image

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also make all the badges the lucide brand color, still pretty low tho.

Copy link
Contributor Author

@realguse realguse Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should we even care about the contrast, there's a bunch of repos (and I mean a bunch) using brightgreen and white (fg) in their badges

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not necessary to be readable, it's not necessary and should be removed entirely, I'm pretty strongly opinionated on this one :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some colors with high enough (very close at least) contrast ratio. I'm not updating the other ones until we've found the final colors

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🅰️ angular package About the Angular package 🧳 lucide package About the lucide package ⚛️ preact package About the Preact Package ⚛️ react native package About the React Native package ⚛️ react package Lucide React Package 🪝 solid package About the Solid package 🪨 static package About the static package 🧣 svelte package About the Svelte package 💎 vue package Lucide Vue package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants