[fix] segmented control wrapping issue #12879
Conversation
…th border removal Fixes #12857 Replaces negative marginRight approach with borderLeft removal to prevent flexbox width calculation errors that caused buttons to wrap unexpectedly.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
sfc-gh-lwilby
added
security-assessment-completed
sfc-gh-lwilby
changed the title
sfc-gh-lwilby
changed the title
| rowGap: spacing.twoXS, | ||
| // Adding an empty pseudo-element after the last button in the group. | ||
| // This will make buttons only as big as needed without stretching to the whole container width (aka let them 'hug' to the side) | ||
| // This is only needed if the button group has content width. |
There was a problem hiding this comment.
I don't see regression tests failing after removing this. Perhaps there are other changes that make it unnecessary now.
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where segmented control buttons wrap unexpectedly to a new line when using width='content' (default setting). The root cause was a pseudo-element with flex: 10000 that, combined with negative margins in BaseWeb, caused sub-pixel rounding errors in flexbox width calculations.
Key Changes:
- Removed the
segmentedControlNoStretchvariable and its associated::afterpseudo-element that was causing precision errors in content-width layouts
|
Just for sanity sake I did another look at what this was used for and it looks like it was necessary due to the flex-grow: 1 property on the button prior to the width changes were added. Now, we don't add this if the button group is content-width (only case where pseudo-element is still used) so this seems safe to remove and clean up the implementation. |
Describe your changes
Fixed segmented control buttons wrapping unexpectedly to a new line when using
width='content'(default). The bug was caused by negative margins interfering with flexbox width calculations, leading to sub-pixel rounding errors.Changes Made:
Root Cause:
The original implementation used negative right margins (-1px) to overlap borders between adjacent buttons. However, when flexbox calculates available space for wrapping, it computes flex-basis BEFORE considering negative margins. Combined with the
::after { flex: 10000 }pseudo-element trick used for content-width layouts, this caused precision errors in width calculations that triggered premature wrapping of the last button.Additional Notes:
I was not able to reproduce the issue locally since we have to deterministic repro steps.
Removing the negative margins was the first attempt, but this leads to other style regressions when buttons wrap or are selected.
I am not sure in which scenario we need the pseudo-element, but we do have quite extensive regression test coverage and removing this does not lead to any failures. It could be that this element is not needed anymore with the other changes that were made to support more width options.
GitHub Issue Link
Fixes #12857
Testing Plan
st_segmented_control_test.pyContribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.