Conversation
- Added a "Copy" button next to the "Save" button in the Generate page. - Implemented `handleCopy` function with fallback for older browsers. - Added visual feedback (Check icon) and toast notification. - Used `Tooltip` for better accessibility. 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 Generate page by introducing a dedicated button to copy generated lyrics. This micro-UX improvement streamlines the workflow for users, providing immediate feedback and ensuring accessibility across various browser environments, ultimately making the application more intuitive and efficient for content management. Highlights
Changelog
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
|
📝 WalkthroughWalkthroughThe Generate page component now includes clipboard copy functionality. Users can copy generated lyrics or content via a new copy button with visual feedback. Copy and Check icons from lucide-react, along with Tooltip components, were imported and integrated into the UI across two action areas. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 adds a "Copy to Clipboard" button to the Generate page, which is a great micro-UX improvement. The implementation includes visual feedback and a fallback for older browsers. My review includes a suggestion to refactor the copy handler to reduce code duplication and improve maintainability.
| const handleCopy = async () => { | ||
| const textToCopy = currentLyrics || generatedContent; | ||
| if (!textToCopy) return; | ||
|
|
||
| // Fallback for older browsers | ||
| if (!navigator.clipboard) { | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = textToCopy; | ||
| document.body.appendChild(textArea); | ||
| textArea.focus(); | ||
| textArea.select(); | ||
| try { | ||
| document.execCommand('copy'); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ description: "Lyrics copied to clipboard" }); | ||
| } catch (err) { | ||
| console.error('Unable to copy to clipboard', err); | ||
| } | ||
| document.body.removeChild(textArea); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await navigator.clipboard.writeText(textToCopy); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ | ||
| description: "Lyrics copied to clipboard", | ||
| }); | ||
| } catch (err) { | ||
| console.error('Failed to copy text: ', err); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The handleCopy function has duplicated logic for showing a success state. This logic is repeated for handling modern and fallback clipboard operations. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this logic can be extracted into a helper function. Additionally, the fallback implementation can be made more robust by checking the boolean return value of document.execCommand('copy').
const handleCopy = async () => {
const textToCopy = currentLyrics || generatedContent;
if (!textToCopy) return;
const onCopySuccess = () => {
setHasCopied(true);
setTimeout(() => setHasCopied(false), 2000);
toast({ description: "Lyrics copied to clipboard" });
};
// Fallback for older browsers
if (!navigator.clipboard) {
const textArea = document.createElement("textarea");
textArea.value = textToCopy;
document.body.appendChild(textArea);
textArea.focus();
textArea.select();
try {
if (document.execCommand('copy')) {
onCopySuccess();
}
} catch (err) {
console.error('Unable to copy to clipboard', err);
}
document.body.removeChild(textArea);
return;
}
try {
await navigator.clipboard.writeText(textToCopy);
onCopySuccess();
} catch (err) {
console.error('Failed to copy text: ', err);
}
};
![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: 71f829c417
ℹ️ 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".
| document.execCommand('copy'); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ description: "Lyrics copied to clipboard" }); |
There was a problem hiding this comment.
In the fallback path (used when navigator.clipboard is unavailable), document.execCommand('copy') can return false without throwing, but this code still sets hasCopied and shows a success toast unconditionally. In browsers where the command is blocked or fails silently, users will be told the lyrics were copied even though the clipboard did not change; check the boolean return value and only show success UI when it is true (with an error toast otherwise).
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client/src/pages/Generate.tsx`:
- Around line 48-81: The handleCopy function currently logs errors to console
only; update both the fallback path (the document.execCommand block) and the
navigator.clipboard.writeText catch block to show a user-facing toast on failure
(use the existing toast function) in addition to logging, e.g., call toast with
a clear error description like "Failed to copy lyrics to clipboard" and
optionally include minimal error info; ensure you still clear the temporary
textarea in the fallback and do not change the success behavior
(setHasCopied/setTimeout).
🧹 Nitpick comments (1)
client/src/pages/Generate.tsx (1)
53-68: Consider wrapping fallback textarea cleanup in afinallyblock.If
textArea.focus()ortextArea.select()throws unexpectedly,removeChildon line 67 would be skipped, leaving a hidden textarea in the DOM. Moving the cleanup intofinallymakes it resilient.Proposed fix
if (!navigator.clipboard) { const textArea = document.createElement("textarea"); textArea.value = textToCopy; + textArea.style.position = "fixed"; + textArea.style.opacity = "0"; document.body.appendChild(textArea); - textArea.focus(); - textArea.select(); try { + textArea.focus(); + textArea.select(); document.execCommand('copy'); setHasCopied(true); setTimeout(() => setHasCopied(false), 2000); toast({ description: "Lyrics copied to clipboard" }); } catch (err) { console.error('Unable to copy to clipboard', err); + toast({ variant: "destructive", description: "Failed to copy lyrics" }); + } finally { + document.body.removeChild(textArea); } - document.body.removeChild(textArea); return; }This also adds
position: fixed; opacity: 0to prevent the textarea from causing a brief layout shift.
| const handleCopy = async () => { | ||
| const textToCopy = currentLyrics || generatedContent; | ||
| if (!textToCopy) return; | ||
|
|
||
| // Fallback for older browsers | ||
| if (!navigator.clipboard) { | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = textToCopy; | ||
| document.body.appendChild(textArea); | ||
| textArea.focus(); | ||
| textArea.select(); | ||
| try { | ||
| document.execCommand('copy'); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ description: "Lyrics copied to clipboard" }); | ||
| } catch (err) { | ||
| console.error('Unable to copy to clipboard', err); | ||
| } | ||
| document.body.removeChild(textArea); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await navigator.clipboard.writeText(textToCopy); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ | ||
| description: "Lyrics copied to clipboard", | ||
| }); | ||
| } catch (err) { | ||
| console.error('Failed to copy text: ', err); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Missing user-facing error feedback on copy failure.
Both the modern clipboard API path (line 78-79) and the fallback path (line 64-65) only log to console.error without notifying the user. As per coding guidelines, "Use toast notifications for user feedback in React components."
Proposed fix
} catch (err) {
console.error('Unable to copy to clipboard', err);
+ toast({ variant: "destructive", description: "Failed to copy lyrics" });
}
document.body.removeChild(textArea);
return;
}
try {
await navigator.clipboard.writeText(textToCopy);
setHasCopied(true);
setTimeout(() => setHasCopied(false), 2000);
toast({
description: "Lyrics copied to clipboard",
});
} catch (err) {
console.error('Failed to copy text: ', err);
+ toast({ variant: "destructive", description: "Failed to copy lyrics" });
}🤖 Prompt for AI Agents
In `@client/src/pages/Generate.tsx` around lines 48 - 81, The handleCopy function
currently logs errors to console only; update both the fallback path (the
document.execCommand block) and the navigator.clipboard.writeText catch block to
show a user-facing toast on failure (use the existing toast function) in
addition to logging, e.g., call toast with a clear error description like
"Failed to copy lyrics to clipboard" and optionally include minimal error info;
ensure you still clear the temporary textarea in the fallback and do not change
the success behavior (setHasCopied/setTimeout).
There was a problem hiding this comment.
Pull request overview
This PR adds a "Copy to Clipboard" button to the Generate page, allowing users to copy generated lyrics with a single click. The implementation provides visual feedback through an icon change (Copy → Check) and displays a toast notification upon successful copy.
Changes:
- Added clipboard copy functionality with fallback support for older browsers
- Introduced a new icon button with tooltip next to the "Save to Library" button
- Implemented visual feedback state (hasCopied) with automatic reset after 2 seconds
| const handleCopy = async () => { | ||
| const textToCopy = currentLyrics || generatedContent; | ||
| if (!textToCopy) return; | ||
|
|
||
| // Fallback for older browsers | ||
| if (!navigator.clipboard) { | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = textToCopy; | ||
| document.body.appendChild(textArea); | ||
| textArea.focus(); | ||
| textArea.select(); | ||
| try { | ||
| document.execCommand('copy'); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ description: "Lyrics copied to clipboard" }); | ||
| } catch (err) { | ||
| console.error('Unable to copy to clipboard', err); | ||
| } | ||
| document.body.removeChild(textArea); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await navigator.clipboard.writeText(textToCopy); | ||
| setHasCopied(true); | ||
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ | ||
| description: "Lyrics copied to clipboard", | ||
| }); | ||
| } catch (err) { | ||
| console.error('Failed to copy text: ', err); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This clipboard copy logic is duplicated from SongDetails.tsx (lines 53-70). The repo already has this exact pattern in at least two places (Generate and SongDetails). Consider extracting this into a reusable hook like useClipboard in client/src/hooks/use-clipboard.ts to avoid duplication and make maintenance easier. The hook should accept text and success message as parameters, return a copy function and hasCopied state, and handle both modern and legacy clipboard APIs consistently.
| setTimeout(() => setHasCopied(false), 2000); | ||
| toast({ description: "Lyrics copied to clipboard" }); | ||
| } catch (err) { | ||
| console.error('Unable to copy to clipboard', err); |
There was a problem hiding this comment.
The fallback error handling in the execCommand path doesn't provide user feedback when copy fails. In the catch block at line 65, consider adding a toast notification with variant "destructive" to inform the user of the failure, similar to how SongDetails.tsx handles this scenario (line 68 in SongDetails.tsx shows the pattern). This ensures consistent error feedback across both clipboard API paths.
| console.error('Unable to copy to clipboard', err); | |
| console.error('Unable to copy to clipboard', err); | |
| toast({ | |
| description: "Failed to copy lyrics to clipboard. Please try again.", | |
| variant: "destructive", | |
| }); |
| description: "Lyrics copied to clipboard", | ||
| }); | ||
| } catch (err) { | ||
| console.error('Failed to copy text: ', err); |
There was a problem hiding this comment.
The modern clipboard API error path (catch at line 78) only logs the error to console but doesn't provide user feedback via toast. Add a toast notification with variant "destructive" to inform users when the copy operation fails, ensuring consistent user experience with the fallback path and matching the pattern used in SongDetails.tsx.
| console.error('Failed to copy text: ', err); | |
| console.error("Failed to copy text: ", err); | |
| toast({ | |
| title: "Copy failed", | |
| description: "Could not copy lyrics to your clipboard. Please try again.", | |
| variant: "destructive", | |
| }); |
1 participant
Added a "Copy to Clipboard" button for generated lyrics in the Generate page. This micro-UX improvement allows users to easily copy lyrics without manual selection. It provides visual feedback with an icon change and a toast notification.
PR created automatically by Jules for task 4181731622216068255 started by @Krosebrook
Summary by cubic
Added a Copy button next to Save on the Generate page so users can quickly copy generated lyrics. Shows a tooltip, a Check icon when copied, and a toast; uses navigator.clipboard with a textarea fallback for older browsers.
Written for commit 71f829c. Summary will update on new commits.
Summary by CodeRabbit