Skip to content

Add Primitives v8 colors #529

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.

Sign up for GitHub

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

Merged
langermank merged 11 commits into from
Mar 28, 2023
Merged

Add Primitives v8 colors #529

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
af22fc4
move over from old pr
langermank Mar 26, 2023
180d645
remove old files
langermank Mar 26, 2023
d4d996c
replace decorative with muted
langermank Mar 26, 2023
fcc6841
remove decorative
langermank Mar 26, 2023
405f202
fix high contrast
langermank Mar 26, 2023
f931a27
lukas' tests saving the day
langermank Mar 26, 2023
dad00b1
Create .changeset/cold-houses-swim.md
langermank Mar 26, 2023
b58db20
use correct alpha for muted bg in dark
langermank Mar 27, 2023
3bb80ed
Merge branch 'main' into colors-primitives-v8
langermank Mar 28, 2023
1f024b8
fix dark alpha
langermank Mar 28, 2023
99be152
undo change to css build
langermank Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cold-houses-swim.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/primitives": patch
---

Add Primitives v8 colors (private dist for testing)
Copy link
Contributor

@simurai simurai Mar 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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. 🤔

Copy link
Contributor Author

@langermank langermank Mar 27, 2023

Choose a reason for hiding this comment

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

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.

2 changes: 1 addition & 1 deletion docs/storybook/.storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ const tempTheme = deepmerge(theme, {
borderHover: 'var(--underlineNav-borderColor-hover)',
},
actionListItem: {
inlineDivider: 'var(--borderColor-decorative)',
inlineDivider: 'var(--borderColor-muted)',
default: {
hoverBg: 'var(--control-transparent-bgColor-hover)',
hoverBorder: 'var(--control-transparent-borderColor-hover)',
Expand Down
8 changes: 1 addition & 7 deletions docs/storybook/stories/Color/Functional/Border.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,7 @@ export default {
},
}

const borderColors = [
'borderColor-default',
'borderColor-muted',
'borderColor-decorative',
'borderColor-emphasis',
'borderColor-disabled',
]
const borderColors = ['borderColor-default', 'borderColor-muted', 'borderColor-emphasis', 'borderColor-disabled']

export const Border = () => {
return (
Expand Down
2 changes: 1 addition & 1 deletion docs/storybook/stories/Demos/BorderDemo.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const Borders: ComponentStory<typeof ColorPreview> = () => {
<ActionList.Item variant="danger">Danger</ActionList.Item>
</ActionList>
</div>
<CSSTokenSwatch color="borderColor-decorative" />
<CSSTokenSwatch color="borderColor-muted" />
</div>
<div style={{display: 'flex', flexDirection: 'column', gap: '1rem'}}>
<div className="ActionList alt" style={{display: 'flex', gap: '1rem'}}>
Expand Down
4 changes: 2 additions & 2 deletions src/platforms/css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ const getCssSelectors = (outputFile: string): {selector: string; selectorLight:
const lastSlash = outputFile.lastIndexOf('/')
const outputBasename = outputFile.substring(lastSlash + 1, outputFile.indexOf('.'))
const themeName = outputBasename.replace(/-/g, '_')
const mode = outputBasename.substring(0, 4) === 'dark' ? 'dark' : 'light'
// const mode = outputBasename.substring(0, 4) === 'dark' ? 'dark' : 'light'

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}"]`,
Copy link
Collaborator

@lukasoppermann lukasoppermann Mar 27, 2023

Choose a reason for hiding this comment

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

Why are you changing this?

Copy link
Contributor Author

@langermank langermank Mar 27, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

@lukasoppermann lukasoppermann Mar 28, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Member

@jonrohan jonrohan Mar 28, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@langermank langermank Mar 28, 2023

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@colebemis colebemis Mar 29, 2023

Choose a reason for hiding this comment

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

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

selectorLight: `[data-color-mode="auto"][data-light-theme="${themeName}"]`,
selectorDark: `[data-color-mode="auto"][data-dark-theme="${themeName}"]`,
}
Expand Down
2 changes: 1 addition & 1 deletion src/tokens/base/color/light/light.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
base: {
color: {
black: {
$value: '#1b1f24',
$value: '#1f2328',
$type: 'color',
},
transparent: {
Expand Down
2 changes: 1 addition & 1 deletion src/tokens/functional/border/dark.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
outline: {
focus: {
$value: {
color: '{color.focus}',
color: '{focus.outlineColor}',
style: 'solid',
width: '1px',
},
Expand Down
6 changes: 3 additions & 3 deletions src/tokens/functional/border/light.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
outline: {
focus: {
focus: {
outline: {
$value: {
color: '{color.focus}',
color: '{focus.outlineColor}',
style: 'solid',
width: '1px',
},
Expand Down
74 changes: 0 additions & 74 deletions src/tokens/functional/color/dark/ansi-dark.json5
View file
Open in desktop

This file was deleted.

242 changes: 242 additions & 0 deletions src/tokens/functional/color/dark/app-dark.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
{
topicTag: {
borderColor: {
$value: '{base.color.transparent}',
$type: 'color',
},
},
highlight: {
neutral: {
bgColor: {
$value: '{base.color.yellow.3}',
$type: 'color',
alpha: 0.4,
},
},
},
page: {
header: {
bgColor: {
$value: '{bgColor.default}',
$type: 'color',
},
},
},
diffBlob: {
addition: {
fgColor: {
text: {
$value: '{fgColor.default}',
$type: 'color',
},
num: {
$value: '{fgColor.default}',
$type: 'color',
},
},
bgColor: {
num: {
$value: '{base.color.green.3}',
$type: 'color',
alpha: 0.3,
},
line: {
$value: '{base.color.green.4}',
$type: 'color',
alpha: 0.15,
mix: null,
},
word: {
$value: '{base.color.green.4}',
$type: 'color',
alpha: 0.4,
},
},
},
deletion: {
fgColor: {
text: {
$value: '{fgColor.default}',
$type: 'color',
},
num: {
$value: '{fgColor.default}',
$type: 'color',
},
},
bgColor: {
num: {
$value: '{base.color.red.4}',
$type: 'color',
alpha: 0.3,
},
line: {
$value: '{base.color.red.4}',
$type: 'color',
alpha: 0.15,
mix: null,
},
word: {
$value: '{base.color.red.4}',
$type: 'color',
alpha: 0.4,
},
},
},
hunk: {
bgColor: {
num: {
$value: '{borderColor.accent.muted}',
$type: 'color',
},
},
},
expander: {
iconColor: {
$value: '{fgColor.muted}',
$type: 'color',
},
},
},
diffStat: {
addition: {
bgColor: {
$value: '{bgColor.success.emphasis}',
$type: 'color',
},
},
},
codeMirror: {
fgColor: {
$value: '{fgColor.default}',
$type: 'color',
},
bgColor: {
$value: '{bgColor.default}',
$type: 'color',
},
gutters: {
bgColor: {
$value: '{bgColor.default}',
$type: 'color',
},
},
gutterMarker: {
fgColor: {
default: {
$value: '{bgColor.default}',
$type: 'color',
},
muted: {
$value: '{fgColor.muted}',
$type: 'color',
},
},
},
lineNumber: {
fgColor: {
$value: '{fgColor.muted}',
$type: 'color',
},
},
cursor: {
fgColor: {
$value: '{fgColor.default}',
$type: 'color',
},
},
selection: {
bgColor: {
$value: '{borderColor.accent.muted}',
$type: 'color',
},
},
activeline: {
bgColor: {
$value: '{bgColor.neutral.muted}',
$type: 'color',
},
},
matchingBracket: {
fgColor: {
$value: '{fgColor.default}',
$type: 'color',
},
},
lines: {
bgColor: {
$value: '{bgColor.default}',
$type: 'color',
},
},
syntax: {
fgColor: {
comment: {
$value: '{base.color.gray.3}',
$type: 'color',
},
constant: {
$value: '{base.color.blue.2}',
$type: 'color',
},
entity: {
$value: '{base.color.purple.2}',
$type: 'color',
},
keyword: {
$value: '{base.color.red.3}',
$type: 'color',
},
storage: {
$value: '{base.color.red.3}',
$type: 'color',
},
string: {
$value: '{base.color.blue.1}',
$type: 'color',
},
support: {
$value: '{base.color.blue.2}',
$type: 'color',
},
variable: {
$value: '{base.color.orange.2}',
$type: 'color',
},
},
},
},
header: {
fgColor: {
default: {
$value: '{fgColor.onEmphasis}',
$type: 'color',
alpha: 0.7,
},
logo: {
$value: '{fgColor.onEmphasis}',
$type: 'color',
},
},
bgColor: {
$value: '{base.color.gray.9}',
$type: 'color',
},
borderColor: {
divider: {
$value: '{base.color.gray.3}',
$type: 'color',
},
},
},
headerSearch: {
bgColor: {
$value: '{base.color.gray.9}',
$type: 'color',
},
borderColor: {
$value: '{base.color.gray.6}',
$type: 'color',
},
},
}
Loading