Add tier indicator to the tabs component as optional - #3790
Open
missating wants to merge 6 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 92a32ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
missating
changed the title
Contributor
|
Size Change: +131 B (+0.02%) Total Size: 850 kB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3790 +/- ##
=======================================
Coverage 93.56% 93.57%
=======================================
Files 208 208
Lines 4614 4619 +5
Branches 1808 1807 -1
=======================================
+ Hits 4317 4322 +5
Misses 268 268
Partials 29 29
🚀 New features to boost your workflow:
|
connor-baer
reviewed
Jul 31, 2026
Comment on lines
+27
to
+30
| import { | ||
| TierIndicator, | ||
| type TierIndicatorProps, | ||
| } from '../../../TierIndicator/TierIndicator.js'; |
Member
There was a problem hiding this comment.
Importing the TierIndicator in the Tab component causes it to be bundled whether the trailingComponent prop is used or not.
When we discussed this feature a while ago, we had agreed that the component should be injected to avoid this bundle size increase. What happened to that approach?
Contributor
Author
There was a problem hiding this comment.
You're right, and I'd forgotten we'd agreed on that.
One thing, though: injecting the component moves the size="s" restriction from compile time to runtime, but I think 🤔 that should be fine - we can still enforce it, just not via the type system.
Member
There was a problem hiding this comment.
I think it should still be possible to enforce this with types. I imagine the trailingComponent to by typed like so:
trailingComponent: ComponentType<Omit<TierIndicatorProps, 'size'>>
sirineJ
reviewed
Aug 4, 2026
| "@sumup-oss/circuit-ui": minor | ||
| --- | ||
|
|
||
| Added a `trailingComponent` prop to the Tab component to display a TierIndicator badge next to a tab's label to indicate features that are part of the plus plan. |
Contributor
There was a problem hiding this comment.
trailingComponent seems like a broad prop name when in reality it only accepts a specific component.
Member
There was a problem hiding this comment.
@missating and I had discussed the prop name and opted to keep it generic in case we want to open it up to other content (e.g. the Status component) in the future.
| const content = TrailingComponent ? ( | ||
| <span className={classes.content}> | ||
| {children} | ||
| <TrailingComponent variant="plus" size="s" /> |
Contributor
There was a problem hiding this comment.
currently we only have one variant, but what if we added more in the future ?
Member
There was a problem hiding this comment.
The variant could be passed like so:
<Tab trailingComponent={(props) => <TierIndicator {...props} variant="new-variant" />} />| ]} | ||
| />, | ||
| ); | ||
| expect(screen.getByText('Services')).toBeVisible(); |
Contributor
There was a problem hiding this comment.
shouldn't we test for the visibility of the tier indicator too ?
Suggested change
| expect(screen.getByText('Services')).toBeVisible(); | |
| expect(screen.getByText('Services')).toBeVisible(); | |
| expect(screen.getByText('plus')).toBeVisible(); |
|
|
||
| export const WithTierIndicator = (args: TabsProps) => <Tabs {...args} />; | ||
|
|
||
| WithTierIndicator.args = { |
Contributor
There was a problem hiding this comment.
maybe we can disable the snapshots on the other stories and keep them only for this one to reduce chromatic consumption
connor-baer
reviewed
Aug 5, 2026
| <Tab trailingComponent={TierIndicator}>Services</Tab>, | ||
| ); | ||
| expect(screen.getByText('Services')).toBeVisible(); | ||
| expect(container.querySelector('svg')).toHaveAttribute('height', '16'); |
Member
There was a problem hiding this comment.
The height attribute doesn't necessarily reflect the true size of the SVG as it could be resized using CSS.
Testing styles in unit tests is unreliable, that's why we have snapshot tests. Unit tests should focus on the UI logic, semantics, and public API.
| const content = TrailingComponent ? ( | ||
| <span className={classes.content}> | ||
| {children} | ||
| <TrailingComponent variant="plus" size="s" /> |
Member
There was a problem hiding this comment.
The variant could be passed like so:
<Tab trailingComponent={(props) => <TierIndicator {...props} variant="new-variant" />} />
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses DSYS-1824
Purpose
This adds an optional
trailingComponentprop to Tab that renders a TierIndicator badge next to the tab's label to indicate that a feature is behind a paid plan.Approach and changes
trailingComponent?: ComponentType<TierIndicatorProps>to TabProps. Consumers inject theTierIndicatorcomponent itself rather than passing rendered JSX or a props object.Tab.tsxnow destructures children explicitly (it previously flowed through implicitly via the props spread)Tabs. tsxitems mapping now forwardstrailingComponenttoo (previously onlyid/tabmade it through), so the badge works via the top-levelitemsAPI, not justTabList'stabs propDefinition of done