Implement functional color vars [SideNav, StateLabel, SubNav, TabNav] #1085

VanAnderson · 2021-02-25T17:33:42Z

This PR updates the following components to use functional color variables in preparation for color mode support:

SideNav
StateLabel
SubNav
TabNav

Part of https://github.com/github/design-systems/issues/1219

vercel · 2021-02-25T17:33:44Z

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/AJeKKuWEf6DvgieoyKKNAP7MsReY
✅ Preview: https://primer-compone-git-vananderson-implement-functional-colo-639b39.vercel.app

VanAnderson · 2021-02-25T17:35:32Z

src/__tests__/__snapshots__/SideNav.tsx.snap

@@ -87,7 +87,7 @@ exports[`SideNav SideNav.Link renders consistently 1`] = `

 .c1.variant-normal > .c0[aria-current='page']::before,
 .c1.variant-normal > .c0[aria-selected='true']::before {
-  background-color: #f66a0a;
+  background-color: #f9826c;


Wasn't quite sure if this was okay or not - my understanding was that implementing these functional color variables would not change the presentation.

This change comes from using colors.sidenav.borderActive which seems like the obvious functional choice here and is very close to the original color. Thoughts on what we should do here?

changeset-bot · 2021-02-26T16:06:47Z

🦋 Changeset detected

Latest commit: b578a73

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

This PR includes changesets to release 1 package

Name	Type
@primer/components	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

VanAnderson · 2021-02-26T16:08:58Z

src/__tests__/__snapshots__/SubNavLink.tsx.snap

@@ -114,7 +114,7 @@ exports[`SubNav.Link renders consistently 1`] = `

 .c0:hover .SubNav-octicon,
 .c0:focus .SubNav-octicon {
-  color: #6a737d;
+  color: #586069;


shift to colors.icon.secondary - changes by 1 shade

src/Link.tsx

colebemis

Looking good! StateLabel needs a few updates to match the Primer CSS implementation.

colebemis · 2021-03-01T17:53:30Z

src/StateLabel.tsx

@@ -38,7 +38,7 @@ const StateLabelBase = styled.span<StyledStateLabelBaseProps>`
  align-items: center;


StateLabel has variants that will need to be moved into this file. You can use https://github.com/primer/css/blob/mkt/color-modes-whee/src/labels/states.scss as an implementation reference.

Here's documentation on how to create variants with styled system: https://styled-system.com/variants

VanAnderson · 2021-03-02T17:45:39Z

@colebemis I think you commented out the default theme for sidenav and it might be causing this test to fail. Should I remove the test?

colebemis · 2021-03-03T15:58:15Z

src/StateLabel.tsx

 })

 const sizeVariants = variant({
  prop: 'variant',
-  scale: 'stateLabels.sizes'
+  scale: 'stateLabels.sizes',


I don't think this line is necessary anymore

Suggested change

scale: 'stateLabels.sizes',

colebemis · 2021-03-03T15:58:27Z

src/StateLabel.tsx

@@ -18,12 +18,54 @@ const octiconMap = {

 const colorVariants = variant({
  prop: 'status',
-  scale: 'stateLabels.status'
+  scale: 'stateLabels.status',


Suggested change

scale: 'stateLabels.status',

scale: 'stateLabels.status',

src/StateLabel.tsx

Co-authored-by: Cole Bemis <colebemis@github.com>

src/SubNav.tsx

Co-authored-by: Cole Bemis <colebemis@github.com>

colebemis

🎉 Nice work!

VanAnderson · 2021-03-03T17:09:06Z

@colebemis thanks for the assist on SubNav there - I think this first one was before I fully understood how to pull over the right vars from mkt/color-modes-whee

VanAnderson added the status: wip label Feb 25, 2021

VanAnderson commented Feb 25, 2021

View reviewed changes

VanAnderson force-pushed the VanAnderson/implement-functional-color-vars branch from 40e0a12 to 327dc6f Compare February 26, 2021 16:06

vercel bot temporarily deployed to Preview February 26, 2021 16:06 Inactive

VanAnderson commented Feb 26, 2021

View reviewed changes

VanAnderson changed the title ~~[WIP] implement functional color vars~~ Implement functional color vars [SideNav, StateLabel, SubNav, TabNav, Link] Feb 26, 2021

VanAnderson marked this pull request as ready for review February 26, 2021 16:11

VanAnderson removed the status: wip label Feb 26, 2021

colebemis reviewed Feb 26, 2021

View reviewed changes

src/Link.tsx Outdated Show resolved Hide resolved

VanAnderson force-pushed the VanAnderson/implement-functional-color-vars branch from 327dc6f to 56bf18a Compare February 26, 2021 18:00

VanAnderson changed the title ~~Implement functional color vars [SideNav, StateLabel, SubNav, TabNav, Link]~~ Implement functional color vars [SideNav, StateLabel, SubNav, TabNav] Feb 26, 2021

vercel bot temporarily deployed to Preview March 1, 2021 17:25 Inactive

vercel bot temporarily deployed to Preview March 1, 2021 17:28 Inactive

colebemis requested changes Mar 1, 2021

View reviewed changes

vercel bot temporarily deployed to Preview March 1, 2021 17:55 Inactive

VanAnderson and others added 7 commits March 2, 2021 11:19

implement functional color variables for SideNav

f2c5779

implement functional color variables for StateLabel

4e19045

implement functional color variables for SubNav

c44ee36

implement functional color variables for TabNav

6b2dc95

Update SideNav colors to match Primer CSS

320e5a5

update SideNav snapshot

ff6b35b

move stateLabel variants to component

1a33cf0

VanAnderson force-pushed the VanAnderson/implement-functional-color-vars branch from cefc1b6 to 1a33cf0 Compare March 2, 2021 17:20

vercel bot temporarily deployed to Preview March 2, 2021 17:20 Inactive

Merge branch 'main' into VanAnderson/implement-functional-color-vars

579db1e

vercel bot temporarily deployed to Preview March 3, 2021 15:56 Inactive

colebemis reviewed Mar 3, 2021

View reviewed changes

remove scale from StateLabel variants

1b49ec5

vercel bot temporarily deployed to Preview March 3, 2021 16:05 Inactive

colebemis reviewed Mar 3, 2021

View reviewed changes

src/StateLabel.tsx Outdated Show resolved Hide resolved

src/StateLabel.tsx Outdated Show resolved Hide resolved

src/StateLabel.tsx Show resolved Hide resolved

src/StateLabel.tsx Outdated Show resolved Hide resolved

StateLabel revisions

c3fbffa

Co-authored-by: Cole Bemis <colebemis@github.com>

vercel bot temporarily deployed to Preview March 3, 2021 16:37 Inactive

colebemis reviewed Mar 3, 2021

View reviewed changes

src/SubNav.tsx Outdated Show resolved Hide resolved

src/SubNav.tsx Outdated Show resolved Hide resolved

src/SubNav.tsx Outdated Show resolved Hide resolved

Updates to SubNav functional variables

f9e9189

Co-authored-by: Cole Bemis <colebemis@github.com>

vercel bot temporarily deployed to Preview March 3, 2021 17:01 Inactive

update StateLabel and SubNav snapshots

b578a73

vercel bot temporarily deployed to Preview March 3, 2021 17:05 Inactive

colebemis approved these changes Mar 3, 2021

View reviewed changes

colebemis merged commit 108c273 into main Mar 3, 2021

colebemis deleted the VanAnderson/implement-functional-color-vars branch March 3, 2021 17:12

github-actions bot mentioned this pull request Mar 3, 2021

Release Tracking #1093

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement functional color vars [SideNav, StateLabel, SubNav, TabNav] #1085

Implement functional color vars [SideNav, StateLabel, SubNav, TabNav] #1085

VanAnderson commented Feb 25, 2021 •

edited

Loading

vercel bot commented Feb 25, 2021 •

edited

Loading

VanAnderson Feb 25, 2021

changeset-bot bot commented Feb 26, 2021 •

edited

Loading

VanAnderson Feb 26, 2021

colebemis left a comment

colebemis Mar 1, 2021

VanAnderson Mar 2, 2021

VanAnderson commented Mar 2, 2021 •

edited

Loading

colebemis Mar 3, 2021

colebemis Mar 3, 2021

colebemis left a comment

VanAnderson commented Mar 3, 2021

		@@ -38,7 +38,7 @@ const StateLabelBase = styled.span<StyledStateLabelBaseProps>`
		align-items: center;

Implement functional color vars [SideNav, StateLabel, SubNav, TabNav] #1085

Implement functional color vars [SideNav, StateLabel, SubNav, TabNav] #1085

Conversation

VanAnderson commented Feb 25, 2021 • edited Loading

vercel bot commented Feb 25, 2021 • edited Loading

VanAnderson Feb 25, 2021

Choose a reason for hiding this comment

changeset-bot bot commented Feb 26, 2021 • edited Loading

🦋 Changeset detected

VanAnderson Feb 26, 2021

Choose a reason for hiding this comment

colebemis left a comment

Choose a reason for hiding this comment

colebemis Mar 1, 2021

Choose a reason for hiding this comment

VanAnderson Mar 2, 2021

Choose a reason for hiding this comment

VanAnderson commented Mar 2, 2021 • edited Loading

colebemis Mar 3, 2021

Choose a reason for hiding this comment

colebemis Mar 3, 2021

Choose a reason for hiding this comment

colebemis left a comment

Choose a reason for hiding this comment

VanAnderson commented Mar 3, 2021

VanAnderson commented Feb 25, 2021 •

edited

Loading

vercel bot commented Feb 25, 2021 •

edited

Loading

changeset-bot bot commented Feb 26, 2021 •

edited

Loading

VanAnderson commented Mar 2, 2021 •

edited

Loading