-
Notifications
You must be signed in to change notification settings - Fork 258
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
chore(README): badges now are in the terminal theme #1026
Conversation
I think passing / failing should still be green / red and not some custom Color. When the successful Color is closer to red it’s harder to see at a glance. |
The left side should probably be the dark grey color (light black), so the badges stand out from the background. If there's several different shades of the same color (like with the greens / reds), that seems like it would look a bit worse compared to the existing situation where the colors are consistent but not the same as the terminal. This is a good idea, but I think it might be blocked by constraints. |
I like this idea, and I follow it for my projects as well. Color coordinating stuff is nice. But as other maintainers said, not sure if we can make much customizations due to the fact that some of the colors are the indicator of the status. I still like the purple though :) |
You're right. The grey looks much like the default one, so we rather keep the background. We can atleast keep the blue and magenta. I think the rust logos should have the red as the failing red for consistency. Also I think it would be better if we had for the badge style because on larger fonts, the dependencies is misrendered. |
Btw is there a reason we don't use &link in the badges? |
Yeah - the blue and magenta change seems like it would be nice
The badges get rendered in a bit of a constrained area in https://crates.io/crates/ratatui so the current size works better there. I think it might be difficult to find something that works well at all font sizes unfortunately, though perhaps changes "dependencies" to "deps.rs" would work.
It's not something I've seen before and had to look it up (https://shields.io/badges):
These get rendered as img tags, so the &link approach won't work. |
Do you know what red the failing build produces? |
No, my code is always perfect and never breaks the build ;P I jest, but I don't know off the top of my head what the exact color is ;) |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The badges in the readme were all the default theme. Giving them prettier colors that match the terminal gif is better. I've used the colors from the VHS repo.
The badges in the readme were all the default theme. Giving them prettier colors that match the terminal gif is better. I've used the colors from the VHS repo.
The dependencies badge couldn't unfortunately be themed