Skip to content

chore(Checkbox): migrate to CSS Modules with visual regression baseline#1052

Open
DreaminDani wants to merge 4 commits into
mainfrom
chore/migrate-checkbox-css-modules
Open

chore(Checkbox): migrate to CSS Modules with visual regression baseline#1052
DreaminDani wants to merge 4 commits into
mainfrom
chore/migrate-checkbox-css-modules

Conversation

@DreaminDani
Copy link
Copy Markdown
Contributor

@DreaminDani DreaminDani commented May 27, 2026

Commits

  • test(Checkbox): add visual regression baseline before CSS Modules migration — adds stories, Playwright spec, and fresh PNG snapshots that capture the current rendering.
  • chore(Checkbox): migrate styling from styled-components to CSS Modules — replaces styled-components with .module.css + cva/cn. DOM-identical, byte-for-byte visual parity verified.

Verification

  • yarn test:visual tests/forms/checkbox.spec.ts passes with zero snapshot regenerations (33 tests across light/dark themes, 7 color variants, indeterminate/checked/disabled states, hover/focus, keyboard activation, and label-click toggling)
  • yarn lint:css, yarn lint:code, yarn build all green
  • grep -r "from 'styled-components'" src/components/Checkbox/ empty

Notes

  • New tests/forms/ directory: Checkbox is the first form-input migration; the directory mirrors the Storybook Forms/* category and gives Switch / RadioGroup / other form inputs a home as they migrate.
  • The wrapper class uses a double-class selector (.wrapper.wrapper) so it reliably overrides the underlying FormRoot styled-components rules (align-items: flex-start) regardless of stylesheet insertion order between static CSS Modules and runtime-injected styled-components. Without this boost, the screenshots drift by 1px because the wrapper falls back to flex-start.

Closes CUI-17


Note

Low Risk
Presentation-only refactor on a single form control, guarded by broad visual and a11y tests; wrapper specificity vs FormRoot is the main subtle regression surface.

Overview
Checkbox styling moves from styled-components to Checkbox.module.css, with variant classes driven by cva / cn on the Radix root and a className passthrough. Layout still uses FormRoot (still styled-components); a .wrapper.wrapper specificity boost preserves align-items: center and max-width: fit-content against FormRoot’s default flex-start.

Storybook gains dedicated stories (states, var1–var6, orientation, no label) to feed a new tests/forms/checkbox.spec.ts Playwright suite (light/dark snapshots, hover/focus, a11y). The unit test drops getComputedStyle for disabled cursor in jsdom and asserts disabled / data-disabled instead. A patch changeset records no intended behavior change.

Reviewed by Cursor Bugbot for commit 3a9fd2d. Bugbot is set up for automated code reviews on this repo. Configure here.

@DreaminDani DreaminDani self-assigned this May 27, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 27, 2026

🦋 Changeset detected

Latest commit: 3a9fd2d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clickhouse/click-ui Patch

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

`window.getComputedStyle(checkbox).cursor === 'not-allowed'` relied on
the styled-components runtime injecting a <style> tag that jsdom could
read. CSS Modules import the stylesheet via Vite at build time and jsdom
does not compute its rules, so the assertion now reads 'default'.

Replace the brittle computed-style probe with the semantic intent of the
test: the rendered button is `disabled` and Radix's `data-disabled`
attribute is present. The visual disabled cursor is still covered
byte-for-byte by the Playwright snapshots in
`tests/forms/checkbox.spec.ts`.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the Checkbox component styling from styled-components to CSS Modules while adding Storybook stories and Playwright visual regression coverage to lock in byte-for-byte rendering parity across themes, variants, and states.

Changes:

  • Added a Playwright visual regression suite for Checkbox (light/dark themes, variants, states, interaction snapshots) plus basic accessibility checks.
  • Migrated Checkbox styles from styled-components to Checkbox.module.css, wiring classes via cva/cn.
  • Expanded Storybook stories to cover Checkbox variants, states, and layout permutations, and added a patch changeset entry.

Reviewed changes

Copilot reviewed 6 out of 35 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/forms/checkbox.spec.ts Adds Checkbox visual regression snapshots + basic a11y behavior checks.
src/components/Checkbox/Checkbox.tsx Switches Checkbox styling from styled-components to CSS Modules (cva/cn) and updates DOM styling hooks.
src/components/Checkbox/Checkbox.stories.tsx Adds dedicated stories for Checkbox states/variants/layout used by visual tests.
src/components/Checkbox/Checkbox.module.css New CSS Module implementing former styled-components rules + specificity override for FormRoot.
.changeset/migrate-checkbox-to-css-modules.md Records the migration as a patch (no intended behavior change).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

data-testid="checkbox"
variant={variant}
disabled={disabled}
aria-label={`${label}`}
Copy link
Copy Markdown
Contributor Author

@DreaminDani DreaminDani May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserving this exactly as-is in the migration: the original styled-components Checkbox had the same aria-label={\${label}`}` template-string. Per the migration project's scope rule, ARIA refinements are kept out of the CSS Modules PR so the visual-regression byte-for-byte guarantee holds. Filed as a follow-up

Comment on lines +145 to 149
export const NoLabel: Story = {
args: {
label: 'Accept terms and conditions',
label: undefined,
},
};
Copy link
Copy Markdown
Contributor Author

@DreaminDani DreaminDani May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NoLabel story is intentionally exercising the current (broken) rendering so the visual-regression snapshot captures it. The underlying aria-label="undefined" bug is a pre-existing styled-components behavior the migration preserves; fixing the story without first fixing the component would change what the snapshot tests. The fix will land separately and the story can be updated alongside it.

Comment on lines +423 to +429
it('disabled checkbox is not focusable via Tab', async ({ page }) => {
await page.goto(getStoryUrl('forms-checkbox--disabled', 'light'), {
waitUntil: 'networkidle',
});
const checkbox = page.getByRole('checkbox');
await expect(checkbox).toBeDisabled();
});
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in the follow-up commit by adding the actual Tab navigation assertion (page.keyboard.press('Tab')expect(checkbox).not.toBeFocused()).

The "disabled checkbox is not focusable via Tab" test had a misleading
name — it only checked `toBeDisabled()` without exercising Tab. Add the
Tab keypress and assert the checkbox does not receive focus so the test
matches its stated behavior.
@workflow-authentication-public
Copy link
Copy Markdown
Contributor

Storybook Preview Deployed

✅ Preview URL: https://click-2z3d9a7ps-clickhouse.vercel.app

Built from commit: 6a5f2b2d8de3fa680a32501f6773206b0c26c87d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants