fix: support padded tabs #330
Open
Conversation
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
Collaborator
🟡 Heimdall Review Status
🟡
|
| Code Owner | Status | Calculation | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| ui-systems-eng-team |
🟡
0/1
|
Denominator calculation
|
hcopp
commented
Jan 27, 2026
| const animatedBoxStyle = useAnimatedStyle( | ||
| () => ({ | ||
| transform: [{ translateX: animatedTabRect.value.x }], | ||
| transform: [{ translateX: animatedTabRect.value.x }, { translateY: animatedTabRect.value.y }], |
Contributor
Author
There was a problem hiding this comment.
On mobile we only needed to track the y value
hcopp
commented
Jan 27, 2026
| data-testid="tabs-active-indicator" | ||
| height={height} | ||
| initial={false} | ||
| left={0} |
Contributor
Author
There was a problem hiding this comment.
On web we only needed to set the left offset to 0, so that the logic for tracking position doesn't add the left offset on top of the default padding. This isn't a worry for y since we aren't animating by y it will simply use the default padding.
We could in theory adjust the activeTabRect above to track y and update active animation as well and then we would need to add top={0}. But this is a simpler solution.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
What changed? Why?
This change enables support for using padding props on Tab related components, including SegmentedTabs and PeriodSelector.
This request came in by a user wanting to use PeriodSelector with padding. While I do not foresee users wanting to add padding to SegmentedTabs, it is much easier to visualize the issue there as it has a background.
Root cause (required for bugfixes)
On web we were "double counting" padding by having the active indicator be offset by padding and then adding it to our x value. This meant that it would be too far over to the right.
On mobile we weren't setting the y value of the element, since it needed explicit x/y offset. This meant that it would sit at the top, not impacted by padding.
We fixed these by setting left={0} on web and on mobile setting the y value of the active indicator.
UI changes
Testing
How has it been tested?
Testing instructions
Illustrations/Icons Checklist
Required if this PR changes files under
packages/illustrations/**orpackages/icons/**Change management
type=routine
risk=low
impact=sev5
automerge=false