-
Notifications
You must be signed in to change notification settings - Fork 628
Add count
to SegmentedControlButton
#6721
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
Conversation
Modeled after the counter in packages/react/src/UnderlineNav/UnderlineNavItem.tsx and how it's rendered in packages/react/src/internal/components/UnderlineTabbedInterface.tsx.
🦋 Changeset detectedLatest commit: 175a276 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/1723 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new counter
prop to SegmentedControl.Button
to display count labels on the right side of buttons, following the same pattern as UnderlineNav.Item
. This feature enables displaying numeric counts alongside segment labels, useful for scenarios like showing issue counts by label type.
- Adds
counter
prop accepting number or string values toSegmentedControlButton
- Implements counter rendering using the existing
CounterLabel
component with proper spacing - Updates Storybook stories and documentation to showcase the new functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
SegmentedControlButton.tsx | Adds counter prop type definition and conditional rendering logic |
SegmentedControlButton.stories.tsx | Updates Playground story to include counter control |
SegmentedControl.module.css | Adds CSS styling for counter positioning and alignment |
SegmentedControl.features.stories.tsx | Adds new "With Counter Labels" feature story |
SegmentedControl.docs.json | Documents the new counter prop in component metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for taking the time to submit a PR! Appreciate it a ton 🙏
Just left a couple of comments/suggestions, hope they make sense. I also tagged in @langermank on the design side to review 🔍
@@ -111,6 +111,12 @@ | |||
"defaultValue": "", | |||
"description": "Whether the segment is selected. This is used for uncontrolled SegmentedControls to pick one SegmentedControlButton that is selected on the initial render." | |||
}, | |||
{ | |||
"name": "counter", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random question for you @langermank, do you know why for Button
we call this count
. but for UnderlineNav.Item
it's counter
? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's any logical reason other than they were probably added at different times and by different people, and it would be nice if they matched 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@langermank do you have a preference for count
or counter
? Mine would be count
, I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree on "count"!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can update!
packages/react/src/SegmentedControl/SegmentedControl.module.css
Outdated
Show resolved
Hide resolved
https: //github.com//pull/6721#discussion_r2322729732 Co-Authored-By: Josh Black <[email protected]>
https: //github.com//pull/6721#discussion_r2322722420 Co-Authored-By: Josh Black <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some squishyness happening, I haven't taken a closer look to find a solution but happy to if needed!

@cheshire137 if you wouldn't mind, could you add the new feature story you added to this file so it will create a VRT snapshot? No worries if not, we can add it in a followup!
@@ -111,6 +111,12 @@ | |||
"defaultValue": "", | |||
"description": "Whether the segment is selected. This is used for uncontrolled SegmentedControls to pick one SegmentedControlButton that is selected on the initial render." | |||
}, | |||
{ | |||
"name": "counter", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's any logical reason other than they were probably added at different times and by different people, and it would be nice if they matched 😄
This reverts commit 6a9f2a4.
https: //github.com//pull/6721/files#r2330801548 Co-Authored-By: Katie Langerman <[email protected]> Co-Authored-By: Josh Black <[email protected]>
count
to SegmentedControlButton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to open a followup PR to fix the overflow problem 👍
Oh heck, I forgot all about that comment, sorry about that and thank you @langermank! If you'd prefer, we could get it fixed in this branch. |
@cheshire137 no worries at all. I dug into it and its not really tied to the count addition, it's pre-existing. So I think it's probably better to fix it in a separate PR anyway! |
I'll get conflicts resolved. |
This adds a new
count
prop toSegmentedControl.Button
, modeled after thecounter
thatUnderlineNav.Item
has and thecount
thatButton
has.cc @francinelucca with whom I talked about adding such a prop
Changelog
New
Adds the
count
prop toSegmentedControl.Button
to allow displaying a count label on the right side of the button.Changed
Removed
Rollout strategy
Testing & Reviewing
The SegmentedControl > Features > With Counter Labels story and SegmentedControl > SegmentedControl.Button > Playground story have been updated to show off the new prop.
Merge checklist