fix(Icon): enlarge check glyph to fill its viewBox#1068
Conversation
The `check` glyph was small and offset within its 24x24 viewBox, so it rendered noticeably smaller than other icons. Scale the path up ~1.25x about the viewBox center; stroke weight stays at 2. Render the Checkbox's check at 0.8rem (= 1rem / 1.25) so the checkmark inside the box keeps its current size; the indeterminate (minus) icon is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 6c4fe47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Storybook Preview Deployed✅ Preview URL: https://click-29e5m3a1k-clickhouse.vercel.app Built from commit: |
The enlarged `check` glyph renders at a compensated size inside the checkbox, keeping the checkmark's dimensions but with a slightly thinner stroke. Regenerate the affected checked-state snapshots (default, disabled, var1–var6; light + dark) via the Docker Playwright suite. Indeterminate and unchecked states are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // The `check` glyph was scaled up 1.25x at the SVG level; render it | ||
| // here at 1rem / 1.25 = 0.8rem so the checkmark keeps its original | ||
| // size inside the box. `minus` (indeterminate) is unaffected. |
There was a problem hiding this comment.
this comment doesn't carry meaning outside of this PR
“original size” is undefined, “keeps” requires pre-existing knowledge
the comment should state why we need this adjustment rather than why we made it
| // size inside the box. `minus` (indeterminate) is unaffected. | ||
| {...(checked === 'indeterminate' | ||
| ? {} | ||
| : { width: '0.8rem', height: '0.8rem' })} |
There was a problem hiding this comment.
that said, I don't like that we're doing this. Why change the icon size if we need this adjustment? If we are doing this adjustment, why are we not doing it for both check mark and minus? It signals that either one of the icons is incorrect, or the checkbox layout is incorrect. Can this change be made without introducing workarounds for components?
There was a problem hiding this comment.
- we can scale both check and minus
- (need to scale plus then as well)
- we can accept the new checkbox appearance w/o adjusting it
- we can resize the checkbox icon for both icons so that minus is a bit smaller, but the layout is consistent (and probably we'll then see that the minus icon should be scaled to match after all)
There was a problem hiding this comment.
we can also not scale the check mark, I'm looking at other icons, and plus/minus/arrows/etc all have spacings around them in svg, this change makes the check mark a bit of an outlier. It looks bigger. If we're ok with that, or maybe if we plan to migrate them gradually one by one, fine. But maybe we want to make an approach to unify all icons in a batch, to avoid an inconsistent look of similarly-shaped icons.
| before | after |
|---|---|
 |
 |
There was a problem hiding this comment.
@crisalbu this is worth considering. Should we also address these other icons? What about the chevrons?
3 participants
Why?
The
checkicon glyph rendered noticeably smaller than the other icons. Itspath sat offset within the 24×24 viewBox (≈67% width, ≈46% height), leaving a
lot of empty space, so the standalone glyph looked undersized.
How?
checkpath up ~1.25× about the viewBox center so it fills the boxlike the other glyphs (
d="M22 5.125L8.25 18.875L2 12.625"); stroke weightstays at
2.Checkbox's check at0.8rem(=1rem/ 1.25) so the checkmarkinside the box keeps its current size — the check in the checkbox will be slightly thinner. If we don't like that, I can make the stroke weight 2.5 so it visually matches... but that might make it too bold in places.
minus) icon is untouched.Note for reviewers: the icon-gallery visual-regression baseline will change
(intended). The Checkbox baselines should stay unchanged, since the check is
compensated to render at its existing size.
Tickets?
N/A
Contribution checklist?
buildcommand runs locallySecurity checklist?
dangerouslySetInnerHTMLPreview?
Verified in Storybook (Icon gallery + Checkbox stories) and in control-plane via
0.6.1-rc2.Note
Low Risk
Visual-only icon and Checkbox sizing tweaks with no auth, data, or API changes; minor risk of inconsistent check appearance where the icon is used without the Checkbox compensation.
Overview
The standalone
checkicon is enlarged by rescaling its SVG path (~1.25×) so the glyph fills the 24×24 viewBox and aligns visually with other icons, with stroke weight unchanged.Checkboxstill shows the same check size inside the box by rendering the check icon at0.8rem(1rem ÷ 1.25); the indeterminateminusicon is unchanged. A patch changeset documents the@clickhouse/click-uirelease.Icon-gallery visual baselines are expected to change; Checkbox baselines should stay the same.
Reviewed by Cursor Bugbot for commit 6c4fe47. Bugbot is set up for automated code reviews on this repo. Configure here.