Repository navigation
Make the comment's controls legible in the dark - #55
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shadcn's ghost carries `dark:hover:bg-accent/50` beside `hover:bg-accent`. The two have different modifiers, so tailwind-merge does not treat them as the same class: a caller writing `hover:bg-sunken` replaces the first and the second survives, and in the dark the `dark:` one wins. Fourteen buttons in this app set their own hover background and every one of them had it quietly taken away. Most only came out the wrong colour. The armed close button — the "Sure?" confirmation — did not: its text is dark because its background is meant to be light, so the swap left near-black on near-black at 1.1:1 and the word simply disappeared under the pointer. The light half already behaved, because that rule never applied there. Now both do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Reported: hovering a comment in dark mode shows the copy and close controls, and
the contrast is bad enough that they cannot be made out.
What was wrong
The three controls on a comment — copy, the published link, close — were drawn
in
faint, the app's quiet grey for icons.faintis held at 3:1 on purpose,and the stylesheet says why: an icon carries its meaning in a label and is a
control rather than prose, so brightening it to 4.5:1 would flatten the one
distinction the quiet inks exist to draw.
That reasoning is sound, and it is written against the surfaces the rest of the
app is drawn on. The comment row is not one of them: it has a tint of its own,
comment-bg. Against that, the same grey lands at#6c747don#141d29#7f8791on#eef3faRight on the floor, under a 14px glyph with a thin stroke.
styles.test.tsalready measured this —
comment-bgwas inBEHIND_ICONSand the assertion is>= 3— so it passed the guard and could not be found on the screen. The numberwas right and the bar was wrong.
What it does now
A
comment-fainttoken, dimmed from the row's own ink rather than brightenedfrom the grey, so the controls still read as quiet parts of the comment instead
of chrome that wandered in:
4.6:1 light, 5.3:1 dark. Hover is unchanged: full
comment-inkonsunken,which was already fine at 7.7:1.
comment-bgcomes out ofBEHIND_ICONSand gets its own assertion at 4.5:1,plus one keeping the controls quieter than the prose they sit beside — lifting
them until they compete with the comment is the other way of getting this wrong.
Verified on screen
Same comment row, same element, photographed twice in dark mode: once with the
old grey forced back on, once with the new ink. The copy and check icons go from
melting into the row to legible.
Checks
Rust passes in full. On the web side
App.test.tsx > light and dark > puts the choice on the root elementfails on my machine withlocalStorageundefined;it fails the same way on
mainuntouched and passes on CI.The one that was actually reported (added after the first review)
The above is real, but it is not what was being described. The complaint was the
armed close button — the "Sure?" confirmation — going unreadable on
hover, and that had nothing to do with tokens.
shadcn's
ghostvariant carriesdark:hover:bg-accent/50besidehover:bg-accent. The two have different modifiers, so tailwind-merge does nottreat them as the same class: the pill's own
hover:bg-comment-ink/90replacesthe first, the second survives, and in the dark the
dark:one wins. On hoverthe background became
accent-dimat half — a dark olive — while the textstayed
surface, near-black, because that override did match and stick.Near-black on dark olive is 1.1:1. The word disappears.
Fourteen buttons in this app set their own hover background and every one of
them had it quietly taken away in the dark. The others only came out the wrong
colour; this one is the only place the text was dark, which is why it is the one
that broke.
Dropping the
dark:rule lets each caller's hover apply, and makes the dark halfbehave the way the light half already did.
button.test.tspins it, so a shadcnupdate that puts it back fails rather than going unnoticed.
Verified on screen: the armed pill under a real pointer hover, before and after.
Before, the pill is an empty dark rectangle.