-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(web): add copy-to-clipboard to CLI install CTA with success check icon #8411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| <div className="text-sm font-medium text-foreground">or via CLI</div> | ||
| <div className="flex items-center justify-between"> | ||
| <div className="text-sm font-medium text-foreground">or via CLI</div> | ||
| <button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI labels (aria-label, title, and sr-only text) are hardcoded in English. Consider using the project’s translation function (e.g. t('...')) to support internationalization.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| const handleCopy = async () => { | ||
| try { | ||
| await navigator.clipboard.writeText(installCmd) | ||
| setCopied(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In handleCopy, the logic for setting the copied state and timeout is duplicated in both try and fallback blocks. Also, consider clearing any existing timeout before setting a new one to avoid potential overlaps on rapid clicks.
|
🚀 Preview deployed! Your changes have been deployed to Vercel: Preview URL: https://roo-code-website-jh2lo7mnn-roo-code.vercel.app This preview will be updated automatically when you push new commits to this PR. |
|
@roomote-agent apply the PR feedback, and, additionally:
|
|
Acknowledged. I will apply the feedback:
I will push updates to the feat/homepage-cli-copy-cta branch shortly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review protocol engaged: I argued with myself, lost, and the linter took notes.
| try { | ||
| await navigator.clipboard.writeText(installCmd) | ||
| setCopied(true) | ||
| const id = setTimeout(() => setCopied(false), 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Clear any existing timeout before scheduling a new one to avoid overlapping timers and stale state on rapid clicks.
| } catch (_e) { | ||
| // Fallback for environments without Clipboard API support | ||
| const textarea = document.createElement("textarea") | ||
| textarea.value = installCmd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] In fallback, set textarea to readonly and position it offscreen (fixed, left:-9999px) before appendChild to avoid layout shift and improve reliability.
| } | ||
| }, []) | ||
|
|
||
| const handleCopy = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] Optional: feature-detect isSecureContext and navigator.clipboard?.writeText before calling Clipboard API to avoid relying on exceptions for control flow.
|
|
||
| export function InstallSection({ downloads }: InstallSectionProps) { | ||
| const [copied, setCopied] = useState(false) | ||
| const installCmd = "code --install-extension RooVeterinaryInc.roo-cline" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] Optional: extract installCmd to a shared constant (e.g., apps/web-roo-code/src/lib/constants.ts) to keep it DRY across the site.
| <Copy className="h-4 w-4" /> | ||
| )} | ||
| <span className="sr-only" aria-live="polite" role="status"> | ||
| {copied ? "Copied" : "Copy"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] Optional a11y: use a more descriptive SR announcement like 'Copied to clipboard' rather than 'Copied'.
|
Update applied per feedback:
Local checks: lint, type-check, and tests pass. CI checks are green. Please take another look. |
Summary:
Implementation:
Testing:
Acceptance criteria:
Important
Adds copy-to-clipboard feature to CLI install section with success icon in
install-section.tsx.install-section.tsx.useStatefor "copied" state anduseEffectfor timeout cleanup.handleCopy()usesnavigator.clipboardand a textarea fallback for copying.timeoutRefand cleared on unmount.installCmdconstant.This description was created by
for f8d8120. You can customize this summary. It will automatically update as commits are pushed.