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

Sync Label component with PVC's Label component #1797

Merged
merged 33 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
5c9018a
introduces new label component
mperrotti Nov 3, 2021
6a66da2
updates storybook stories and adds React docs for Label components
mperrotti Jan 12, 2022
fae2072
adds tests
mperrotti Jan 12, 2022
73e2911
uses hacky fix for rendering the correct Label component in the docs
mperrotti Jan 12, 2022
7c5bcc0
reduces scope of new label components
mperrotti Jan 13, 2022
96aa767
rm StateLabel2
mperrotti Jan 13, 2022
e94b1ec
fix React docs
mperrotti Jan 13, 2022
2e9587b
syncs Label styling with PVC/PCSS
mperrotti Jan 13, 2022
4754dd1
adds a changeset
mperrotti Jan 14, 2022
0863146
renames 'scheme' prop to 'variant'
mperrotti Jan 14, 2022
7bfc14b
Update src/Label2.tsx
mperrotti Jan 14, 2022
8c903c5
Update src/Label2.tsx
mperrotti Jan 14, 2022
cbf7dc6
Merge branch 'main' into mp/label-sync-with-pvc
mperrotti Jan 14, 2022
cb8469e
Merge github.com:primer/react into mp/label-sync-with-pvc
mperrotti Jan 14, 2022
1974be8
addresses more PR feedback
mperrotti Jan 14, 2022
9ad5680
Merge branch 'mp/label-sync-with-pvc' of github.com:primer/react into…
mperrotti Jan 14, 2022
ebb8756
Update docs/content/drafts/Label2.mdx
mperrotti Jan 14, 2022
106a854
Update docs/content/drafts/Label2.mdx
mperrotti Jan 21, 2022
9ff1905
Update src/Label2.tsx
mperrotti Jan 21, 2022
590c7a2
Update src/Label2.tsx
mperrotti Jan 21, 2022
f494349
does not export Label as the default export of Label2.tsx
mperrotti Jan 21, 2022
0e6490b
Merge branch 'mp/label-sync-with-pvc' of github.com:primer/react into…
mperrotti Jan 21, 2022
f015e1f
Update docs/content/drafts/Label2.mdx
mperrotti Jan 21, 2022
a9af0ba
Merge branch 'main' into mp/label-sync-with-pvc
mperrotti Jan 21, 2022
83ddc6d
fix imports
mperrotti Jan 21, 2022
72084d4
Merge branch 'mp/label-sync-with-pvc' of github.com:primer/react into…
mperrotti Jan 21, 2022
7b69c5f
Update src/Label2.tsx
mperrotti Jan 24, 2022
8fe7f81
Update docs/content/drafts/Label2.mdx
mperrotti Jan 24, 2022
6286cf0
Update docs/content/drafts/Label2.mdx
mperrotti Jan 24, 2022
9510286
addresses more PR feedback
mperrotti Jan 24, 2022
659e6c8
Merge branch 'main' into mp/label-sync-with-pvc
mperrotti Jan 25, 2022
4f612a5
revert package-lock to main
mperrotti Jan 25, 2022
08b3e11
Merge branch 'mp/label-sync-with-pvc' of github.com:primer/react into…
mperrotti Jan 25, 2022
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/stale-badgers-search.md
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
102 changes: 102 additions & 0 deletions docs/content/drafts/Label2.mdx
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 ↑
Copy link
Contributor Author

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

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
}}
/>
98 changes: 98 additions & 0 deletions src/Label2.tsx
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'
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can use the variants utility from styled-system to clean up all the get() calls here.

Copy link
Contributor

Choose a reason for hiding this comment

The 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',
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use relative units like PVC too? 2em

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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'
}
40 changes: 40 additions & 0 deletions src/__tests__/Label2.test.tsx
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
1 change: 1 addition & 0 deletions src/drafts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ export * from './ActionList2'
export * from './Button2'
export * from './ActionMenu2'
export * from './DropdownMenu2'
export * from './Label2'
53 changes: 53 additions & 0 deletions src/stories/Label2.stories.tsx
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>