Sync Label component with PVC's Label component

[mperrotti]

Instead of introducing a breaking change to the existing Label component, I'm proposing that we take the same path we did with ActionList, Button, and others: introduce a "draft" component.

Screenshots

Merge checklist

• Added/updated tests
• Added/updated documentation
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

Reference

Issue: https://github.com/github/primer/issues/568
PVC's Label component: https://primer.style/view-components/components/label

[changeset-bot]

🦋 Changeset detected

Latest commit: 08b3e11

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

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

[mperrotti]

We can remove these once #1785 is merged

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	61.25 KB (0%)
dist/browser.umd.js	61.63 KB (0%)

[colebemis]

Looks like this prop is called scheme in the code.

Also, I'm curious if we should call this prop variant to be consistent with the Button API: https://primer.style/react/drafts/Button2#button

[mperrotti]

Yup, this is just a mistake. Good catch!

Agreed on changing to variant too. I was just copying the PVC API.

[colebemis]

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

[colebemis]

Maybe something like this:

importstyledfrom'styled-components'import{sx,SxProp}from'./sx'import{variant,SystemStyleObject}from'styled-system'constvariants={default: {borderColor: 'border.default'},primary: {borderColor: 'fg.default'},
...
}asconstconstsizes={small: {height: '20px',padding: '7px',},large: ...
}asconstexporttypeLabelProps={variant: keyoftypeofvariantssize: keyoftypeofsizes}&SxPropconstLabel=styled.span<LabelProps>` ...styles...${variant({ variants })};${variant({prop: 'size',variants: sizes})};${sx};`Label.defaultProps={variant: 'default',size: 'small',}

[mperrotti]

Ah - yea, this feels cleaner. I'll give it a shot 👍

[colebemis]

It seems like this is equivalent to:

const Label: React.FC<Props> = props => <LabelContainer {...props} />

Is there a reason we need to have this proxy Label component? What if we exported the styled component directly?

[mperrotti]

This is leftover from a more complex implementation where we handled a leadingVisual. We can just use the styled component directly.

Good catch.

[colebemis]

Is there a reason this export doesn't follow the same pattern as the others (i.e export * from './Label2')?

[mperrotti]

It was because Label was the default export. I changed it.

[rezrah]

Should we use relative units like PVC too? 2em

[mperrotti]

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.

[rezrah]

✅

Was the package-lock change intentional?

[mperrotti]

@rezrah - the package-lock change was not intentional. I'll just pull in whatever is on main