test(IconButton): 💍 visual regression

[punkbit]

Why?

Introduce visual regression tests for ButtonGroup, as a prerequisite for migrating styled components into CSS modules.

How?

• Extend IconButton storybook stories
• Introduce IconButton test spec for visual regression
• Store snapshots

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?

[changeset-bot]

⚠️ No Changeset found

Latest commit: d6d10fa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

Claude finished @punkbit's task in 2m 25s —— View job

---

Code Review

• Read conventions files
• Review IconButton.stories.tsx changes
• Review iconbutton.spec.ts test spec
• Check snapshot coverage and consistency

---

Nice work — the coverage is solid, the story additions are well-structured, and the pattern follows button.spec.ts closely. A few things worth addressing before merge:

---

🚨 Must Fix

• iconbutton.spec.ts line 447-452 — missing aria-disabled assertion: The disabled test only checks toBeDisabled(), but button.spec.ts (line 534-535) also asserts aria-disabled="true". Per CONVENTIONS.md: "Disabled states need both disabled prop AND aria-disabled="true"". Adding this assertion is the right call — and it will reveal a pre-existing gap in the component: IconButton.tsx (line 15) passes disabled={disabled} but never sets aria-disabled.

  The fix in the test:

  await expect(button).toBeDisabled();
  await expect(button).toHaveAttribute('aria-disabled', 'true'); // add this

  And in the component (IconButton.tsx line 15):

  disabled={disabled}
  aria-disabled={disabled}  // add this

  Fix this →

---

♿ Accessibility (pre-existing, not introduced by this PR — worth a follow-up issue)

• IconButton.tsx line 57 — &:focus should be &:focus-visible: Mouse clicks also trigger the active focus ring. CONVENTIONS.md requires focus-visible to avoid unwanted focus outlines on click.
• IconButton.tsx line 71-75 — disabled state missing border-color: The disabled style sets background-color and color but leaves border-color unchanged, so the variant's active stroke color bleeds through.
• IconButton.tsx line 17 — redundant role="button": A <button> element has this role implicitly; the explicit attribute is harmless but unnecessary.

---

💡 Suggestions (non-blocking)

• iconbutton.spec.ts lines 424-443 — over-complex click verification: The click counter injected via page.evaluate with document.querySelector('[role="button"]') is fragile if the page has multiple button elements. The existing button.spec.ts test just does await button.click() without verification — if you want to assert the handler fired, the cleaner approach is checking the Storybook actions panel or a data-testid attribute. Either simplify to match button.spec.ts, or keep the verification but use the Playwright button locator reference instead of re-querying the DOM.

• Duplicate snapshots for DefaultSize + Primary: Both stories render a primary button with default size, producing identical snapshots (same SHA: f9101145... for dark, 9da06413... for light). Not a bug, but DefaultSize could be an AllSizes comparison story to add more value.

• page.waitForTimeout(100) in interactive state tests: Consistent with button.spec.ts so it's fine as-is, but fixed delays can be flaky on slow CI. A page.waitForFunction or animations: 'disabled' approach is more resilient for transitions.

---

[github-actions]

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

---

Code Review

• Read CONVENTIONS.md and CLAUDE.md
• Review IconButton component
• Review stories changes
• Review test spec
• Post review findings

---

Great foundation — the spec structure is clean, coverage across themes/variants/sizes/states is thorough, and the pattern mirrors button.spec.ts well. Two items need addressing before merge.

---

🚨 Must Fix

• IconButton.tsx:15 — missing aria-disabled: Every other interactive component in the codebase sets this (Button.tsx:77, CardSecondary.tsx:116, CardHorizontal.tsx:219, etc.) and CONVENTIONS.md mandates it. The component only passes disabled={disabled} but omits aria-disabled:

  // IconButton.tsx, inside the JSX return
  disabled={disabled}
  aria-disabled={disabled || undefined}  // add this line

  Fix this →

• iconbutton.spec.ts:438 — disabled test missing aria-disabled assertion: button.spec.ts:534 asserts both toBeDisabled() and toHaveAttribute('aria-disabled', 'true'). This test only has the former:

  await expect(button).toBeDisabled();
  await expect(button).toHaveAttribute('aria-disabled', 'true'); // add this

  Fix this →

---

♿ Accessibility (pre-existing in IconButton.tsx — worth a follow-up issue)

• IconButton.tsx:57 — &:focus should be &:focus-visible: Pointer clicks also trigger the focus ring. CONVENTIONS.md requires focus-visible to limit outlines to keyboard navigation only.
• IconButton.tsx:71-74 — disabled style missing border-color: The disabled block sets background-color and color but leaves border-color unset, so the active variant stroke bleeds through on disabled buttons.
• IconButton.tsx:17 — redundant role="button": <button> already has this role implicitly.

---

💡 Suggestions (non-blocking)

• Interactive story uses console.log: The tests rely on console.log('clicked') appearing in console output. Consider using Storybook's fn() from @storybook/test (consistent with how other stories in the library handle action logging) — it's more resilient and integrates with the Storybook actions panel.
• DefaultSize story is a duplicate of Primary: Both produce identical snapshots (same SHA). Renaming it to AllSizes with a wrapper rendering all three sizes side-by-side would add meaningful visual regression value.

[workflow-authentication-public]

📚 Storybook Preview Deployed

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

Built from commit: 139d6360f0e83db963063b78b09f687bec66d2b2