fix(ui): draw the split button's divider on every surface - #173
Open
renefloor wants to merge 7 commits into
Open
fix(ui): draw the split button's divider on every surface#173renefloor wants to merge 7 commits into
renefloor wants to merge 7 commits into
Conversation
The divider took borderDisabled while disabled, close enough to the disabled surface to vanish, so a disabled split button read as one wide pill with two icons in it. Checking the other states turned up more of the same: the divider is drawn on the button's own surface, so what it should be is a question about that surface rather than about the button's style. solid, accent fill border/core/on-accent, per the design solid, secondary border/core/default outline the button's own border, so the divider continues it ghost the button's icon colour, the only colour it has disabled border/core/default, whatever the style Only the first two are in the design; there is none for outline and ghost, and the previous grey hairline cut across a blue or red outline rather than continuing it. These are unreleased and unused, so the choice is cheap to revisit. The style defaults now take the resolved button style, since the divider's colour is a property of what it is drawn on. Adds a disabled matrix golden in both brightnesses. The state assertions read what is painted — the surface's outline, the glyph's own text style — rather than restating the tokens the implementation uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
renefloor
commented
Sep 1, 2026
Comment on lines
-437
to
+459
| if (states.contains(WidgetState.disabled)) return _colorScheme.borderDisabled; | ||
| return _colorScheme.borderDefault; | ||
| // A disabled button drops its fill, its outline and its icon colour for | ||
| // the shared disabled treatment, so the divider goes back to the hairline | ||
| // that reads on it rather than disappearing into it as borderDisabled did. | ||
| if (states.contains(WidgetState.disabled)) return _colorScheme.borderDefault; | ||
|
|
||
| return switch (_type) { | ||
| // Drawn against the fill, unless the fill is already a light one. | ||
| StreamButtonType.solid => | ||
| _style == StreamButtonStyle.secondary ? _colorScheme.borderDefault : _colorScheme.borderOnAccent, | ||
| // Continues the outline it sits inside. | ||
| StreamButtonType.outline => _buttonStyle.borderColor?.resolve(states) ?? _colorScheme.borderDefault, | ||
| // No fill and no outline, so the icons either side of it are the only | ||
| // colour the button has to match. | ||
| StreamButtonType.ghost => _buttonStyle.foregroundColor?.resolve(states) ?? _colorScheme.borderDefault, | ||
| }; |
Collaborator
Author
There was a problem hiding this comment.
Maybe in the next major release for chat, but I think we should have something like StreamMessageLayoutData for buttons, so you can pass all relevant fields to the theme resolver instead of the nested button styles we have now.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #173 +/- ##
==========================================
+ Coverage 65.93% 67.82% +1.89%
==========================================
Files 203 208 +5
Lines 8198 8528 +330
==========================================
+ Hits 5405 5784 +379
+ Misses 2793 2744 -49 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xsahil03x
approved these changes
Sep 2, 2026
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
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.
Submit a pull request
Linear: FLU-754
Github Issue: #
CLA
Description of the pull request
The divider took borderDisabled while disabled, close enough to the disabled surface to vanish, so a disabled split button read as one wide pill with two icons in it. Checking the other states turned up more of the same: the divider is drawn on the button's own surface, so what it should be is a question about that surface rather than about the button's style.
Screenshots / Videos