Skip to content

fix: consistent Copied feedback on every copy button - #61

Merged
marekdano merged 3 commits into
mainfrom
44-copied-feedback-on-copy-button
Aug 21, 2026
Merged

fix: consistent Copied feedback on every copy button#61
marekdano merged 3 commits into
mainfrom
44-copied-feedback-on-copy-button

Conversation

@marekdano

Copy link
Copy Markdown
Contributor

Closes #44

Summary

Copy-to-clipboard buttons existed in 17 places behaving four different ways — most gave no signal that a click did anything, including TokenCreatedDialog, where a copy failure is costliest since the token is shown exactly once.

This consolidates all of them onto two new shared primitives:

  • useCopyToClipboard (src/hooks/useCopyToClipboard.ts) — owns the clipboard write, transient idle/copied/error status, reset timeout, and unmount cleanup.
  • CopyButton (src/components/ui/copy-button.tsx) — the icon button: a hybrid tooltip that stays uncontrolled on hover/focus (so the ordinary "Copy X" hint keeps working) but force-opens on copy so touch users see the confirmation too; a fixed aria-label; a single role="status" region for the transient announcement; and internal stopPropagation so it can't be forgotten at a call site.

copyToClipboard (src/lib/clipboard.ts) now returns Promise<boolean> instead of firing-and-forgetting, so success/failure can actually be shown.

Consolidated onto CopyButton

  • CopyValue and CodeBlock now delegate to it instead of managing their own state
  • All 12 previously-silent inline copy buttons across ToolsTable, ToolSchemaDialog, ResourcesTable, VirtualServerDetailsPanel, PromptDefinitionTable, TokenCreatedDialog, MCPServerDetailsPanel, and TestConnectionPanel now show the same feedback
  • ServersTable and ToolForm dropped their hand-rolled useState/setTimeout copy logic (including a timer leak in ToolForm)

Root TooltipProvider

Mounted once in App.tsx (delayDuration=0), replacing three duplicated local providers in code-block.tsx, card-tag.tsx, and Gateways.tsx. CopyButton overrides to ~400ms locally so the 14 newly-tooltipped icon buttons don't feel twitchy on hover.

i18n

Added common.copied / common.copyFailed to en-US, es-ES, pt-BR; retired the three now-redundant per-feature "Copied!" keys (mcpServer.table.copied, tools.form.copied, prompts.details.code.copySuccess).

Bug fix along the way

Wrapping the copy buttons in TokenCreatedDialog and ToolSchemaDialog in tooltips broke Escape-to-close for those dialogs: Radix auto-focuses the first focusable descendant on open, and when that's a CopyButton, its tooltip opens on focus and becomes the topmost dismissable layer — so the first Escape press closed the tooltip instead of the dialog. Fixed by redirecting each dialog's initial auto-focus to its Close button.

Non-goals (per issue)

  • No document.execCommand fallback for non-secure contexts
  • Not converting any of this to toasts
  • No new copy buttons added where none existed today

Test plan

  • npx vitest run — 2948 tests passing (171 files), including new useCopyToClipboard and CopyButton unit tests
  • npx tsc -b --noEmit — clean
  • npx eslint src — clean
  • npx playwright test — all 213 e2e tests passing across every spec, including the ToolSchemaDialog and TokenCreatedDialog flows and the escape-to-close drawer test

@marekdano
marekdano requested review from a-effort, gcgoncalves and vishu-bh and removed request for a-effort August 20, 2026 19:29
@marekdano marekdano self-assigned this Aug 21, 2026

@vishu-bh vishu-bh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @marekdano !

Solid improvements, Just some minor things to check


const copy = useCallback(
async (value: string) => {
const ok = await copyToClipboard(value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Multiple clicks can leave overlapping copyToClipboard promises. An older request resolving last can overwrite newer feedback and reset its timer. Also, if the component unmounts before this await completes, cleanup runs before any timer exists; completion then updates unmounted state and schedules an uncleared timer.
Could we track mounted state plus a request sequence, allowing only the latest mounted invocation to update status/timer? Please add deferred-promise tests covering out-of-order completion and unmount-before-resolution.

aria-label={`Copy ${title.toLowerCase()}`}
<CopyButton
value={schemaText}
label={`Copy ${title.toLowerCase()}`}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CopyButton displays label in its idle tooltip, so this formerly aria-only English string is now visible in Spanish and Portuguese locales. Same pattern exists in TestConnectionPanel and both server details panels. Please move these labels to locale messages. PromptPreviewResult should also use an actionable label such as “Copy JSON” instead of only “JSON”.

<DialogContent className="max-w-2xl">
<DialogContent
className="max-w-2xl"
onOpenAutoFocus={(event) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This autofocus override fixes a specific layered-tooltip regression, but current tests do not assert initial focus or single-press Escape dismissal for this dialog. Please add a test confirming the footer Close button receives initial focus and one Escape closes the dialog.

@marekdano
marekdano requested a review from vishu-bh August 21, 2026 09:22
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
@marekdano
marekdano force-pushed the 44-copied-feedback-on-copy-button branch from 5875826 to bb3f5d0 Compare August 21, 2026 11:01
Signed-off-by: Marek Dano <mk.dano@gmail.com>

@vishu-bh vishu-bh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM ☘️

@marekdano
marekdano merged commit ebc7dc3 into main Aug 21, 2026
5 checks passed
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.

Consistent "Copied" feedback on every copy button

2 participants