chore(IconButton): 🤖 Migrate component to CSS Modules

[punkbit]

Why?

Migrate the component ButtonGroup from Styled-Components to CSS Modules.

⚠️ Depends on #990 (should merge first)

How?

• Migrates Styled Components styles onto CSS Modules
• Use utility CVA for variants
• Use CSS Variables for tokens

Tickets?

N/A

Contribution checklist?

• You've done enough research before writing
• You have reviewed the PR
• The commit messages are detailed
• The build command runs locally
• Assets or static content are linked and stored in the project
• For documentation, guides or references, you've tested the commands

Preview?

N/A

[changeset-bot]

🦋 Changeset detected

Latest commit: 7392c94

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	Minor

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

[github-actions]

Claude finished @punkbit's task in 1m 46s —— View job

---

PR Review: chore(IconButton) — Migrate component to CSS Modules

• Gather context
• Read changed files (IconButton.tsx, IconButton.module.css, tests)
• Check if previous review issues were addressed
• Post final review

---

Great work across multiple iterations — this push clears all outstanding must-fix items. The component is in solid shape.

---

✅ Previously flagged — all fixed

Issue	Status
transition only on hover-in	✅ Fixed — on base .icon-button rule (line 14)
@media (prefers-reduced-motion) had no effect	✅ Fixed
Missing aria-disabled	✅ Fixed — line 39
Missing type="button"	✅ Fixed — line 34
outline: none without comment	✅ Fixed
aria-label non-overridable	✅ Fixed — now placed before {...props} (line 35), consumers can override

---

💡 Suggestions (non-blocking)

• IconButton.tsx:42 — <Icon> is missing aria-hidden: The button already has aria-label={iconName} which provides the accessible name. Adding aria-hidden={true} to the inner <Icon> prevents some screen readers from double-announcing the icon's SVG content alongside the button label. This is standard practice for icon-only buttons. Fix this →

• IconButton.test.tsx:39 — disabled test missing aria-disabled assertion: expect(button).toBeDisabled() is there but expect(button).toHaveAttribute('aria-disabled', 'true') is not. Since aria-disabled was explicitly added, locking it in a test prevents future regressions.

• IconButton.types.ts:6 — HTMLAttributes vs ButtonHTMLAttributes (pre-existing): HTMLAttributes<HTMLButtonElement> omits button-specific props (form, formAction, formMethod, name, value). ButtonHTMLAttributes<HTMLButtonElement> is a more accurate base.

• Changeset version: Marked minor, but per CONVENTIONS.md (patch = fix, minor = feature), a non-breaking internal style refactor is a patch. Nothing in the public API has changed.

---

[github-actions]

Claude finished @punkbit's task in 1m 58s —— View job

---

PR Review: chore(IconButton) — Migrate component to CSS Modules

• Gather context
• Read CLAUDE.md and conventions
• Read changed files (IconButton.tsx, IconButton.module.css, IconButton.types.ts, tests, Icon.tsx)
• Post final review

---

Great migration overall — the CSS, BEM naming, token usage, and interactive states all look solid. One accessibility item needs to land before merge.

---

🚨 Must Fix

IconButton.tsx:42 — <Icon> is missing aria-hidden={true}

CONVENTIONS.md explicitly requires: "Icons must have aria-hidden unless they're the only content."

More critically, reading Icon.tsx:112 shows that <Icon> internally sets role="img" and aria-label={name} on the SVG wrapper for every standard icon. This means the current implementation causes screen readers to double-announce:

1. The button: "user, button" (from aria-label={iconName} on the <button>)
2. The icon inside: "user, image" (from role="img" + aria-label={name} added by <Icon>)

Fix:

<Icon
  name={icon}
  size="sm"
  aria-hidden={true}
/>

Fix this →

---

💡 Suggestions (non-blocking)

• IconButton.test.tsx:39 — aria-disabled not asserted in the disabled test: expect(button).toBeDisabled() is there but expect(button).toHaveAttribute('aria-disabled', 'true') is missing. CONVENTIONS.md says "Verify accessibility attributes exist" in tests. Since aria-disabled was explicitly added to the implementation, a regression test for it is worth adding.

• .changeset/migrate-iconbutton-css-modules.md — marked minor, should be patch: Per CONVENTIONS.md: "patch (fix), minor (feature), major (breaking)". This migration has no public API changes — it's an internal implementation detail. A patch bump is the correct semver signal here.

• IconButton.types.ts:6 — pre-existing: HTMLAttributes vs ButtonHTMLAttributes: HTMLAttributes<HTMLButtonElement> omits form, formAction, formMethod, name, value, and the native type attribute. ButtonHTMLAttributes<HTMLButtonElement> is a more accurate base. Note: if you switch, the variant type prop would shadow the native type — consider renaming the variant prop to variant to avoid the clash and match the convention used in most component libraries.

---

[github-actions]

Claude finished @punkbit's task in 1m 54s —— View job

---

PR Review: chore(IconButton) — Migrate component to CSS Modules

• Gather context
• Read CLAUDE.md and CONVENTIONS.md
• Read changed files (IconButton.tsx, IconButton.module.css, tests, Icon.tsx)
• Check if previous review issues were addressed
• Post final review

---

The CSS, BEM naming, token usage, transition, reduced-motion, focus-visible, and attribute ordering are all solid. One accessibility item from the last two reviews is still open and needs to land before merge.

---

✅ Previously flagged — fixed

Issue	Status
transition only on hover-in	✅ Fixed
@media (prefers-reduced-motion) had no effect	✅ Fixed
Missing aria-disabled	✅ Fixed (line 39)
Missing type="button"	✅ Fixed (line 34)
outline: none without comment	✅ Fixed
aria-label non-overridable	✅ Fixed (placed before {...props})

---

🚨 Must Fix (flagged in the previous two reviews — still not addressed)

IconButton.tsx:42 — <Icon> is missing aria-hidden={true}

CONVENTIONS.md mandates: "Icons must have aria-hidden unless they're the only content."

Looking at Icon.tsx:108-115, every standard icon variant renders with role="img" and aria-label={name} applied to its wrapper <div>. This causes screen readers to double-announce:

1. The button: "user, button" (from aria-label={iconName} on <button>)
2. The icon wrapper: "user, image" (from role="img" + aria-label={name} inside <Icon>)

Fix:

<Icon
  name={icon}
  size="sm"
  aria-hidden={true}
/>

Fix this →

---

💡 Suggestions (non-blocking)

• IconButton.test.tsx:39 — aria-disabled not asserted: The test asserts toBeDisabled() but not toHaveAttribute('aria-disabled', 'true'). CONVENTIONS.md says "Verify accessibility attributes exist" — since aria-disabled was explicitly added to the implementation, a regression test for it is worth adding.

• .changeset/migrate-iconbutton-css-modules.md — minor should be patch: Per CONVENTIONS.md: "patch (fix), minor (feature), major (breaking)". This migration has zero public API changes. A patch is the correct semver signal.

• IconButton.types.ts:6 — pre-existing: HTMLAttributes vs ButtonHTMLAttributes: HTMLAttributes<HTMLButtonElement> omits button-specific props (form, formAction, formMethod, name, value). ButtonHTMLAttributes<HTMLButtonElement> is more accurate. Note: if you switch, the variant type prop would shadow the native type — consider renaming the variant prop to variant to match the convention used in most component libraries and avoid the clash.

---

[workflow-authentication-public]

📚 Storybook Preview Deployed

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

Built from commit: 767c59c2680a889ac549037088467ca90c2e0b97