Skip to content

Badge component #3429

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

Merged
merged 14 commits into from
Sep 7, 2022
Merged

Badge component #3429

merged 14 commits into from
Sep 7, 2022

Conversation

jluyau
Copy link
Member

@jluyau jluyau commented Aug 23, 2022

#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 jluyau linked an issue Aug 23, 2022 that may be closed by this pull request
@jluyau jluyau added the rsp:Badge Badge component label Aug 23, 2022
@adobe-bot
Copy link

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

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

doesn't quite look right?

@adobe-bot
Copy link

@adobe-bot
Copy link

@jluyau jluyau marked this pull request as ready for review August 26, 2022 22:54
@adobe-bot
Copy link

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

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

lol, nice

@adobe-bot
Copy link

@jluyau
Copy link
Member Author

jluyau commented Aug 31, 2022

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
Copy link
Member Author

jluyau commented Sep 1, 2022

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?

@adobe-bot
Copy link

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

Awesome, couple comments

@@ -11,17 +11,25 @@
*/


import badgeStyles from '@adobe/spectrum-css-temp/components/label/vars.css';
import {Badge} from '@react-spectrum/badge';
Copy link
Member

Choose a reason for hiding this comment

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

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.

@@ -0,0 +1,20 @@
{
"name": "@react-types/badge",
"version": "3.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

should start at "3.0.0-alpha.0"

---
category: Status
keywords: [badge]
---
Copy link
Member

Choose a reason for hiding this comment

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

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
Copy link
Member

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
Copy link
Member

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

@adobe-bot
Copy link

@adobe-bot
Copy link

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

LGTM

@adobe-bot
Copy link

@devongovett devongovett merged commit 769a91a into main Sep 7, 2022
@devongovett devongovett deleted the badge-component branch September 7, 2022 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rsp:Badge Badge component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Badge component
5 participants