Skip to content

fix: handle clipboard write failure in CopyButton, remove duplicate catch#1765

Merged
lalalune merged 1 commit intomilady-ai:developfrom
NubsCarson:fix/copy-button-clipboard-handling
Apr 10, 2026
Merged

fix: handle clipboard write failure in CopyButton, remove duplicate catch#1765
lalalune merged 1 commit intomilady-ai:developfrom
NubsCarson:fix/copy-button-clipboard-handling

Conversation

@NubsCarson
Copy link
Copy Markdown
Contributor

Summary

  • CopyButton (packages/ui/src/components/ui/copy-button.tsx): Previously used void navigator.clipboard.writeText(value) which fires-and-forgets the clipboard write, then immediately shows the "copied" checkmark — even when the write fails (permission denied, insecure context, etc.). Now the component awaits the promise and only shows success feedback on actual success.
  • CopyButton timer leak: The setTimeout that resets the copied state was never cleaned up on unmount, which could call setCopied on an unmounted component. Added a useRef + cleanup useEffect to clear the timer.
  • useDataLoaders (packages/app-core/src/state/useDataLoaders.ts): Removed a duplicate .catch(() => {}) that was chained after an identical .catch(() => {}) — the second handler is unreachable dead code (a caught promise resolves to undefined, so the second catch never fires).

Test plan

  • Click the copy button in the chat UI — verify checkmark appears and text is in clipboard
  • Revoke clipboard permission (or test in an insecure context like HTTP) — verify the button does NOT show the checkmark
  • Rapidly click copy, then navigate away — verify no React warnings about setState on unmounted component
  • Verify owner name loading in data loaders still works (no behavioral change from removing the duplicate catch)

🤖 Generated with Claude Code

…e catch

CopyButton previously showed "copied" feedback even when the clipboard
write failed (e.g. permission denied, insecure context). Now the check
icon only appears on successful writes. Also cleans up the feedback
timer on unmount to prevent setState on unmounted components.

Removes a duplicate .catch(() => {}) in useDataLoaders that was
unreachable dead code from a copy-paste error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

Agent Review — Service Unavailable

Both AI reviewers failed to complete the review for this run:

  • Claude: failure
  • Codex (fallback): failure

This is likely a temporary API outage. The PR will need to be re-reviewed once services recover.

View workflow run

  1. Decision: REQUEST CHANGES (automated review unavailable — please re-trigger or wait for service recovery)

@github-actions github-actions bot added category:bugfix Auto-managed semantic PR category trust:probationary Building trust, closer scrutiny (auto-managed) labels Apr 9, 2026
@lalalune lalalune merged commit 17fd556 into milady-ai:develop Apr 10, 2026
15 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:bugfix Auto-managed semantic PR category trust:probationary Building trust, closer scrutiny (auto-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants