From e9423adf5996754e0296e3882e9d64037b6d94ff Mon Sep 17 00:00:00 2001 From: Andrey Goder Date: Wed, 5 Aug 2026 16:32:11 -0700 Subject: [PATCH] Center the indeterminate checkbox mark (#376) The checked and indeterminate marks shared the icon slot's `mt: '1px'`, an optical correction shaped for the lucide Check glyph, whose stroke sits above the middle of its viewBox. The Minus bar is already on that centre line, so the shared nudge pushed it 0.5px below the box's centre at every size (a flex-centred item with a 1px top margin shifts by half the margin). Replace the `isChecked` variant, which collapsed both marked states, with a three-state `mark` variant. Both marked states still paint the same filled box; only `check` carries the nudge. Measured in headless Chromium against Storybook, comparing the glyph's painted bounding box to the box's centre at sm/md/lg: indeterminate goes from +0.5px to 0px, checked stays at +0.083px. Claude-Session: https://claude.ai/code/session_01XFkkvUQfynGRedEYDNxKoY --- .../CheckboxInput/CheckboxInput.recipe.ts | 19 ++++++--- .../CheckboxInput/CheckboxInput.test.tsx | 41 +++++++++++++++++++ .../CheckboxInput/CheckboxInput.tsx | 2 +- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/components/CheckboxInput/CheckboxInput.recipe.ts b/src/components/CheckboxInput/CheckboxInput.recipe.ts index 7677e81..3684b6f 100644 --- a/src/components/CheckboxInput/CheckboxInput.recipe.ts +++ b/src/components/CheckboxInput/CheckboxInput.recipe.ts @@ -1,5 +1,8 @@ import {sva, type RecipeVariantProps} from 'styled-system/css'; +// Both marked states paint the same filled box; only the glyph inside differs. +const markedBox = {bg: 'primary', borderColor: 'primary'}; + export const checkboxInputRecipe = sva({ slots: ['root', 'boxWrap', 'input', 'box', 'icon', 'label', 'tooltipIcon'], base: { @@ -44,7 +47,6 @@ export const checkboxInputRecipe = sva({ icon: { w: '70%', h: '70%', - mt: '1px', }, label: { display: 'inline-flex', @@ -64,9 +66,16 @@ export const checkboxInputRecipe = sva({ md: {box: {w: '5.5', h: '5.5'}}, lg: {box: {w: '6.5', h: '6.5'}}, }, - isChecked: { - true: {box: {bg: 'primary', borderColor: 'primary'}}, - false: {}, + mark: { + none: {}, + // The lucide Check glyph's stroke sits above the middle of its viewBox, + // so the checked mark needs an optical nudge down to read as centered. + // The nudge belongs to this glyph alone — the spacing scale bottoms out + // at 0.5 (2px), so there is no token for it. + check: {box: markedBox, icon: {mt: '1px'}}, + // The Minus bar is already on the viewBox's center line; nudging it + // would push it off the box's center instead of onto it. + indeterminate: {box: markedBox}, }, isDisabled: { true: { @@ -78,7 +87,7 @@ export const checkboxInputRecipe = sva({ }, defaultVariants: { size: 'md', - isChecked: false, + mark: 'none', isDisabled: false, }, }); diff --git a/src/components/CheckboxInput/CheckboxInput.test.tsx b/src/components/CheckboxInput/CheckboxInput.test.tsx index d8422fe..a563083 100644 --- a/src/components/CheckboxInput/CheckboxInput.test.tsx +++ b/src/components/CheckboxInput/CheckboxInput.test.tsx @@ -27,6 +27,47 @@ describe('CheckboxInput', () => { ); }); + it('keeps the checked glyph nudge off the indeterminate mark', () => { + // The lucide Check glyph needs a 1px nudge to look centered; the Minus bar + // is already centered, so inheriting the nudge pushes it below the box's + // center line. + const nudge = css({mt: '1px'}); + const {container, rerender} = render( + {}} value />, + ); + + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- the mark is intentionally hidden from the accessibility tree + expect(container.querySelector('.lucide-check')).toHaveClass(nudge); + + rerender( + {}} value="indeterminate" />, + ); + + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- the mark is intentionally hidden from the accessibility tree + expect(container.querySelector('.lucide-minus')).not.toHaveClass(nudge); + }); + + it('fills the box for both the checked and indeterminate marks', () => { + const filled = css({bg: 'primary'}); + const {container, rerender} = render( + {}} value />, + ); + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- the box is intentionally hidden from the accessibility tree + const box = (): Element | null => container.querySelector('[aria-hidden]'); + + expect(box()).toHaveClass(filled); + + rerender( + {}} value="indeterminate" />, + ); + + expect(box()).toHaveClass(filled); + + rerender( {}} value={false} />); + + expect(box()).not.toHaveClass(filled); + }); + it('renders React nodes in the label', () => { render(