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

Implement functional color variables part 3 [CounterLabel, Dialog, Dropdown, DropdownStyles, FilterList] #1099

Merged
merged 14 commits into from
Mar 4, 2021
Merged
5 changes: 5 additions & 0 deletions .changeset/famous-guests-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

Use functional color variables in FilterList
5 changes: 5 additions & 0 deletions .changeset/tall-cougars-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

Use functional color variables in DropdownStyles
5 changes: 5 additions & 0 deletions .changeset/twelve-suits-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

Use functional color variables in CounterLabel
5 changes: 5 additions & 0 deletions .changeset/wild-lies-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

Use functional color variables in Dialog
10 changes: 5 additions & 5 deletions src/CounterLabel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ const colorStyles = ({scheme, ...props}: StyledCounterLabelProps) => {
return {
color:
scheme === 'gray-light'
? get('colors.gray.9')(props)
? get('colors.counter.text')(props)
: scheme === 'gray'
? get('colors.white')(props)
: get('colors.gray.6')(props)
? get('colors.counter.primary.text')(props)
: get('colors.counter.text')(props)
}
}

const bgStyles = ({scheme, ...props}: StyledCounterLabelProps) => {
return {
backgroundColor:
scheme === 'gray-light'
? get('colors.blackfade15')(props)
? get('colors.counter.bg')(props)
: scheme === 'gray'
? get('colors.gray.5')(props)
? get('colors.counter.primary.bg')(props)
: get('colors.counter.bg')(props)
}
}
Expand Down
16 changes: 10 additions & 6 deletions src/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type StyledDialogBaseProps = {
SxProp

const DialogBase = styled.div<StyledDialogBaseProps>`
box-shadow: 0px 4px 32px rgba(0, 0, 0, 0.35);
box-shadow: ${get('shadows.shadow.large')};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

couldn't find a primer/css file for dialog, so I'm guessing at some of these. Dark mode doesn't seem quite right:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

border-radius: ${get('radii.2')};
position: fixed;
top: 0;
Expand All @@ -27,7 +27,7 @@ const DialogBase = styled.div<StyledDialogBaseProps>`
max-height: 80vh;
z-index: 999;
margin: 10vh auto;
background-color: ${get('colors.white')};
background-color: ${get('colors.bg.canvas')};
width: ${props => (props.narrow ? '320px' : props.wide ? '640px' : '440px')};
outline: none;

Expand All @@ -45,7 +45,7 @@ const DialogBase = styled.div<StyledDialogBaseProps>`

const DialogHeaderBase = styled(Flex)<SxProp>`
border-radius: 4px 4px 0px 0px;
colebemis marked this conversation as resolved.
Show resolved Hide resolved
border-bottom: 1px solid #dad5da;
border-bottom: 1px solid ${get('colors.border.primary')};

@media screen and (max-width: 750px) {
border-radius: 0px;
Expand All @@ -58,7 +58,7 @@ export type DialogHeaderProps = ComponentProps<typeof DialogHeaderBase>
function DialogHeader({theme, children, backgroundColor = 'gray.1', ...rest}: DialogHeaderProps) {
if (React.Children.toArray(children).every(ch => typeof ch === 'string')) {
children = (
<Text theme={theme} color="gray.9" fontSize={1} fontWeight="bold" fontFamily="sans-serif">
<Text theme={theme} color="text.primary" fontSize={1} fontWeight="bold" fontFamily="sans-serif">
{children}
</Text>
)
Expand All @@ -84,7 +84,7 @@ const Overlay = styled.span`
content: ' ';
background: transparent;
z-index: 99;
background: rgba(27, 31, 35, 0.5);
background: ${get('colors.fade.fg50')};
colebemis marked this conversation as resolved.
Show resolved Hide resolved
}
`

Expand Down Expand Up @@ -139,7 +139,11 @@ const Dialog = forwardRef<HTMLElement, InternalDialogProps>(
)

DialogHeader.defaultProps = {
backgroundColor: 'gray.1'
backgroundColor: 'bg.canvasInset'
}

DialogHeader.propTypes = {
...Flex.propTypes
}

DialogHeader.displayName = 'Dialog.Header'
Expand Down
22 changes: 11 additions & 11 deletions src/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ type StyledDropdownMenuProps = {

const DropdownMenu = styled.ul<StyledDropdownMenuProps>`
background-clip: padding-box;
background-color: ${get('colors.white')};
border: 1px solid rgba(27, 31, 35, 0.15);
background-color: ${get('colors.bg.overlay')};
border: 1px solid ${get('colors.border.overlay')};
border-radius: ${get('radii.2')};
box-shadow: 0 3px 12px rgba(27, 31, 35, 0.15);
box-shadow: ${get('shadows.dropdown.shadow')};
left: 0;
list-style: none;
margin-top: 2px;
Expand All @@ -83,12 +83,12 @@ const DropdownMenu = styled.ul<StyledDropdownMenuProps>`

&::before {
border: 8px solid transparent;
border-bottom-color: ${get('colors.blackfade15')};
border-bottom-color: ${get('colors.bg.overlay')};
}

&::after {
border: 7px solid transparent;
border-bottom-color: ${get('colors.white')};
border-bottom-color: ${get('colors.bg.overlay')};
}

// stylelint-disable-next-line selector-max-type
Expand All @@ -104,11 +104,11 @@ const DropdownItem = styled.li`
display: block;
padding: ${get('space.1')} 10px ${get('space.1')} 15px;
overflow: hidden;
color: ${get('colors.gray.9')};
color: ${get('colors.text.primary')};
text-overflow: ellipsis;
white-space: nowrap;
a {
color: ${get('colors.gray.9')};
color: ${get('colors.text.primary')};
text-decoration: none;
display: block;
overflow: hidden;
Expand All @@ -119,16 +119,16 @@ const DropdownItem = styled.li`

&:focus,
a:focus {
color: ${get('colors.white')};
color: ${get('colors.state.hover.primaryText')};
text-decoration: none;
background-color: ${get('colors.blue.5')};
background-color: ${get('colors.state.hover.primaryBg')};
}

&:hover,
&:hover a {
color: ${get('colors.white')};
color: ${get('colors.state.hover.primaryText')};
text-decoration: none;
background-color: ${get('colors.blue.5')};
background-color: ${get('colors.state.hover.primaryBg')};
outline: none;
}
${COMMON};
Expand Down
12 changes: 6 additions & 6 deletions src/DropdownStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ const getDirectionStyles = (theme: any, direction: 'ne' | 'e' | 'se' | 's' | 'sw
right: -16px;
left: auto;
border-color: transparent;
border-left-color: ${get('colors.blackfade15')(theme)};
border-left-color: ${get('colors.border.overlay')(theme)};
}

&::after {
top: 11px;
right: -14px;
left: auto;
border-color: transparent;
border-left-color: ${get('colors.white')(theme)};
border-left-color: ${get('colors.border.overlay')(theme)};
}
`,
e: `
Expand All @@ -37,14 +37,14 @@ const getDirectionStyles = (theme: any, direction: 'ne' | 'e' | 'se' | 's' | 'sw
top: 10px;
left: -16px;
border-color: transparent;
border-right-color: ${get('colors.blackfade15')(theme)};
border-right-color: ${get('colors.border.overlay')(theme)};
}

&::after {
top: 11px;
left: -14px;
border-color: transparent;
border-right-color: ${get('colors.white')(theme)};
border-right-color: ${get('colors.border.overlay')(theme)};
}
`,
ne: `
Expand All @@ -62,15 +62,15 @@ const getDirectionStyles = (theme: any, direction: 'ne' | 'e' | 'se' | 's' | 'sw
&::before {
bottom: -8px;
left: 9px;
border-top: 8px solid ${get('colors.blackfade15')(theme)};
border-top: 8px solid ${get('colors.border.overlay')(theme)};
border-bottom: 0;
border-left: 8px solid transparent;
}

&::after {
bottom: -7px;
left: 10px;
border-top: 7px solid ${get('colors.white')(theme)};
border-top: 7px solid ${get('colors.border.overlay')(theme)};
border-right: 7px solid transparent;
border-bottom: 0;
border-left: 7px solid transparent;
Expand Down
10 changes: 5 additions & 5 deletions src/FilterList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,20 @@ const FilterListItemBase = styled.a<StyledFilterListItemBaseProps>`
margin: ${props => (props.small ? '0 0 2px' : '0 0 5px 0')};
overflow: hidden;
font-size: ${get('fontSizes.1')};
color: ${props => (props.selected ? get('colors.white') : get('colors.gray.6'))};
background-color: ${props => (props.selected ? get('colors.blue.5') : '')}!important;
color: ${props => (props.selected ? get('colors.state.selected.primaryText') : get('colors.text.secondary'))};
background-color: ${props => (props.selected ? get('colors.state.selected.primaryBg') : '')}!important;
text-decoration: none;
text-overflow: ellipsis;
white-space: nowrap;
cursor: pointer;
border-radius: ${get('radii.1')};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border-radius: ${get('radii.1')};
border-radius: ${get('radii.2')};

&:hover {
text-decoration: none;
background-color: ${get('colors.filterList.hoverBg')};
background-color: ${get('colors.bg.tertiary')};
}
&:active {
color: ${get('colors.white')};
background-color: ${get('colors.blue.5')};
color: ${get('colors.state.selected.primaryText')};
background-color: ${get('colors.state.selected.primaryBg')};
}
.count {
float: right;
Expand Down
20 changes: 16 additions & 4 deletions src/__tests__/CounterLabel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {colors} from '../theme'
import {render as HTMLRender, cleanup} from '@testing-library/react'
import {axe, toHaveNoViolations} from 'jest-axe'
import 'babel-polyfill'
import {default as primitives} from '@primer/primitives'
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of importing from primitives we can import the theme:

- import {default as primitives} from '@primer/primitives'
+ import theme from '../theme'

That might make values easier to reference in the tests


expect.extend(toHaveNoViolations)

describe('CounterLabel', () => {
Expand All @@ -27,15 +29,25 @@ describe('CounterLabel', () => {
})

it('respects the gray "scheme" prop', () => {
expect(render(<CounterLabel scheme="gray" />)).toHaveStyleRule('color', colors.white.trim())
expect(render(<CounterLabel scheme="gray" />)).toHaveStyleRule('background-color', colors.gray[5].trim())
const p = primitives
expect(render(<CounterLabel scheme="gray" />)).toHaveStyleRule(
'color',
primitives.colors.light.counter.primary.text.trim()
)
expect(render(<CounterLabel scheme="gray" />)).toHaveStyleRule(
'background-color',
primitives.colors.light.counter.primary.bg.trim()
)
})

it('respects the gray-light "scheme" prop', () => {
expect(render(<CounterLabel scheme="gray-light" />)).toHaveStyleRule('color', colors.gray[9].trim())
expect(render(<CounterLabel scheme="gray-light" />)).toHaveStyleRule(
'color',
primitives.colors.light.counter.text.trim()
)
expect(render(<CounterLabel scheme="gray-light" />)).toHaveStyleRule(
'background-color',
colors.blackfade15.replace(/\s/g, '')
primitives.colors.light.counter.bg
)
})
})
2 changes: 1 addition & 1 deletion src/__tests__/__snapshots__/CounterLabel.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ exports[`CounterLabel renders consistently 1`] = `
font-weight: 600;
line-height: 1;
border-radius: 20px;
color: #586069;
color: #24292e;
background-color: rgba(209,213,218,0.5);
}

Expand Down
10 changes: 5 additions & 5 deletions src/__tests__/__snapshots__/Dialog.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ exports[`Dialog Dialog.Header renders consistently 1`] = `
display: -ms-flexbox;
display: flex;
border-radius: 4px 4px 0px 0px;
border-bottom: 1px solid #dad5da;
border-bottom: 1px solid #e1e4e8;
}

@media screen and (max-width:750px) {
Expand All @@ -35,7 +35,7 @@ exports[`Dialog Dialog.Header renders consistently 1`] = `
>
<span
className="c1"
color="gray.9"
color="text.primary"
fontFamily="sans-serif"
fontSize={1}
fontWeight="bold"
Expand Down Expand Up @@ -99,7 +99,7 @@ Array [
}

.c0 {
box-shadow: 0px 4px 32px rgba(0,0,0,0.35);
box-shadow: 0 8px 24px rgba(149,157,165,0.2);
border-radius: 6px;
position: fixed;
top: 0;
Expand Down Expand Up @@ -127,7 +127,7 @@ Array [
display: -ms-flexbox;
display: flex;
border-radius: 4px 4px 0px 0px;
border-bottom: 1px solid #dad5da;
border-bottom: 1px solid #e1e4e8;
}

@media screen and (max-width:750px) {
Expand Down Expand Up @@ -187,7 +187,7 @@ Array [
>
<span
className="c3"
color="gray.9"
color="text.primary"
fontFamily="sans-serif"
fontSize={1}
fontWeight="bold"
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/__snapshots__/Dropdown.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ exports[`Dropdown.Menu renders consistently 1`] = `
.c0 {
background-clip: padding-box;
background-color: #ffffff;
border: 1px solid rgba(27,31,35,0.15);
border: 1px solid #e1e4e8;
border-radius: 6px;
box-shadow: 0 3px 12px rgba(27,31,35,0.15);
box-shadow: 0 8px 24px rgba(149,157,165,0.2);
left: 0;
list-style: none;
margin-top: 2px;
Expand All @@ -209,7 +209,7 @@ exports[`Dropdown.Menu renders consistently 1`] = `

.c0::before {
border: 8px solid transparent;
border-bottom-color: rgba(27,31,35,0.15);
border-bottom-color: #ffffff;
}

.c0::after {
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/__snapshots__/FilterListItem.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ exports[`FilterList.Item renders consistently 1`] = `
.c0:hover {
-webkit-text-decoration: none;
text-decoration: none;
background-color: #eaecef;
background-color: #f6f8fa;
}

.c0:active {
Expand Down Expand Up @@ -60,7 +60,7 @@ exports[`FilterList.Item respects the "selected" prop 1`] = `
.c0:hover {
-webkit-text-decoration: none;
text-decoration: none;
background-color: #eaecef;
background-color: #f6f8fa;
}

.c0:active {
Expand Down