fix(#4009): align checkbox and radio item label weight and color#4010
fix(#4009): align checkbox and radio item label weight and color#4010twjeffery wants to merge 1 commit into
Conversation
|
Preview links
Built from commit 3d620c7. Previews are removed automatically when this PR closes. |
| }); | ||
| }); | ||
|
|
||
| it("applies the medium font-weight token to the label", async () => { |
There was a problem hiding this comment.
Are we sure we want to add these browser tests? We normally use these tests for regressions in functionality, not styling. I don't know how I'd feel if someone's PR was blocked because of a font weight.
There was a problem hiding this comment.
Good call. I've removed the browser style assertions from both checkbox.browser.spec.tsx and radio.browser.spec.tsx so this fix won't gate PRs on a computed font weight or color. The change is verified visually via the 4009 playground pages in React and Angular instead. (The web-component unit tests for both components still pass.)
Generated by Claude Code
bdfranck
left a comment
bdfranck
left a comment
There was a problem hiding this comment.
I reviewed the changes...
- ✅ I see the checkbox label is the correct font weight
- ✅ I see the radio item label is the correct colour
Looks good to me! 👍 I have one comment about the tests.
Checkbox and radio item labels rendered inconsistently. The checkbox label had the correct color but regular weight, while the radio item label had the correct medium weight but no color, so it inherited the near black page default instead of the secondary input text color. Set font-weight: medium on the checkbox base and compact labels (the font shorthand resets the weight to the token default), and resolve the radio item label color through a new --goa-radio-color-label component token that falls back to the shared --goa-input-color-text-secondary, matching how the checkbox label resolves its color. Adds React and Angular playground pages.
twjeffery
force-pushed
the
claude/confident-bell-2WDNZ
branch
from
42f8c24 to
3d620c7
Compare
bdfranck
left a comment
bdfranck
left a comment
There was a problem hiding this comment.
I looked at the latest changes and see the browser tests are gone. Looks good to me! 👍
3 participants
Before (the change)
The checkbox label and the radio item label rendered differently even though they are the same kind of form control label, side by side:
--goa-checkbox-color-label) but regular weight (400). Thefontshorthand from--goa-checkbox-label-font-sizeresolves to the regular body token, so the label was regular.So each was wrong on a different axis.
After (the change)
Both labels now render in medium weight (500) and resolve their color the same way, through their own component token pointing at the shared secondary input text color.
Checkbox.svelte,.textand.v2.compact .text): addedfont-weight: var(--goa-font-weight-medium). Thefontshorthand resets weight to the token default, so it has to be restated, including on the compact label whose shorthand also resets it.RadioItem.svelte,.label): addedcolor: var(--goa-radio-color-label, var(--goa-input-color-text-secondary)). This introduces a new--goa-radio-color-labelcomponent token (to be defined the same way as--goa-checkbox-color-labelin the tokens package) and falls back to the shared--goa-input-color-text-secondaryso the color resolves correctly today, matching the repo's V2 fallback convention. No color change was needed on the checkbox; it already resolves correctly.This is a CSS only change in the Svelte source, so the React and Angular wrappers did not need updates (wrappers pass attributes through and own no styling).
Make sure that you've checked the boxes below before you submit the PR
Steps needed to test
npm run serve:prs:reactand open the4009 Checkbox and radio label consistencypage from the side menu.npm run serve:prs:angular, same4009page.Tests
checkbox.browser.spec.tsx: asserts the base and compact labels consume the medium font-weight token.radio.browser.spec.tsx: asserts the item label resolves its color through the shared secondary input text token via the new component token's fallback.Fixes #4009
Generated by Claude Code