Skip to content

chore(CardHorizontal): migrate to CSS Modules with visual regression baseline#1039

Open
DreaminDani wants to merge 2 commits into
mainfrom
chore/migrate-cardhorizontal-css-modules
Open

chore(CardHorizontal): migrate to CSS Modules with visual regression baseline#1039
DreaminDani wants to merge 2 commits into
mainfrom
chore/migrate-cardhorizontal-css-modules

Conversation

@DreaminDani
Copy link
Copy Markdown
Contributor

Summary

Migrates CardHorizontal from styled-components to CSS Modules (cva + cn), following the procedure established by ButtonGroup (PR #1034), IconButton (PR #1036), and CardPrimary (PR #1038) and codified in the `component-css-modules-migration` skill.

Two commits, each green on its own:

  • `test(CardHorizontal): add visual regression baseline before CSS Modules migration` — extends the stories with one named story per visual variant (color, size, alignment, selectable, selected, disabled, disabled+selected, with-badge, with-button) and adds `tests/cards/cardhorizontal.spec.ts`. 28 snapshots captured against the existing styled-components rendering, covering light + dark themes plus hover/focus interactive states on both selectable and non-selectable cards. The stories-only `GridCenter` decorator (a Storybook display helper) is converted from `styled.div` to inline style in the same commit to keep the directory free of `styled-components` after the migration.
  • `chore(CardHorizontal): migrate styling from styled-components to CSS Modules` — adds `CardHorizontal.module.css` consolidating six styled-components (Wrapper, Header, Description, CardIcon, ContentWrapper, IconTextContentWrapper) into one stylesheet. Re-asserts the baseline snapshots byte-for-byte with zero regenerations.

The migration is a pure styling refactor — no a11y refinements, no type changes, no consumer updates.

Judgment calls to flag

  1. Local CSS variables for color variants. `CardHorizontal` has two color variants (`default`, `muted`) and each appears in four states (default, hover, active/focus, disabled) across four token families (background, title, stroke, description). Instead of emitting a full state matrix per color (≈80 rules), I exposed the per-color tokens as locally-scoped CSS custom properties (`--card-bg-default`, `--card-stroke-active`, etc.) on `.wrapper_color_default` and `.wrapper_color_muted`, then write the state rules once against `var(--card--)`. This kept the stylesheet compact without changing any rendered behavior — confirmed by zero snapshot regeneration.

  2. Scoped `stylelint-disable no-descending-specificity` around the disabled-state block. The disabled rules intentionally come after hover/active rules to mirror the source cascade order. `pointer-events: none` plus `tabIndex=-1` on disabled cards prevent the hover/active rules from ever firing, so the lint complaint is structural rather than behavioral. Scoped narrowly to that block.

  3. Scoped `stylelint-disable-next-line media-feature-range-notation` on the responsive `@media`. The repo's stylelint-config-standard expects modern range notation (`(width <= 768px)`), but `plugin/no-unsupported-browser-features` flags that syntax as unsupported in Edge 102–103, Chrome 102–103, Safari 15.6, etc. The two rules conflict; the prefix notation (`(max-width: 768px)`) is required for browser compatibility per `.browserslistrc`. Single-line disable.

Test plan

  • `yarn test:visual tests/cards/cardhorizontal.spec.ts` — all 28 snapshots pass against the baseline with zero regenerations after the migration commit.
  • `yarn test CardHorizontal` — unit tests (18) pass unchanged.
  • `yarn lint:code src/components/CardHorizontal/` — no new errors.
  • `yarn lint:css` — clean (with the scoped disables noted above).
  • `yarn build` — succeeds.
  • `grep -r 'styled-components' src/components/CardHorizontal/` — empty.

🤖 Generated with Claude Code

DreaminDani and others added 2 commits May 19, 2026 18:45
…es migration

Captures the current styled-components rendering for color variants
(default, muted), size (md, sm), alignment (top), selectable/selected,
disabled (with and without isSelected), badge, button, and the hover
and focus interactive states (on both selectable and non-selectable
cards) under both light and dark themes. These snapshots will be
re-asserted byte-for-byte after the CSS Modules migration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Modules

Replaces six styled.div templates (Header, Description, Wrapper, CardIcon,
ContentWrapper, IconTextContentWrapper) with a single CardHorizontal.module.css
+ cva/cn. Per-color tokens are exposed as local CSS variables so the state
rules (default, hover, active/focus/focus-within, disabled) are written once
and consumed by both the default and muted color variants. The DOM tree and
every visual state are preserved byte-for-byte against the snapshots
captured in the baseline commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 20, 2026

🦋 Changeset detected

Latest commit: f7558ef

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

@workflow-authentication-public
Copy link
Copy Markdown
Contributor

Storybook Preview Deployed

✅ Preview URL: https://click-48rdhyimj-clickhouse.vercel.app

Built from commit: 58d8110dfac3a22e65a76e490c0192503061438f

@DreaminDani
Copy link
Copy Markdown
Contributor Author

Scoped stylelint-disable-next-line media-feature-range-notation on the responsive @media. The repo's stylelint-config-standard expects modern range notation ((width <= 768px)), but plugin/no-unsupported-browser-features flags that syntax as unsupported in Edge 102–103, Chrome 102–103, Safari 15.6, etc. The two rules conflict; the prefix notation ((max-width: 768px)) is required for browser compatibility per .browserslistrc. Single-line disable.

These rules feel like they conflict with one another. I don't think we should use the width <= 768px if it's going to cause all this extra rule disabling inline.

font: var(--click-card-horizontal-typography-title-default);
}

.wrapper_color_default {
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.

I kinda like this pattern. Should we codify it into the skill?

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.

1 participant