feat(permissions): add copy-to-clipboard button for plan content#3102
feat(permissions): add copy-to-clipboard button for plan content#3102MattPua wants to merge 1 commit into
Conversation
Generated-By: PostHog Code
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "feat(permissions): add copy-to-clipboard..." | Re-trigger Greptile |
| const handleCopy = () => { | ||
| navigator.clipboard.writeText(plan); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 1500); | ||
| }; |
There was a problem hiding this comment.
navigator.clipboard.writeText returns a Promise that can be rejected — for example when the Permissions Policy blocks clipboard access, the page is not focused, or the user denies the permission. Because the return value is never awaited, setCopied(true) is called unconditionally even when the write silently failed, so users see "Copied!" while nothing was placed on the clipboard. Per the project's async/await convention and the rule to wrap async calls in try/catch, this should be awaited inside a try/catch.
| const handleCopy = () => { | |
| navigator.clipboard.writeText(plan); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }; | |
| const handleCopy = async () => { | |
| try { | |
| await navigator.clipboard.writeText(plan); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| } catch { | |
| // clipboard write failed; do not show confirmation | |
| } | |
| }; |
Rule Used: Always wrap asynchronous calls that may fail, such... (source)
Learned From
PostHog/posthog#32098
| const handleCopy = () => { | ||
| navigator.clipboard.writeText(plan); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 1500); |
There was a problem hiding this comment.
Timeout not cleaned up on unmount
The setTimeout handle is never stored or cleared. If the component unmounts within 1.5 seconds of the click, the callback fires on a stale closure. Consider storing the ID in a useRef and clearing it in a useEffect cleanup — or at minimum clearing any outstanding timer when handleCopy is called again in quick succession, to avoid racing timeouts resetting the state at unexpected times.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
TL;DR
Added a copy-to-clipboard button to the plan content display that allows users to easily copy the full plan text. The button shows a confirmation state ("Copied!") after clicking and appears in both normal and fullscreen views.
Problem
Users viewing plan content had limited options for interacting with it. Adding a copy-to-clipboard button makes it easier to share or work with plan text without manual selection and copying.
Changes
CopyandCheckicons from@phosphor-icons/reactfor the copy button and confirmation stateTooltipcomponent for better UX on icon buttonscopiedstate to track clipboard action confirmationhandleCopy()function that copies plan text to clipboard and temporarily shows "Copied!" confirmation (1.5 second duration)Flexcontainer for consistent spacing and alignmentHow did you test this?
Automatic notifications
Created with PostHog Code