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

Refresh labels #1034

Merged
merged 10 commits into from Feb 24, 2020
Merged

Refresh labels #1034

merged 10 commits into from Feb 24, 2020

Conversation

simurai
Copy link
Contributor

@simurai simurai commented Feb 21, 2020

This updates the labels.

Screen Shot 2020-02-21 at 11 34 49 PM

👀 Preview

It's probably not the final version, but maybe ok to start testing on dotcom.

TODO

  • Update docs
  • Update styles: Different colors, rounded corners, adjusted padding etc.

API changes

There should be no breaking changes. But there are a few additions:

  • .Counter--small
  • .Label--yellow
  • .Label--red
  • .Label--green
  • .Label--blue

/cc @primer/ds-core

@simurai simurai added this to 🚧 Work in Progress in 📦 Primer CSS release tracking via automation Feb 21, 2020
@vercel
Copy link

vercel bot commented Feb 21, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/primer/primer-css/j11mu7ck5
✅ Preview: https://primer-css-git-next-labels.primer.now.sh

@simurai
Copy link
Contributor Author

simurai commented Feb 21, 2020

With some colors we're kinda "cheating".. or "work around the system".

For example instead of using...

.Label--green {
  color: $text-green;
  border-color: $border-green;
}

...we use:

.Label--green {
  color: $green-600;
  border-color: $green-500;
}

So I wonder if it would be better to change these "default" $text-green + $border-green variables everywhere and not just for labels?

- $text-green: $green-500;
+ $text-green: $green-600;

- $border-green: $green-400;
+ $border-green: $green-500;

Anyways, we can come back to balance the colors in a follow-up PR.

@simurai
Copy link
Contributor Author

simurai commented Feb 21, 2020

There is a new Label--custom that uses currentColor for the border. So you can use any text- utility or inline the color to create custom colored labels.

Screen Shot 2020-02-21 at 10 40 37 PM

Not sure if it's a good idea to add it? It's like IssueLabels but with an "outline style".

@auareyou
Copy link
Contributor

So I wonder if it would be better to change these "default" $text-green + $border-green variables everywhere and not just for labels?

Is there any way to know easily where we use this border other than labels? I changed it to the darker one because the current one is too light so with any green text that passes contrast it looks too different.

There is a new Label--custom that uses currentColor for the border. So you can use any text- utility or inline the color to create custom colored labels.

I think we might find ourselves not passing contrast ratio. Do you have an idea of a use case that could benefit from these?

@vercel vercel bot temporarily deployed to Preview February 21, 2020 14:04 Inactive
@simurai
Copy link
Contributor Author

simurai commented Feb 21, 2020

Is there any way to know easily where we use this border other than labels? I changed it to the darker one because the current one is too light so with any green text that passes contrast it looks too different.

We can search where they get used, but yeah.. it's not easy to find the right URL to also see the example.

For now, maybe better to just change the labels. We can revisit this later.

I think we might find ourselves not passing contrast ratio. Do you have an idea of a use case that could benefit from these?

On dotcom there is a script that switches between light/dark text color for the issue labels. So it would need to be something similar. But yeah, let's remove Label--custom again. 👍 It's easy to add if we need it at some point.

Copy link
Contributor

@auareyou auareyou left a comment

Choose a reason for hiding this comment

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

Fantastic! 🙌

📦 Primer CSS release tracking automation moved this from 🚧 Work in Progress to ✅ Approved for release Feb 21, 2020
@vercel vercel bot temporarily deployed to Preview February 21, 2020 14:29 Inactive
@simurai simurai merged commit 0dbb09a into next Feb 24, 2020
📦 Primer CSS release tracking automation moved this from ✅ Approved for release to 💜 Done Feb 24, 2020
@simurai simurai deleted the next-labels branch February 24, 2020 01:05
@simurai simurai mentioned this pull request Feb 24, 2020
34 tasks
@simurai simurai mentioned this pull request Jul 2, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants