-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5321
Abidit (contributor, fork) · OPEN, approved + auto-merge armed · view on GitHub
Verdict: approve and merge
Head reviewed: 4dbf888dcca0684465f93ce748d88c26a286a492. No formal review; Abhirup0
commented and asked for four things, all four are in the head.
In dark mode, initials on an imageless avatar sink into the grey chip — 4.37:1
against a 4.5:1 bar. The +N overflow chip sits on the identical effective
background and reads the same.
The fallback colour becomes light-dark(--color-text-secondary, --color-text-primary) at both sites painting on --color-neutral. One decision,
not two: same background, and shipping one alone leaves +N dimmer than the
initials beside it.
Everyone on the next release, dark only — 4.37→10.57 (neutral), 4.27→7.26 (raw tokens). Light byte-identical. Reaches less far than it looks: chocolate, matcha and stone still fail on the LIGHT side and the mechanism structurally cannot reach them.
No API change. Theme targets: none.
- API — no.
- Visual — yes, deliberate: a dark recolour in every consumer. Nothing grew.
-
Theme — one real change: dark stops consulting
--color-text-secondary, so a theme tuning that token loses the lever..astryx-avatar-fallbackstill overrides, verified in Chromium on both builds.
Nothing — two static style values.
Real Chromium, bare story iframe, all frames opened with the read tool:
shots-rG-5321/group/{before,after}__neutral__dark.png (initials),
shots-rG-5321/ovf/{before,after}__neutral__dark.png (the +N chip),
shots-rG-5321/group/after__stone__light.png (the side it cannot reach, 3.36:1).
Matrix (--port 6100 vs 65321): none 5.42/5.07→5.42/8.60 · neutral 6.86/4.37
→6.86/10.57 · stone 3.36/4.81→3.36/11.70 · y2k 5.71/6.12→5.71/11.82.
Low-risk on all three tests — but a deliberate recolour in every dark app, so not an unattended post.
approve and merge
1. [not blocking] the changeset names Avatar only; the +N chip changed too
→ a changelog reader does not learn the face-pile overflow recoloured
· .changeset/avatar-fallback-dark-contrast.md:5
2. [not blocking, not theirs] three themes fail this element on the LIGHT side and
Kbd pairs the same two tokens identically (Kbd.tsx:45,49)
→ held as a ruling for the maintainer, not raised on the PR
CI had never run — four workflows sat action_required (first-time fork). Diff
scanned for safety (four files: a changeset, a test, two one-line colour changes; no
workflow, lifecycle script, dependency or lockfile change) and the runs were
approved so CI could build it. Avatar.test.tsx 53/53 locally beforehand.
Thanks, this holds up — measured 4.37:1 to 10.57:1 in dark, light byte-identical. Abhirup0's four points all landed. Merging; the wider secondary-on-neutral pairing is ours to sort.
-
.changeset/avatar-fallback-dark-contrast.md:5— Changeset says Avatar only. Might want to name the AvatarGroup +N indicator too.
Posted as drafted.