shimmer: Keep the default highlight visible on foreground text in dark mode - #3328
Merged
huacnlee merged 1 commit intoOct 1, 2026
Merged
Conversation
…k mode In a dark theme the default highlight mixes the text 80% toward `foreground`. For text that already is `foreground` (the inherited default) the band had the text's own color, so nothing visibly swept. When the target's lightness is within 0.1 of the text's, mix toward the opposite end (`background` in dark, `foreground` in light) instead. Muted text and the light theme keep today's highlight. Closes longbridge#3327 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Member
|
Upload video or screenshot when you have changed UI. |
This was referenced Sep 30, 2026
Open
Contributor
Author
|
Thanks, done. I've added before/after recordings from a physical Android phone (dark theme) to the Screenshot section above. Before pr3328-before.mp4After pr3328-after.mp4The top line is the unstyled |
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
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.
Closes #3327
Description
Thank you for the quick feedback on #3310. This is the first of the focused fixes.
In a dark theme,
ShimmerText's default highlight is the text mixed 80% towardforeground(mix_oklab's factor weights the first color). For text that already isforeground, which is the inherited default, e.g.ShimmerText::new("Thinking…")in the story, the band has the text's own color, so nothing visibly sweeps.shimmer_highlight_colornow keeps the current target (foregroundin dark,backgroundin light) unless its lightness is within0.1of the text's; in that case it mixes toward the opposite end.muted_foregroundtext, the light theme and explicithighlight_color(..)produce exactly the same color as before; only the case that was invisible changes, to a band that dims the glyphs as it passes.Happy to change the threshold or the approach, e.g. if you'd rather always use a muted base color, as
Markerdoes.Screenshot
Physical Android phone, dark theme, unstyled
ShimmerText::new("Thinking…")(as in the Shimmer story) above amuted_foregroundone for comparison.Before
pr3328-before.mp4
After
pr3328-after.mp4
Before: the first line never visibly changes. After: a band sweeps across it. The muted line is the same in both.
Builds: demo-pr3328 (Kit 0.7.0 with and without this diff).
How to Test
cargo test -p gpui-component --lib shimmer: 4 passed. The extendedtest_shimmer_highlight_stays_bright_in_both_themesasserts thatforegroundtext in dark mode andbackground-colored text in light mode get a highlight at least0.3darker than the text. With the fallback disabled (threshold0.0), that assertion fails.highlight_colorare unchanged and still pass.ShimmerText::new("Thinking…")) now shows the sweep.Checklist
cargo runfor story tests related to the changes. (Not run: no desktop session here; see How to Test.)Thanks for taking the time to review this.
🤖 Generated with Claude Code