Add Primitives v8 colors #529

langermank · 2023-03-26T15:22:55Z

Summary

A much cleaner version of #478 😄

This is the initial setup for testing our next major v8 release. Everything will be tested behind a feature flag to start, but ideally we avoid visual bugs by testing all of this in Storybook first! 🤞

Notes

I removed some GitHub specific tokens that we have moved to dotcom, such as Checks and some old marketing tokens
I'm keeping the shadow values the same for an easier rollout, and we can revisit later
All states are using our new formula with mix/alpha, which I will document soon
The disabled ToggleSwitch hover state is a bug in PRC, not with primitives

What should reviewers focus on?

This is a challenging PR to review. I would recommend focusing on visually testing in Storybook. Please focus on the following:

Ensure that I have copied over all a11y color updates correctly
Test the Button variants and states in all color themes. I did quite a bit of work on these. I feel like something might be off about the primary border → Storybook link
Test all "control" type components in all color themes → Storybook link
Scan through the comparison tables for large discrepancies → Storybook link

Supporting resources (related issues, external links, etc):

Part of https://github.com/github/primer/issues/1345

Contributor checklist:

All new and existing CI checks pass
Tests prove that the feature works and covers both happy and unhappy paths
Any drop in coverage, breaking changes or regressions have been documented above
All developer debugging and non-functional logging has been removed
Related issues have been referenced in the PR description

Reviewer checklist:

Check that pull request and proposed changes adhere to our contribution guidelines and code of conduct
Check that tests prove the feature works and covers both happy and unhappy paths
Check that there aren't other open Pull Requests for the same update/change
Verify the design tokens changed in this PR are expected using the diffing results in this PR

changeset-bot · 2023-03-26T15:22:58Z

🦋 Changeset detected

Latest commit: 99be152

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/primitives	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-03-26T15:23:13Z

Variables changed

No variables changed

github-actions · 2023-03-26T15:25:47Z

🟢 No design token changes found

lukasoppermann · 2023-03-27T06:49:05Z

src/platforms/css.ts


  return {
-    selector: `[data-color-mode="${mode}"][data-${mode}-theme="${themeName}"]`,
+    selector: `[data-color-mode="light"][data-light-theme="${themeName}"], [data-color-mode="dark"][data-dark-theme="${themeName}"]`,


Why are you changing this?

I was working on updating the theme picker in Storybook last week with @colebemis and we found a bug with the selector stack. I think the theme picker was written in such a way that the theme would never fail to match the selector, and Cole suggested this change to make it match the logic in dotcom. What do you think?

If this is really the case, I will do a PR just for this, so that it is not hidden in this giant one and can be more easily tracked.

However I am not sure this is actually correct. Maybe @colebemis can add to this discussion.
In the current version we get the following:

[data-color-mode="light"][data-light-theme="light_colorblind"], [data-color-mode="auto"][data-light-theme="light_colorblind"] { or [data-color-mode="dark"][data-dark-theme="dark_high_contrast"], [data-color-mode="auto"][data-light-theme="dark_high_contrast"] {

With the proposed changes we get:

[data-color-mode="light"][data-light-theme="light_colorblind"], [data-color-mode="dark"][data-dark-theme="light_colorblind"], [data-color-mode="auto"][data-light-theme="light_colorblind"] { or [data-color-mode="light"][data-light-theme="dark_high_contrast"], [data-color-mode="dark"][data-dark-theme="dark_high_contrast"], [data-color-mode="auto"][data-light-theme="dark_high_contrast"] {

What is added is a line I think will never execute. As far as I see you can't get a combination of data-color-mode="dark" and a light theme like data-dark-theme="light_colorblind". The only way you can get a light theme in dark mode is using auto.

Am I missing something?

What is added is a line I think will never execute. As far as I see you can't get a combination of data-color-mode="dark" and a light theme like data-dark-theme="light_colorblind". The only way you can get a light theme in dark mode is using auto.

Am I missing something?

Yeah that's correct, in dotcom ui, when selecting single mode, you'll only get the corresponding data-color-mode to the stored theme value. The mismatch only happens if someone selects the opposite theme for there light/dark auto themes.

Removing this change for now, and we can bring it back later if we find it necessary in dotcom!

Ah, you're right. I got confused because that combination is technically possible in code. I didn't realize dotcom prevents that from happening

simurai

The diff is kinda hard to read. So I mostly clicked through Storybook.

Haven't really noticed anything except maybe the Banners in the dark theme:

Before	After

But I assume that's intended?

simurai · 2023-03-27T08:08:14Z

.changeset/cold-houses-swim.md

+"@primer/primitives": patch
+---
+
+Add Primitives v8 colors (private dist for testing)


Do we have another name for this effort? It would be v3 of renaming efforts, although it doesn't match Primitives's current semver version. 🤔

I'm trying to refer to this as the next major (v8) release of Primitives vs. v3, which I think sounds a little nicer and more accurate! In v8 we will also ship all the sizing tokens without fallbacks, so it truly feels like a big major release.

langermank · 2023-03-27T15:28:49Z

@simurai @lukasoppermann I think I've addressed the alpha issue with status colors in dark mode 🤞

simurai · 2023-03-28T01:28:47Z

I think I've addressed the alpha issue with status colors in dark mode

Yes, Banners look good. 👍

Scanned through the Tables > Primitives in the dark theme again:

Replacing color-neutral-subtle with bgColor-neutral-muted would have a bigger color difference. Is that intentional? Haven't checked where we use that.

Also, I the bgColor-closed-muted seems to still use less transparency:

And these two, that I'm not sure 🤔 :

langermank · 2023-03-28T02:33:22Z

@simurai thank you! Great catch, appreciate that attention to detail ✨

I fixed both bgColor-neutral-muted and bgColor-closed-muted. The other two I'm not really worried about. We introduced a new backdrop color with Overlay, and everyone felt it was too light. So I worked on a new value somewhere in between the new and old :D

I don't think that border token is used much, and we have more control now over "control" borders. So I'm essentially just removing it.

simurai

I fixed both bgColor-neutral-muted and bgColor-closed-muted.

👍 Ok, maybe good to ship? 😅

Doesn't mean everything needs to be super final and we can make follow-ups if we discover problems once we start to migrate to the new variables.

langermank added 6 commits March 26, 2023 07:05

move over from old pr

af22fc4

remove old files

180d645

replace decorative with muted

d4d996c

remove decorative

fcc6841

fix high contrast

405f202

lukas' tests saving the day

f931a27

Create .changeset/cold-houses-swim.md

dad00b1

github-actions bot temporarily deployed to storybook-preview March 26, 2023 15:28 Inactive

langermank temporarily deployed to github-pages March 26, 2023 15:28 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview March 26, 2023 15:29 Inactive

langermank marked this pull request as ready for review March 26, 2023 15:30

langermank requested a review from a team as a code owner March 26, 2023 15:30

langermank requested review from a team, rezrah and broccolinisoup March 26, 2023 15:30

lukasoppermann reviewed Mar 27, 2023

View reviewed changes

simurai reviewed Mar 27, 2023

View reviewed changes

use correct alpha for muted bg in dark

b58db20

langermank temporarily deployed to github-pages March 27, 2023 15:29 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview March 27, 2023 15:29 Inactive

Merge branch 'main' into colors-primitives-v8

3bb80ed

langermank temporarily deployed to github-pages March 28, 2023 00:40 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview March 28, 2023 00:41 Inactive

fix dark alpha

1f024b8

langermank temporarily deployed to github-pages March 28, 2023 02:38 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview March 28, 2023 02:38 Inactive

simurai approved these changes Mar 28, 2023

View reviewed changes

undo change to css build

99be152

langermank temporarily deployed to github-pages March 28, 2023 18:00 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview March 28, 2023 18:01 Inactive

langermank merged commit 05edec6 into main Mar 28, 2023

langermank deleted the colors-primitives-v8 branch March 28, 2023 18:05

langermank mentioned this pull request Mar 28, 2023

[Draft] Updated color names #478

Closed

primer-css mentioned this pull request Mar 28, 2023

Release Tracking #500

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Primitives v8 colors #529

Add Primitives v8 colors #529

langermank commented Mar 26, 2023 •

edited

Loading

changeset-bot bot commented Mar 26, 2023 •

edited

Loading

github-actions bot commented Mar 26, 2023 •

edited

Loading

github-actions bot commented Mar 26, 2023 •

edited

Loading

lukasoppermann Mar 27, 2023

langermank Mar 27, 2023

lukasoppermann Mar 28, 2023

jonrohan Mar 28, 2023

langermank Mar 28, 2023

colebemis Mar 29, 2023

simurai left a comment

simurai Mar 27, 2023 •

edited

Loading

langermank Mar 27, 2023

langermank commented Mar 27, 2023

simurai commented Mar 28, 2023 •

edited

Loading

langermank commented Mar 28, 2023

simurai left a comment

Add Primitives v8 colors #529

Add Primitives v8 colors #529

Conversation

langermank commented Mar 26, 2023 • edited Loading

Summary

Notes

What should reviewers focus on?

Supporting resources (related issues, external links, etc):

Contributor checklist:

Reviewer checklist:

changeset-bot bot commented Mar 26, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 26, 2023 • edited Loading

github-actions bot commented Mar 26, 2023 • edited Loading

🟢 No design token changes found

lukasoppermann Mar 27, 2023

Choose a reason for hiding this comment

langermank Mar 27, 2023

Choose a reason for hiding this comment

lukasoppermann Mar 28, 2023

Choose a reason for hiding this comment

jonrohan Mar 28, 2023

Choose a reason for hiding this comment

langermank Mar 28, 2023

Choose a reason for hiding this comment

colebemis Mar 29, 2023

Choose a reason for hiding this comment

simurai left a comment

Choose a reason for hiding this comment

simurai Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

langermank Mar 27, 2023

Choose a reason for hiding this comment

langermank commented Mar 27, 2023

simurai commented Mar 28, 2023 • edited Loading

langermank commented Mar 28, 2023

simurai left a comment

Choose a reason for hiding this comment

langermank commented Mar 26, 2023 •

edited

Loading

changeset-bot bot commented Mar 26, 2023 •

edited

Loading

github-actions bot commented Mar 26, 2023 •

edited

Loading

github-actions bot commented Mar 26, 2023 •

edited

Loading

simurai Mar 27, 2023 •

edited

Loading

simurai commented Mar 28, 2023 •

edited

Loading