fix(tags): carry tag state in an icon, not the fill - #8569
talissoncosta wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
the
front-end
Docker builds report
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true📝 WalkthroughWalkthroughThe change adds paired light and dark tag surface/text tokens with contrast validation and generated utility classes. It expands Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to System tags can render with the wrong visual treatment, and assistive-technology users cannot tell whether an interactive tag is selected. Correct both before merging. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
4d9d02b to
85e9d84
Compare
❌ private-cloud · depot-ubuntu-latest-16 — run #20583 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss 🗂️ Previous results❌ private-cloud · depot-ubuntu-latest-16 — run #20582 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ oss · depot-ubuntu-latest-16 — run #20583 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ oss · depot-ubuntu-latest-16 — run #20582 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ private-cloud · depot-ubuntu-latest-16 — run #20581 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ private-cloud · depot-ubuntu-latest-arm-16 — run #20581 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ oss · depot-ubuntu-latest-16 — run #20581 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ private-cloud · depot-ubuntu-latest-16 — run #20580 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ oss · depot-ubuntu-latest-16 — run #20580 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ private-cloud · depot-ubuntu-latest-16 — run #20579 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss ❌ oss · depot-ubuntu-latest-16 — run #20579 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/flag-tests.pw.ts › Flag Tests › Feature flags can have tags added and be archived @oss |
Visual Regression18 screenshots compared. See report for details. |
85e9d84 to
2d23e41
Compare
5ab4fa7 to
d59d045
Compare
It was a bespoke pill with its own stylesheet, duplicating shape, sizing and a hand-written dark-mode block that Chip already handles. Rebuilt as a status-to-variant map, deleting 51 lines of SCSS. This changes the experiment status badges from fully rounded to 6px. Deliberate: --radius-full came from StatusBadge.scss and had never been checked against the design system, and nothing in the Figma tags frame is fully rounded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d59d045 to
175ab34
Compare
175ab34 to
e17a0d1
Compare
Custom tags derive their fill, border and text from the tag's own hue at
render time, so the contrast ratio is whatever the hue happens to give and
most of the palette fails WCAG AA.
Adds a scale of validated {surface, text} pairs instead, built from the
existing primitive ramps so a ramp change carries through:
light surface <hue>-100 text <hue>-800
dark surface <hue>-900 text <hue>-100
Two exceptions, both because the ramp offers no step that works. Gold's light
surface is too pale for -800 (2.82), so its text takes -950. Slate's dark
surface at -900 is the page background, so it takes -800.
Splitting them into tag-surface and tag-text lets the generator emit
.bg-tag-<hue> and .text-tag-<hue>, so a tag needs no stylesheet of its own.
The test proves AA once, over the scale, rather than measuring contrast in the
browser. The story shows each pair as a real chip captioned with its measured
ratio, in both themes, so the claim is checkable rather than asserted.
Nothing renders against these yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The design asks for a redrawn Stale, the ionicon being too detailed at 16px, and a PR dequeued icon. The latter is a live gap rather than a nicety: GitHubTag.PR_DEQUEUED is already written by the API when GitHub reports a PR leaving a merge queue, so that tag reaches the UI and renderIcon falls through to a bare return. Both follow the frame's spec, 16px at 1.5px stroke, and are drawn at 0.88 of the box so they carry the same optical weight as the fill-based Octicons beside them. pr-draft moves onto --color-icon-secondary. It was the only status icon on currentColor, so it took the chip's label colour: near-black in light, white in dark, neither of which the design asks for. Its siblings hardcode GitHub's brand colours, which are the same in either theme, but the design gives pr-draft #747B86, a neutral, and a neutral cannot be hardcoded: it has to lift in dark mode or it sinks into the surface. Both new icons are registered in the catalogue, which is a hand-maintained list, so an icon is invisible in Storybook without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e17a0d1 to
0957283
Compare
0957283 to
0d85b05
Compare
Tags rendered the tag's own colour as text on an 8% tint of itself, so the
contrast ratio was whatever the hue happened to give and most of the palette
failed WCAG AA.
System tags (Issue, PR, Stale, Unhealthy) now sit on plain surface with a
neutral border and a coloured icon, so the state is carried by the icon and
the label keeps full contrast. Custom tags take a validated swatch pair from
the scale, keyed on the colour already stored, so no tag changes identity and
nothing needs migrating.
That removes every colour computation, in all four places it had been copied
to, two of which no stylesheet could reach:
Tag.tsx fade(.92) / fade(.76) / darken(.1), plus shouldLighten
and a #344562 special case, both of which existed only
because the fill was the hue
TagContent.tsx darken(.1) for the icon, and the same three again as an
inline style string
ToggleChip.tsx the same three, on Tag's onClick path
FeatureAction.tsx the same three, in a tooltip string
The two string sites stay strings, being rendered through
dangerouslySetInnerHTML, but they now carry the same classes as the real chip
rather than their own copy of the rules.
renderIcon also now answers to GITLAB, which had never been wired. Harmless
while the fill carried the state; a regression once it does not.
TagType gains GITHUB and GITLAB to match the API enum. The icon switch has
always branched on values the type said could not occur.
escapeHTML's character class is restated as the characters it keeps. Same set,
proven equivalent across U+0000..U+1FFF, but without control characters in the
literal, which the pre-commit lint rejects now the file is touched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the semantic variants and the solid fill, then the two shapes a tag takes: a system tag on plain surface with a coloured icon, and a custom tag carrying a swatch pair. The tag stories live here rather than beside Tag because there is no tag component to story. Tag reads the store and the flags, and the appearance decision lives in tagSwatch.ts, which is pure and tested on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0d85b05 to
cb2f9e1
Compare
This branch was successfully deployed
2 participants
docs/if required so people know about the feature.Changes
Contributes to #8465. Supersedes #8505, whose commits are included here.
Tags rendered the tag colour as text on an 8% tint of itself, so contrast was whatever the hue
gave and most of the palette failed AA.
icon, so the icon carries the state.
{surface, text}pair, keyed on the colour already stored, so notag changes identity and nothing migrates.
This deletes the colour maths from all four places it had been copied to, two of them inline
style strings behind
dangerouslySetInnerHTML.Chipgains the semantic variants those callsites need and
StatusBadgemoves onto it, dropping 51 lines of SCSS. Adds a redrawn Stale iconand PR dequeued, which the API already creates but the UI rendered without an icon.
Three things worth knowing:
is open with Dragos. Swapping them in is a
tokens.jsondiff, no component work.to tags, most visible thing here.
Not here: migrating the ~60 remaining legacy
.chipsites, which needs its own issue.How did you test this code?
Storybook in both themes, plus
tagSwatches.test.tsasserting every swatch pair clears 4.5:1 inlight and dark.
tsc910 errors against 913 on main. Not yet exercised on real screens.To check manually:
StatusBadgeshape change.