Skip to content

fix(CodeBlock): Show Action Buttons on Hover#989

Open
dustinhealy wants to merge 6 commits intochore/v0.2.0from
fix/codeblock-button-overlap
Open

fix(CodeBlock): Show Action Buttons on Hover#989
dustinhealy wants to merge 6 commits intochore/v0.2.0from
fix/codeblock-button-overlap

Conversation

@dustinhealy
Copy link
Copy Markdown
Collaborator

Why?

CodeBlock action buttons (copy, wrap lines) are positioned absolutely over the code content, causing text overlap on long first lines. This is especially noticeable when both buttons are visible.

How?

  • Buttons are now hidden by default (opacity: 0) and fade in on hover or focus-within (opacity: 1 with 0.15s ease transition)
  • Buttons get a semi-opaque background pill matching the code block's theme background, so any code behind them is obscured when visible
  • Small border-radius and padding on the button container for a clean pill appearance

Contribution checklist?

  • You've done enough research before writing
  • You have reviewed the PR

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 13, 2026

🦋 Changeset detected

Latest commit: e615c4f

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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Claude finished @dustinhealy's task in 1m 27s —— View job


Code Review: fix(CodeBlock): Show Action Buttons on Hover

  • Gather context and read changed files
  • Review code changes
  • Post findings

Nice approach overall — hiding buttons by default and fading them in on hover/focus is a clean UX improvement. A few things need attention before merging:


🚨 Must Fix

  • CodeBlock.tsx (line 95): transition: opacity 0.15s ease doesn't respect prefers-reduced-motion. Per CONVENTIONS.md: "Respect prefers-reduced-motion for animations". This is mandatory for the component library. Add:

    @media (prefers-reduced-motion: reduce) {
      transition: none;
    }

    Fix this →

  • Changeset description mismatch: .changeset/fix-codeblock-button-overlap.md says "Adds right padding to the highlighted code area proportional to the number of action buttons displayed" — that's from a different implementation strategy, not what this PR does. Update it to describe the actual change (buttons hidden by default, revealed on hover/focus-within with semi-opaque background). Fix this →

  • CodeBlock.tsx (line 96–97): border-radius: 0.25rem and padding: 0.25rem are hardcoded. Conventions require all sizing values to come from theme tokens. The codeblock theme already has theme.click.codeblock.radii.all (0.25rem) — use that. Fix this →


💡 Suggestions (non-blocking)

  • ${bg}e6 hex concatenation (CodeBlock.tsx line 105): Appending e6 to produce an 8-digit hex opacity works for the current tokens (all 6-digit hex values like #282828 and #f6f7fa), but it would silently break if any token were ever expressed as rgb(...), hsl(...), or a 3-digit hex. Consider color-mix(in srgb, ${bg} 90%, transparent) for a more robust approach — it's widely supported in modern browsers.

  • The ButtonContainer buttons stay in the tab order even at opacity: 0. The focus-within trick correctly surfaces them when keyboard-focused, so this is fine for accessibility — just confirming it was intentional.


@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Claude finished @dustinhealy's task in 1m 0s —— View job


Reviewing PR: fix(CodeBlock): Show Action Buttons on Hover

  • Gather context and read changed files
  • Review code changes
  • Post findings

Nice approach — hiding the buttons by default and fading them in on hover/focus-within is clean and solves the overlap well. A few things to address:

🚨 Must Fix

  • CodeBlock.tsx line 57 — Fragile hex alpha concatenation: background: ${bg}e6 appends e6 directly to the token value. This assumes the token is always a 6-digit hex string — if it ever resolves to rgb(...), hsl(...), or a named color it will produce invalid CSS silently. Use CSS color-mix() or opacity on a wrapper instead:
    background: ${bg};
    opacity: 0.9; // on a wrapper, not the container with opacity:0
    Or, if styled-components is available, wrap in a separate element without the opacity animation, or check if the design system provides an --alpha token. Fix this →

♿ Accessibility

  • Missing prefers-reduced-motion support (CONVENTIONS.md mandates it): The transition: opacity 0.15s ease will animate for users who've opted out of motion. Add:
    @media (prefers-reduced-motion: reduce) {
      transition: none;
    }
    Fix this →

💡 Suggestions (non-blocking)

  • Hardcoded spacing (border-radius: 0.25rem, padding: 0.25rem): Per CONVENTIONS.md, spacing/sizes should come from theme.click.* tokens rather than hardcoded rem values. Worth checking if the design system has a relevant radius or space token.

  • Changeset description mismatch: The changeset says "Adds right padding to the highlighted code area proportional to the number of action buttons displayed" — but the actual implementation uses hover-reveal with a background pill, not padding. Worth updating it to accurately describe what shipped (helps with changelogs downstream).

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Claude finished @dustinhealy's task in 1m 29s —— View job


Code Review: fix(CodeBlock): Show Action Buttons on Hover

  • Gather context and read changed files
  • Read CONVENTIONS.md and repo guidelines
  • Review code changes
  • Post findings

Nice approach — hiding the buttons by default and fading them in on hover/focus-within is clean and solves the overlap well. Three things need to be addressed before merging:


🚨 Must Fix

  • CodeBlock.tsx line 47 — Missing prefers-reduced-motion support: transition: opacity 0.15s ease has no reduced-motion override. CONVENTIONS.md mandates: "Respect prefers-reduced-motion for animations." Add inside ButtonContainer:

    @media (prefers-reduced-motion: reduce) {
      transition: none;
    }

    Fix this →

  • CodeBlock.tsx line 48 — Hardcoded border-radius: border-radius: 0.25rem should be border-radius: ${({ theme }) => theme.click.codeblock.radii.all} — the token already exists and equals 0.25rem. CONVENTIONS.md: "NEVER hardcode colors, sizes, or spacing." Fix this →

  • Stale changeset description in .changeset/fix-codeblock-button-overlap.md: Still says "Adds right padding to the highlighted code area proportional to the number of action buttons displayed" — that's the old discarded approach. Update to accurately describe what shipped (buttons hidden by default, revealed on hover/focus-within with a semi-opaque background). Fix this →


💡 Suggestions (non-blocking)

  • ${bg}e6 hex concatenation (CodeBlock.tsx line 57): Works correctly for the current tokens (#282828, #f6f7fa), but would silently produce invalid CSS if any token were ever expressed as rgb(...), hsl(...), or a 3-digit hex. Consider color-mix(in srgb, ${bg} 90%, transparent) as a more robust alternative — it's well-supported in modern browsers.

  • padding: 0.25rem (CodeBlock.tsx line 49): No exact codeblock space token for this value. Worth checking if a general space token covers it, or consider adding a codeblock-specific token (e.g., theme.click.codeblock.space.buttonPadding) if the design team wants to control it.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Claude finished @dustinhealy's task in 52s —— View job


Code Review: fix(CodeBlock): Show Action Buttons on Hover

  • Gather context and read changed files
  • Review latest code changes
  • Post findings

All the feedback from previous reviews has been addressed — nice follow-through! 🙌

✅ Previously flagged issues — now fixed

  • prefers-reduced-motion: @media (prefers-reduced-motion: reduce) { transition: none; } added correctly (lines 48–50)
  • border-radius: now ${theme.click.codeblock.radii.all} (line 56) ✅
  • padding: now ${theme.sizes[1]} (line 57) ✅
  • Hex alpha hack: replaced with color-mix(in srgb, ${bg} 90%, transparent) (line 60) ✅
  • Changeset description: accurately reflects the actual implementation ✅

💡 Suggestion (non-blocking)

  • gap: 0.625rem (CodeBlock.tsx line 55): This is the one remaining hardcoded size. CONVENTIONS.md says "NEVER hardcode colors, sizes, or spacing." Worth replacing with a theme token if one exists (e.g., theme.click.codeblock.space.* or a general space token), or adding one. Low priority since it doesn't affect correctness.

Everything else looks great. The hover-reveal approach is clean, keyboard accessible via focus-within, and the color-mix() background is robust. Ready to merge once the gap is considered (or consciously left as-is).

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Claude finished @dustinhealy's task in 1m 23s —— View job


Code Review: fix(CodeBlock): Show Action Buttons on Hover

  • Gather context and read changed files
  • Review latest code changes
  • Post findings

All previous blockers have been resolved — great follow-through! 🙌

✅ Previously flagged issues — now fixed

  • prefers-reduced-motion: @media (prefers-reduced-motion: reduce) { transition: none; } added ✅
  • border-radius: now theme.click.codeblock.radii.all
  • padding: now theme.sizes[1]
  • Hex alpha hack: replaced with color-mix(in srgb, ${bg} 90%, transparent)
  • Changeset description: accurately reflects the actual implementation ✅

💡 Suggestion (non-blocking)

  • gap: 0.625rem (CodeBlock.tsx line 55): One remaining hardcoded size. No exact match exists in theme.sizes (sizes[3] is 0.5rem, sizes[4] is 0.75rem) and theme.click.codeblock.space.gap (1.5rem) serves a different purpose. Options: use the nearest token, or add a dedicated theme.click.codeblock.space.buttonGap token. Either way, low priority — doesn't affect correctness or behavior.

Everything else looks solid. The hover-reveal is clean, keyboard-accessible via focus-within, and the color-mix() background is robust. Ready to merge once the gap is addressed or consciously left as-is.

Add padding-left and background: inherit to the button container
so buttons have a solid backdrop and don't overlap the code text.
Action buttons (copy, wrap) are now hidden by default and fade in
on hover or focus-within. Buttons have a semi-opaque background
matching the code block theme to prevent overlap with code content.
@dustinhealy dustinhealy force-pushed the fix/codeblock-button-overlap branch from 44309bf to e615c4f Compare April 13, 2026 23:45
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Claude finished @dustinhealy's task in 2m 9s —— View job


Code Review: fix(CodeBlock): Show Action Buttons on Hover

  • Gather context and read changed files
  • Review code changes
  • Post findings

Great follow-through on the previous feedback — the core implementation is solid. One potential bug slipped through, and there's one remaining hardcoded value worth addressing.


🐛 Possible Bug

  • CodeBlock.tsx line 57 — Wrong sizes index for padding: ${theme.sizes[1]} resolves to '1px' (not '0.25rem'). The sizes token array is {'0': '0', '1': '1px', '2': '0.25rem', ...} — so the original padding: 0.25rem intent maps to theme.sizes[2]. With 1px, the semi-opaque pill background will be nearly flush with the icon buttons, losing the intended breathing room. Fix this →

💡 Suggestions (non-blocking)

  • gap: 0.625rem (CodeBlock.tsx line 55): Still the one remaining hardcoded size — no exact token maps to 10px (sizes[3] = 0.5rem, sizes[4] = 0.75rem). Nearest fit is theme.sizes[4] (0.75rem) or a dedicated theme.click.codeblock.space.buttonGap token, but consciously leaving it is also fine.

  • No tests for new hover/focus behavior: There's no CodeBlock.test.tsx (pre-existing gap), but the new opacity/focus-within logic is a good candidate for a quick accessibility smoke test.


✅ Previously flagged — all confirmed fixed

  • prefers-reduced-motion media query ✅
  • border-radius using theme.click.codeblock.radii.all
  • color-mix(in srgb, ${bg} 90%, transparent) replacing the fragile hex concatenation ✅
  • Changeset description accurately describes the implementation ✅
  • top/right positioning using theme.click.codeblock.space.y/x

@dustinhealy dustinhealy changed the base branch from main to chore/v0.2.0 April 13, 2026 23:46
@workflow-authentication-public
Copy link
Copy Markdown
Contributor

📚 Storybook Preview Deployed

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

Built from commit: ad67d9fc4ee2af5bd966076a8bbf7de83a1443f6

@punkbit
Copy link
Copy Markdown
Contributor

punkbit commented Apr 14, 2026

@dustinhealy Do me a favour. Change target branch to "main". We prefer to push changes to "main" as the source of truth. If a pre-release is open, we then cherry-pick; this way, we don't lose track of any changes :)

@danielclickh
Copy link
Copy Markdown

Thanks for the work on this @dustinhealy ! I do have two suggestions to help with accessibility:

  1. Make it sure it's visible when the codeblock is active (this way keyboard users can see that)
  2. Add ARIA labels

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.

3 participants