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

feat(badge-indicators): new component #18342

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ariellalgilmore
Copy link
Member

Closes #

{{short description}}

Changelog

New

  • {{new thing}}

Changed

  • {{change thing}}

Removed

  • {{removed thing}}

Testing / Reviewing

{{ Add descriptions, steps or a checklist for how reviewers can verify this PR works or not }}

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit bcfa94c
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-web-components/deploys/6785b8b414fc94000817f0d0
😎 Deploy Preview https://deploy-preview-18342--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit bcfa94c
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6785b8b4aec4fd0009d37e6e
😎 Deploy Preview https://deploy-preview-18342--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit bcfa94c
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/6785b8b4450a2d0008ce5187
😎 Deploy Preview https://deploy-preview-18342--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit bc15460
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-web-components/deploys/6788261308ece80008c107f8
😎 Deploy Preview https://deploy-preview-18342--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit bc15460
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/67882613047c120009f172f9
😎 Deploy Preview https://deploy-preview-18342--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit bc15460
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/678826137602470008259e04
😎 Deploy Preview https://deploy-preview-18342--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.17%. Comparing base (05e4725) to head (bc15460).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
packages/react/src/components/IconButton/index.tsx 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #18342   +/-   ##
=======================================
  Coverage   84.17%   84.17%           
=======================================
  Files         408      409    +1     
  Lines       14435    14447   +12     
  Branches     4660     4645   -15     
=======================================
+ Hits        12150    12161   +11     
- Misses       2120     2122    +2     
+ Partials      165      164    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tay1orjones
Copy link
Member

Today we decided to pivot on the following:

Based on the guidance from the open PR, the badge indicator is intended to only be used on a Button/IconButton. That supports:

  1. Not exporting BadgeIndicator
  2. Baking BadgeIndicator into Button/IconButton directly, exposing control through a badgeCount (or similar) number prop
  3. Removing experimental/BadgeIndicator stories and instead just have a "with badge indicator" story for both Button and IconButton.
    • This would essentially be the same story you already have, just splitting it into two different locations (Button using hasIconOnly and IconButton)
    • If/when we determine additional uses for BadgeIndicator outside of Button/IconButton we can explore exporting it and if it should have it's own stories.

It feels to me like this should live under packages/react/src/components because it renders elements to the DOM and has styles associated with it. The components currently in packages/react/src/internal don't render any UI and don't have any styles associated with them.

@tay1orjones
Copy link
Member

Another requirement we talked about - the badge indicator should only show on icon-only buttons that have size="lg" and kind="ghost". This is our understanding of the intent from design @thyhmdo.

Rather than doing a bunch of conditional types, we'll just mention it in the prop description for badgeCount.

  /**
   * Display a badge on the button. An empty/dot badge if 0, a numbered badge if >0.
   * Must be used with size=lg and kind=ghost
   */
  badgeCount?: number;

We can also provide a dev-only console warning if badgeCount is configured and size or kind are wrong.

The IconButton badgeCount prop must be used with size="lg" and kind="ghost". This requirement is documented in the design specification: https://carbondesignsystem.com/patterns/status-indicator-pattern/#badge-indicator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants