-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[JEWEL-1221] TabStrip should not change the size when scrollbar appear #3376
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
base: master
Are you sure you want to change the base?
Conversation
- Using the HorizontallyScrollableContainer causes a minor ui glitch as the component getts bigger to accomodate the scrollbar - To prevent this issue to happen, I'm rollabacking to manually create the scrollbar and use a box instead - This is needed because the default guidelines mandate a space for the scrollbar, however the implementation for tabstrip should render over the tabs
| modifier = Modifier.semantics { hideFromAccessibility() }, | ||
| ) { | ||
| HorizontalScrollbar(scrollState, style = style.scrollbarStyle, modifier = Modifier.fillMaxWidth()) | ||
| } |
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.
Scrollbar appears when content doesn't need scrolling
Medium Severity
The AnimatedVisibility visibility condition only checks tabStripState.isHovered, but the old HorizontallyScrollableContainer also checked scrollState.canScroll to ensure the scrollbar only appears when the content actually needs scrolling. The new implementation will show a scrollbar when hovering even if there are only a few tabs and no scrolling is needed, which differs from the intended behavior where the scrollbar should only appear when content overflows.
94c437f to
8fdbcd0
Compare
rock3r
left a comment
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.
Would like a screenrecording or two to validate the behaviour :)
| enter = | ||
| fadeIn( | ||
| tween( | ||
| durationMillis = scrollbarVisibility.trackColorAnimationDuration.inWholeMilliseconds.toInt(), |
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.
Shouldn't this be the thumb animation duration? Same below
Release notes
Note
Ensures the
TabStripno longer resizes when the scrollbar appears by rendering a hover‑activated overlay scrollbar and using nativehorizontalScroll.HorizontallyScrollableContainerwith aBox+RowusinghorizontalScroll; addsAnimatedVisibilityfade forHorizontalScrollbarand hides it from accessibilitytestTags and a new test assertingTabStripheight remains constant while adding many tabs; minor test setup optimization withderivedStateOfWritten by Cursor Bugbot for commit 8fdbcd0. This will update automatically on new commits. Configure here.