-
Notifications
You must be signed in to change notification settings - Fork 0
Review 4311
HelloOjasMutreja · external contributor (57 PRs / 33 merged) · #4311 · MERGED 2026-08-25
Head 499e8493dd9a8b18aef5c443a54416d7e213d5a8. Waited 44h 17m after pushing fixes on 23 Aug 22:05.
| our ask | status at this head |
|---|---|
use var(--color-text-secondary), don't hardcode the light half of the pair |
satisfied — all 71 substitutions are the token, zero hex, zero light-dark(
|
the #666 = 4.48:1 arithmetic is wrong (it is 5.74) |
moot — every value is a token now |
Reversed ourselves to approve, and said so in the comment. Only our account could clear the block.
| theme · scheme | before #666
|
after token |
|---|---|---|
| neutral/butter/chocolate/gothic/matcha · light | 5.74 | 7.81 |
| neutral/butter/chocolate/gothic/matcha · dark | 2.64 ✗ | 6.00 |
| stone · light | 5.74 | 3.76 ✗ |
| stone · dark | 2.99 ✗ | 6.36 |
| y2k · light / dark | 5.74 / 3.05 ✗ | 6.43 / 7.91 |
Net 7 failing cells → 1.
stone light drops below AA because packages/themes/stone/src/stoneTheme.ts:108 sets --color-text-secondary to ['#83838a', '#9d9da3'] with its own comment reading // T55 C=4. 83 files under packages/core/src reach for that token, so they all render sub-AA secondary text in stone light. Blocking on it would have meant asking this contributor to hardcode a hex again — the exact thing we blocked them for two days earlier. Filed as #5505 instead, crediting them.
Thumbnail's RemoveOnHover story kept two #888s while five sibling stories in the same file took the token — including a byte-identical "All removed." line converted twice and missed once (Thumbnail.stories.tsx:93, :111). Inline left; did not hold the merge.
Risk class: low-risk. pr-a11y and pr-rtl were skipped, not passed — path filters exclude apps/storybook, so CI never validated the fix and the numbers above are ours.