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

Add functional color variables from Primer Primitives to the theme #1074

Merged
merged 11 commits into from
Feb 25, 2021

Conversation

VanAnderson
Copy link
Contributor

First item in https://github.com/github/design-systems/issues/1219

This PR deep merges in primer primitives into our theme colors and also separates out colors from compound css statements.

@vercel
Copy link

vercel bot commented Feb 23, 2021

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

🔍 Inspect: https://vercel.com/primer/primer-components/3QotiQcdjH2ncC9TUXbK34SHcv7m
✅ Preview: https://primer-components-git-vananderson-function-color-vars-primer.vercel.app

@vercel vercel bot temporarily deployed to Preview February 23, 2021 20:15 Inactive
@vercel vercel bot temporarily deployed to Preview February 23, 2021 20:42 Inactive
src/theme-preval.ts Outdated Show resolved Hide resolved
src/theme-preval.ts Outdated Show resolved Hide resolved
src/theme-preval.ts Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview February 23, 2021 21:41 Inactive
@vercel vercel bot temporarily deployed to Preview February 24, 2021 17:05 Inactive
@VanAnderson VanAnderson force-pushed the VanAnderson/function-color-vars branch from b9cc84d to b78b8f0 Compare February 24, 2021 17:10
@vercel vercel bot temporarily deployed to Preview February 24, 2021 17:10 Inactive
src/theme-preval.ts Outdated Show resolved Hide resolved
@VanAnderson VanAnderson marked this pull request as ready for review February 24, 2021 17:37
src/theme-preval.ts Outdated Show resolved Hide resolved
src/theme-preval.ts Outdated Show resolved Hide resolved
src/theme-preval.ts Outdated Show resolved Hide resolved
Co-authored-by: Cole Bemis <colebemis@github.com>
@vercel vercel bot temporarily deployed to Preview February 24, 2021 20:13 Inactive
@VanAnderson
Copy link
Contributor Author

@colebemis thank you so much for sending me that refactor! Your code was definitely a big improvement in readability, using reduce was a much better option. I implemented that and added a couple of unit tests to do a basic sanity check on filtering out Shadows and colors. I can get more in depth with the assertions if needed.

package.json Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview February 24, 2021 21:19 Inactive
Turns out preval was silently not working because it doesn't parse TypeScript
@vercel vercel bot temporarily deployed to Preview February 24, 2021 22:33 Inactive
@vercel vercel bot temporarily deployed to Preview February 25, 2021 17:01 Inactive
@vercel vercel bot temporarily deployed to Preview February 25, 2021 17:06 Inactive
Copy link
Contributor

@colebemis colebemis left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

@VanAnderson VanAnderson merged commit 14874ec into main Feb 25, 2021
@VanAnderson VanAnderson deleted the VanAnderson/function-color-vars branch February 25, 2021 17:12
@github-actions github-actions bot mentioned this pull request Feb 25, 2021
@colebemis colebemis changed the title Functional Color variables from primer/primitives in theme Add functional color variables from Primer Primitives to the theme Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants