🎨 Palette: Add Copy to Clipboard button for generated lyrics#54
🎨 Palette: Add Copy to Clipboard button for generated lyrics#54Krosebrook wants to merge 1 commit intomainfrom
Conversation
- Adds a "Copy to Clipboard" button next to "Save to Library" in Generate.tsx - Implements `navigator.clipboard` with `document.execCommand` fallback for robust support - Provides visual feedback (Check icon) and toast notification - Updates Palette journal with clipboard pattern learning Co-authored-by: Krosebrook <214532761+Krosebrook@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @Krosebrook, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience on the lyrics generation page by introducing a direct way to copy generated content. The implementation prioritizes reliability and user feedback, ensuring that the copy feature works across various browser environments and clearly communicates its status to the user. This change streamlines the workflow for users who wish to quickly utilize generated lyrics without additional steps. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a useful 'Copy to Clipboard' feature for the generated lyrics, improving the user experience. The implementation is robust, including a fallback for older browsers, which is a great touch. I've left one comment with a suggestion to refactor the handleCopy function to improve its maintainability and prevent potential UI glitches with the fallback mechanism.
| const handleCopy = async () => { | ||
| if (!generatedContent) return; | ||
| const text = generatedContent; | ||
| try { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| await navigator.clipboard.writeText(text); | ||
| setIsCopied(true); | ||
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | ||
| } else { | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = text; | ||
| document.body.appendChild(textArea); | ||
| textArea.select(); | ||
| document.execCommand("copy"); | ||
| document.body.removeChild(textArea); | ||
| setIsCopied(true); | ||
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | ||
| } | ||
| setTimeout(() => setIsCopied(false), 2000); | ||
| } catch (err) { | ||
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The handleCopy function is well-structured with a fallback mechanism. However, it can be improved in a few ways:
- Reduce Code Duplication: The success handling logic (
setIsCopied(true)and the successtoast) is repeated in both theifandelseblocks. This can be moved outside the conditional to follow the DRY (Don't Repeat Yourself) principle. - Improve Fallback Robustness: The temporary
textareain the fallback is appended to the DOM without styling. This can cause a screen flicker or an unexpected page scroll. It's better to position it off-screen. - Error Logging: It's helpful to log the error in the
catchblock for easier debugging.
Here's a suggested refactoring that addresses these points:
const handleCopy = async () => {
if (!generatedContent) return;
const text = generatedContent;
try {
if (navigator.clipboard && navigator.clipboard.writeText) {
await navigator.clipboard.writeText(text);
} else {
const textArea = document.createElement("textarea");
textArea.value = text;
// Prevent screen flicker and scrolling.
textArea.style.position = "fixed";
textArea.style.top = "-9999px";
textArea.style.left = "-9999px";
document.body.appendChild(textArea);
textArea.select();
document.execCommand("copy");
document.body.removeChild(textArea);
}
setIsCopied(true);
toast({ title: "Copied!", description: "Lyrics copied to clipboard" });
setTimeout(() => setIsCopied(false), 2000);
} catch (err) {
console.error("Failed to copy text:", err);
toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." });
}
};
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80804de1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| await navigator.clipboard.writeText(text); |
There was a problem hiding this comment.
If navigator.clipboard.writeText exists but rejects (common in embedded webviews, denied permissions, or non-user-gesture contexts), the code jumps to the catch path and never tries the execCommand('copy') fallback. That makes copy fail in environments this change is explicitly trying to support, even though the fallback could still succeed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds an in-page “Copy to Clipboard” action on the lyrics generation workflow so users can copy generated lyrics without saving.
Changes:
- Added copy button (icon-only) alongside “Save to Library” on the Generate page with visual “copied” feedback.
- Implemented clipboard copy logic with a legacy
document.execCommand("copy")fallback and toast notifications. - Documented the clipboard fallback pattern in
.jules/palette.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| client/src/pages/Generate.tsx | Adds copy-to-clipboard UI + handler for generated lyrics (with feedback). |
| .jules/palette.md | Documents the intended “robust clipboard” implementation pattern. |
| const text = generatedContent; | ||
| try { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| await navigator.clipboard.writeText(text); | ||
| setIsCopied(true); | ||
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | ||
| } else { | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = text; | ||
| document.body.appendChild(textArea); | ||
| textArea.select(); | ||
| document.execCommand("copy"); | ||
| document.body.removeChild(textArea); | ||
| setIsCopied(true); | ||
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | ||
| } | ||
| setTimeout(() => setIsCopied(false), 2000); | ||
| } catch (err) { | ||
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); |
There was a problem hiding this comment.
The copy-to-clipboard implementation here duplicates the same Clipboard API + textarea fallback logic already present in client/src/pages/SongDetails.tsx (copyToClipboard). Consider extracting a shared helper/hook (e.g., client/src/lib/clipboard.ts) so behavior, toasts, and future fixes stay consistent across the app.
| const text = generatedContent; | |
| try { | |
| if (navigator.clipboard && navigator.clipboard.writeText) { | |
| await navigator.clipboard.writeText(text); | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| } else { | |
| const textArea = document.createElement("textarea"); | |
| textArea.value = text; | |
| document.body.appendChild(textArea); | |
| textArea.select(); | |
| document.execCommand("copy"); | |
| document.body.removeChild(textArea); | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| } | |
| setTimeout(() => setIsCopied(false), 2000); | |
| } catch (err) { | |
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); | |
| if (!navigator.clipboard || !navigator.clipboard.writeText) { | |
| toast({ | |
| variant: "destructive", | |
| title: "Copy not supported", | |
| description: "Your browser does not support copying to the clipboard.", | |
| }); | |
| return; | |
| } | |
| try { | |
| await navigator.clipboard.writeText(generatedContent); | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| setTimeout(() => setIsCopied(false), 2000); | |
| } catch (err) { | |
| toast({ | |
| variant: "destructive", | |
| title: "Failed to copy", | |
| description: "Please try again.", | |
| }); |
| try { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| await navigator.clipboard.writeText(text); | ||
| setIsCopied(true); | ||
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | ||
| } else { | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = text; | ||
| document.body.appendChild(textArea); | ||
| textArea.select(); | ||
| document.execCommand("copy"); | ||
| document.body.removeChild(textArea); | ||
| setIsCopied(true); | ||
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | ||
| } | ||
| setTimeout(() => setIsCopied(false), 2000); | ||
| } catch (err) { | ||
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); |
There was a problem hiding this comment.
In the legacy fallback branch, document.execCommand("copy") returns a boolean indicating success, but the result is ignored and the UI always shows “Copied!”. Please check the return value and surface a failure (e.g., throw / show the destructive toast) when it returns false, and ensure the temporary textarea is removed in a finally so it can’t be leaked if an error occurs mid-flow.
| try { | |
| if (navigator.clipboard && navigator.clipboard.writeText) { | |
| await navigator.clipboard.writeText(text); | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| } else { | |
| const textArea = document.createElement("textarea"); | |
| textArea.value = text; | |
| document.body.appendChild(textArea); | |
| textArea.select(); | |
| document.execCommand("copy"); | |
| document.body.removeChild(textArea); | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| } | |
| setTimeout(() => setIsCopied(false), 2000); | |
| } catch (err) { | |
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); | |
| let textArea: HTMLTextAreaElement | null = null; | |
| try { | |
| if (navigator.clipboard && navigator.clipboard.writeText) { | |
| await navigator.clipboard.writeText(text); | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| } else { | |
| textArea = document.createElement("textarea"); | |
| textArea.value = text; | |
| document.body.appendChild(textArea); | |
| textArea.select(); | |
| const successful = document.execCommand("copy"); | |
| if (!successful) { | |
| throw new Error("Legacy copy command failed"); | |
| } | |
| setIsCopied(true); | |
| toast({ title: "Copied!", description: "Lyrics copied to clipboard" }); | |
| } | |
| setTimeout(() => setIsCopied(false), 2000); | |
| } catch (err) { | |
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); | |
| } finally { | |
| if (textArea && textArea.parentNode) { | |
| textArea.parentNode.removeChild(textArea); | |
| } |
| setTimeout(() => setIsCopied(false), 2000); | ||
| } catch (err) { | ||
| toast({ variant: "destructive", title: "Failed to copy", description: "Please try again." }); | ||
| } |
There was a problem hiding this comment.
setTimeout(() => setIsCopied(false), 2000) isn’t cleaned up. If the user navigates away quickly or clicks copy repeatedly, this can leave multiple pending timers and can trigger state updates after unmount. Store the timeout id (e.g., in a ref), clear any existing timer before starting a new one, and clear it in an effect cleanup.
1 participant
Added a "Copy to Clipboard" button to the lyrics generation page to improve user workflow. This allows users to quickly copy generated lyrics without saving them first. The implementation includes a robust fallback for environments where the Clipboard API is restricted and provides immediate visual feedback.
PR created automatically by Jules for task 13816031286280836629 started by @Krosebrook
Summary by cubic
Added a Copy to Clipboard button on the lyrics generation page so users can quickly copy generated lyrics without saving. Includes a safe fallback and instant visual feedback.
Written for commit 80804de. Summary will update on new commits.