Badge component #3429

devongovett merged 14 commits into main from badge-component
jluyau
jluyau2 years ago (edited 2 years ago)๐ŸŽ‰ 1โค 1

#1341 - Add badge component

  • currently only S size
  • supports icon only, text only, icon + text
  • supports all variants defined in design doc
  • disabled styling

To add in future alphas:

  • other sizes (M, L, XL)
  • fixed styling (API?)

โœ… Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

๐Ÿ“ Test Instructions:

๐Ÿงข Your Project:

Adobe/Quarry

jluyau Badge component - alpha.1
2fb545e4
jluyau jluyau added rsp:Badge
jluyau change VersionBadge to use Badge
c08b7862
jluyau lint fixes
36e1690f
snowystinger
snowystinger commented on 2022-08-23
snowystinger2 years ago

Screen Shot 2022-08-23 at 9 44 13 AM

doesn't quite look right?

Conversation is marked as resolved
Show resolved
packages/@adobe/spectrum-css-temp/components/badge/index.css
36 padding-inline-end: var(--spectrum-global-dimension-size-115);
37 }
38
39
.spectrum-Icon {
snowystinger2 years ago๐Ÿ‘ 1
Suggested change
.spectrum-Icon {
.spectrum-Badge-icon {

we had a lot of issues when we first started using css-modules because this classname exists everywhere and it can't be shared across CSS modules. So lets name it in a way that indicates that more readily.

Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/chromatic/Badge.chromatic.tsx
20
21let variants = ['yellow', 'fuchsia', 'indigo', 'seafoam', 'magenta', 'purple', 'neutral', 'info', 'positive', 'negative'];
22
23
storiesOf('Badge', module)
snowystinger2 years ago๐Ÿ‘ 1

please write in the CSF 3.0 format, see #3129

Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/package.json
37 "@react-spectrum/utils": "^3.7.2",
38 "@react-types/badge": "^3.0.0",
39 "@react-types/shared": "^3.14.0",
40
"@react-types/statuslight": "^3.2.2"
snowystinger2 years ago

copy pasta

jluyau2 years ago

damn thought i got all of them

Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/src/Badge.tsx
39 className={classNames(
40 styles,
41 'spectrum-Badge',
42
'spectrum-Badge--fixed-top',
snowystinger2 years ago

not a real classname? i think at one point we called them anchorTop this should be fixedTop if that's what it's meant to be

jluyau2 years ago

yeah i had the fixed prop in there but removed it in favor of a later version. forgot to remove this from the code

Conversation is marked as resolved
Show resolved
packages/dev/docs/src/VersionBadge.js
3542 }
3643
44 if (size === 'S') {
45
return <Badge variant={versionMap[preRelease[0]]}>{preRelease[0]}</Badge>;
snowystinger2 years ago

didn't think there'd be a provider around this?

jluyau review fixes
df8903fd
jluyau lint fix
34489160
snowystinger
snowystinger commented on 2022-08-23
Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/chromatic/Badge.chromatic.tsx
27
28export default meta;
29
30
const Template = (): Story<SpectrumBadgeProps> => (args) => (
snowystinger2 years ago

haha, almost, this is CSF2

3 is what you used for the stories file down below

jluyau2 years ago

lol i keep referencing the wrong examples ๐Ÿ˜ญ

jluyau update chromatic file to CSF3
5ed0c3aa
jluyau Add tests
62c19f9c
jluyau Merge branch 'main' into badge-component
08f30841
jluyau jluyau marked this pull request as ready for review 2 years ago
jluyau add docs
026d9242
snowystinger
snowystinger commented on 2022-08-31
snowystinger2 years ago

Screen Shot 2022-08-30 at 6 21 47 PM

lol, nice

jluyau
jluyau2 years ago (edited 2 years ago)
Screen Shot 2022-08-30 at 6 21 47 PM

lol, nice

I'm not sure how to apply the theme. Tried using <ThemeProvider> but then Parcel complains about window.

also, dont know why the prop table isn't populating

jluyau update copyright years, fix types export
025125b4
jluyau
jluyau2 years ago

fixed the prop table issue. only thing left is to figure out how to apply the theme to the new Provider in the side rail. or maybe just go without the Provider?

reidbarber
reidbarber commented on 2022-09-02
reidbarber2 years ago

Awesome, couple comments

Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/docs/Badge.mdx
76Use the appropriate color to indicate status as follows.
77
78```tsx example
79
<Badge variant="positive">Green: Approved, Complete, Success, New, Purchased, Licensed</Badge>
reidbarber2 years ago

style nit, but maybe these can be wrapped in a Flex with a gap, and maybe direction="column"

Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/docs/Badge.mdx
42
43## Content
44
45
Badges can hav a label, an icon, or both. An icon is provided by passing an icon component as a child to the Badge.
reidbarber2 years ago
Suggested change
Badges can hav a label, an icon, or both. An icon is provided by passing an icon component as a child to the Badge.
Badges can have a label, an icon, or both. An icon is provided by passing an icon component as a child to the Badge.
devongovett
devongovett commented on 2022-09-03
packages/dev/docs/src/VersionBadge.js
1111 */
1212
1313
14import badgeStyles from '@adobe/spectrum-css-temp/components/label/vars.css';
14
import {Badge} from '@react-spectrum/badge';
devongovett2 years ago๐Ÿ‘ 1

this will need to be temporarily pulled out so we can merge. otherwise, the production docs will try to find this package but it won't be published yet.

packages/@react-types/badge/package.json
1{
2 "name": "@react-types/badge",
3
"version": "3.0.0",
devongovett2 years ago

should start at "3.0.0-alpha.0"

packages/@react-spectrum/badge/docs/Badge.mdx
21---
22category: Status
23keywords: [badge]
24
---
devongovett2 years ago

add after_version: 3.0.0 so that this doesn't go out to the prod docs website right away. it would break until we publish the package.

devongovett
devongovett2 years ago

Not sure if we should support disabled badges. I am not really sure what the use case is, and since badges don't have any accessibility semantics, this would just be a general contrast violation. Spectrum says:

Badges should only be able to be disabled if they are interactive.

https://spectrum.adobe.com/page/badge/#Disabled

but afaik badges are never interactive. ๐Ÿคท

devongovett
devongovett2 years ago๐Ÿ‘ 1

Update: discussed with spectrum. We should remove the disabled state.

jluyau feedback fixes
5d2565b4
jluyau Merge branch 'main' into badge-component
904a056f
devongovett
devongovett commented on 2022-09-07
Conversation is marked as resolved
Show resolved
packages/@react-spectrum/badge/docs/Badge.mdx
87</Flex>
88```
89
90
When status lights are used to color code categories, they use label colors. The ideal usage for these is when there
devongovett2 years ago
Suggested change
When status lights are used to color code categories, they use label colors. The ideal usage for these is when there
When badges are used to color code categories, they use label colors. The ideal usage for these is when there
jluyau Update packages/@react-spectrum/badge/docs/Badge.mdx
d9ec4f0f
devongovett
devongovett approved these changes on 2022-09-07
reidbarber
reidbarber approved these changes on 2022-09-07
reidbarber2 years ago

LGTM

reidbarber Merge branch 'main' into badge-component
e79c7e37
devongovett devongovett merged 769a91a8 into main 2 years ago
devongovett devongovett deleted the badge-component branch 2 years ago

Login to write a write a comment.

Login via GitHub

Assignees
No one assigned
Labels
Milestone