-
Notifications
You must be signed in to change notification settings - Fork 536
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
Sync Label component with PVC's Label component #1797
Changes from all commits
5c9018a
6a66da2
fae2072
73e2911
7c5bcc0
96aa767
e94b1ec
2e9587b
4754dd1
0863146
7bfc14b
8c903c5
cbf7dc6
cb8469e
1974be8
9ad5680
ebb8756
106a854
9ff1905
590c7a2
f494349
0e6490b
f015e1f
a9af0ba
83ddc6d
72084d4
7b69c5f
8fe7f81
6286cf0
9510286
659e6c8
4f612a5
08b3e11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@primer/react': minor | ||
--- | ||
|
||
Introduces a draft for component to replace the existing Label component |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
--- | ||
title: Label v2 | ||
componentId: label2 | ||
status: Alpha | ||
source: https://github.com/primer/react/tree/main/src/Label2 | ||
storybook: '/react/storybook?path=story/labels-label--label' | ||
description: Use Label components to add contextual metadata to a design. | ||
--- | ||
|
||
## Examples | ||
|
||
### Basic | ||
|
||
```javascript live noinline | ||
// import {Label} from '@primer/react/drafts' | ||
const {Label} = drafts // ignore docs silliness; import like that ↑ | ||
|
||
render(<Label>Default</Label>) | ||
``` | ||
|
||
### Variants | ||
|
||
```javascript live noinline | ||
// import {Label} from '@primer/react/drafts' | ||
const {Label} = drafts // ignore docs silliness; import like that ↑ | ||
render( | ||
<> | ||
<Label>Default</Label> | ||
<Label variant="primary">Primary</Label> | ||
<Label variant="secondary">Secondary</Label> | ||
<Label variant="accent">Accent</Label> | ||
<Label variant="success">Success</Label> | ||
<Label variant="attention">Attention</Label> | ||
<Label variant="severe">Severe</Label> | ||
<Label variant="danger">Danger</Label> | ||
<Label variant="done">Done</Label> | ||
<Label variant="sponsors">Sponsors</Label> | ||
</> | ||
) | ||
``` | ||
|
||
### Sizes | ||
|
||
```javascript live noinline | ||
// import {Label} from '@primer/react/drafts' | ||
const {Label} = drafts // ignore docs silliness; import like that ↑ | ||
render( | ||
<> | ||
<Label>Small (default)</Label> | ||
<Label size="large">Large</Label> | ||
</> | ||
) | ||
``` | ||
|
||
## Props | ||
|
||
### Label | ||
|
||
<PropsTable> | ||
<PropsTableRow | ||
name="variant" | ||
type={`| 'default' | ||
| 'primary' | ||
| 'secondary' | ||
| 'accent' | ||
| 'success' | ||
| 'attention' | ||
| 'severe' | ||
| 'danger' | ||
| 'done' | ||
| 'sponsors'`} | ||
defaultValue="'default'" | ||
description="The color of the label" | ||
/> | ||
<PropsTableRow | ||
name="size" | ||
type="'small' | 'large'" | ||
defaultValue="'small'" | ||
description="How large the label is rendered" | ||
/> | ||
</PropsTable> | ||
|
||
## Status | ||
|
||
<ComponentChecklist | ||
items={{ | ||
propsDocumented: true, | ||
noUnnecessaryDeps: true, | ||
adaptsToThemes: true, | ||
adaptsToScreenSizes: true, | ||
fullTestCoverage: true, | ||
usedInProduction: false, | ||
usageExamplesDocumented: true, | ||
hasStorybookStories: true, | ||
designReviewed: false, | ||
a11yReviewed: false, | ||
stableApi: false, | ||
addressedApiFeedback: false, | ||
hasDesignGuidelines: false, | ||
hasFigmaComponent: false | ||
}} | ||
/> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import styled from 'styled-components' | ||
import {variant} from 'styled-system' | ||
import sx, {SxProp, BetterSystemStyleObject} from './sx' | ||
import {get} from './constants' | ||
|
||
export type LabelProps = { | ||
/** The color of the label */ | ||
variant?: LabelColorOptions | ||
/** How large the label is rendered */ | ||
size?: LabelSizeKeys | ||
} & SxProp | ||
|
||
export type LabelColorOptions = | ||
| 'default' | ||
| 'primary' | ||
| 'secondary' | ||
| 'accent' | ||
| 'success' | ||
| 'attention' | ||
| 'severe' | ||
| 'danger' | ||
| 'done' | ||
| 'sponsors' | ||
|
||
type LabelSizeKeys = 'small' | 'large' | ||
|
||
export const variants: Record<LabelColorOptions, BetterSystemStyleObject> = { | ||
default: { | ||
borderColor: 'border.default' | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we can use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe something like this: import styled from 'styled-components'
import {sx, SxProp} from './sx'
import { variant, SystemStyleObject } from 'styled-system'
const variants = {
default: {
borderColor: 'border.default'
},
primary: {
borderColor: 'fg.default'
},
...
} as const
const sizes = {
small: {
height: '20px',
padding: '7px',
},
large: ...
} as const
export type LabelProps = {
variant: keyof typeof variants
size: keyof typeof sizes
} & SxProp
const Label = styled.span<LabelProps>`
...styles...
${variant({ variants })};
${variant({ prop: 'size', variants: sizes })};
${sx};
`
Label.defaultProps = {
variant: 'default',
size: 'small',
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah - yea, this feels cleaner. I'll give it a shot 👍 |
||
primary: { | ||
borderColor: 'fg.default' | ||
}, | ||
secondary: { | ||
borderColor: 'border.muted', | ||
color: 'fg.muted' | ||
}, | ||
accent: { | ||
borderColor: 'accent.emphasis', | ||
color: 'accent.fg' | ||
}, | ||
success: { | ||
borderColor: 'success.emphasis', | ||
color: 'success.fg' | ||
}, | ||
attention: { | ||
borderColor: 'attention.emphasis', | ||
color: 'attention.fg' | ||
}, | ||
severe: { | ||
borderColor: 'severe.emphasis', | ||
color: 'severe.fg' | ||
}, | ||
danger: { | ||
borderColor: 'danger.emphasis', | ||
color: 'danger.fg' | ||
}, | ||
done: { | ||
borderColor: 'done.fg', | ||
color: 'done.emphasis' | ||
}, | ||
sponsors: { | ||
borderColor: 'sponsors.fg', | ||
color: 'sponsors.emphasis' | ||
} | ||
} | ||
|
||
const sizes: Record<LabelSizeKeys, BetterSystemStyleObject> = { | ||
small: { | ||
height: '20px', | ||
padding: '0 7px' // hard-coded to align with Primer ViewCompnents and Primer CSS | ||
}, | ||
large: { | ||
height: '24px', | ||
padding: '0 10px' // hard-coded to align with Primer ViewCompnents and Primer CSS | ||
} | ||
} | ||
|
||
export const Label = styled.span<LabelProps>` | ||
align-items: center; | ||
background-color: transparent; | ||
border-width: 1px; | ||
border-radius: 999px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use relative units like PVC too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The border-radius isn't relative to anything - it's just a really high number to make it perfectly round. border-width is also not relative to anything, it's just 1px. |
||
border-style: solid; | ||
display: inline-flex; | ||
font-weight: ${get('fontWeights.bold')}; | ||
font-size: ${get('fontSizes.0')}; | ||
line-height: 1; | ||
white-space: nowrap; | ||
${variant({variants})}; | ||
${variant({prop: 'size', variants: sizes})}; | ||
${sx}; | ||
` | ||
|
||
Label.defaultProps = { | ||
size: 'small', | ||
variant: 'default' | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import React from 'react' | ||
import {render, cleanup} from '@testing-library/react' | ||
import {axe, toHaveNoViolations} from 'jest-axe' | ||
import 'babel-polyfill' | ||
import {Label, variants, LabelColorOptions} from '../Label2' | ||
import {renderStyles} from '../utils/testing' | ||
expect.extend(toHaveNoViolations) | ||
|
||
describe('Label2', () => { | ||
it('renders text node child', () => { | ||
const container = render(<Label>Default</Label>) | ||
const label = container.baseElement | ||
expect(label.textContent).toEqual('Default') | ||
}) | ||
it('default size is rendered as "small"', () => { | ||
const expectedStyles = { | ||
height: '20px', | ||
padding: '0 7px' | ||
} | ||
const defaultStyles = renderStyles(<Label />) | ||
|
||
expect(defaultStyles).toEqual(expect.objectContaining(expectedStyles)) | ||
}) | ||
it('default variant is rendered as "default"', () => { | ||
const expectedStyles = { | ||
['border-color']: '#d0d7de' | ||
} | ||
const defaultStyles = renderStyles(<Label />) | ||
|
||
expect(defaultStyles).toEqual(expect.objectContaining(expectedStyles)) | ||
}) | ||
it('should have no axe violations', async () => { | ||
for (const variant in variants) { | ||
const {container} = render(<Label variant={variant as LabelColorOptions}>Default</Label>) | ||
const results = await axe(container) | ||
expect(results).toHaveNoViolations() | ||
cleanup() | ||
} | ||
}) | ||
}) | ||
mperrotti marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
import React from 'react' | ||
import {Meta} from '@storybook/react' | ||
import {BaseStyles, ThemeProvider} from '..' | ||
import {ComponentProps} from '../utils/types' | ||
import {Label} from '../Label2' | ||
|
||
type Args = ComponentProps<typeof Label> | ||
|
||
export default { | ||
// TODO: update story nesting | ||
title: 'Labels/Label', | ||
component: Label, | ||
argTypes: { | ||
variant: { | ||
defaultValue: 'default', | ||
control: { | ||
options: [ | ||
'default', | ||
'primary', | ||
'secondary', | ||
'accent', | ||
'success', | ||
'attention', | ||
'severe', | ||
'danger', | ||
'done', | ||
'sponsors' | ||
], | ||
type: 'select' | ||
} | ||
}, | ||
size: { | ||
defaultValue: 'large', | ||
control: { | ||
options: ['small', 'large'], | ||
type: 'radio' | ||
} | ||
} | ||
}, | ||
decorators: [ | ||
Story => { | ||
return ( | ||
<ThemeProvider> | ||
<BaseStyles> | ||
<Story /> | ||
</BaseStyles> | ||
</ThemeProvider> | ||
) | ||
} | ||
] | ||
} as Meta | ||
|
||
export const label = (args: Args) => <Label {...args}>Label</Label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove these once #1785 is merged