Pull Request for Issue #497: Add Copy-Code Functionality #604
Conversation
WalkthroughThe pull request introduces updates to the Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (2)
src/components/ui/Code/Code.tsx (2)
26-40: Enhance accessibility supportWhile the basic accessibility is present with the title attribute, consider these improvements:
<div className="rui-code-root"> <div className='pt-1'> <button onClick={handleCopy} className="copy-button" title="Copy code" + aria-label={isCopied ? "Code copied" : "Copy code"} + aria-pressed={isCopied} + data-state={isCopied ? "copied" : "idle"} > {isCopied ? ( - <CheckCircledIcon width={16} height={16} className='inline-block' /> + <CheckCircledIcon width={16} height={16} className='inline-block' aria-hidden="true" /> ) : ( - <ClipboardIcon width={16} height={16} /> + <ClipboardIcon width={16} height={16} aria-hidden="true" /> )} </button> </div> - <code>{children}</code> + <code className="block relative">{children}</code> </div>
1-41: Consider component architecture improvementsThe component structure is good, but consider these architectural improvements:
- Extract the copy functionality into a custom hook for reusability:
const useCopyToClipboard = (duration = 2000) => { const [isCopied, setIsCopied] = useState(false); const copy = async (text: string) => { try { await navigator.clipboard.writeText(text); setIsCopied(true); setTimeout(() => setIsCopied(false), duration); return true; } catch (error) { console.error('Failed to copy text: ', error); return false; } }; return { isCopied, copy }; };
- Consider adding a loading state for better UX during the copy operation
- Add prop types for customization (e.g., copy duration, custom icons, position of copy button)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)src/components/ui/Code/Code.tsx(1 hunks)
🔇 Additional comments (2)
src/components/ui/Code/Code.tsx (1)
2-3: LGTM: Imports are appropriate for the copy functionality
The imports are correctly structured and include the necessary icons and React hooks for the implementation.
package.json (1)
132-132: Verify dependency version and security implications
The addition of @radix-ui/react-icons looks good, but let's verify:
✅ Verification successful
Let me gather more information about the current React version in the project to ensure compatibility.
✅ React Icons dependency is compatible with project
The @radix-ui/react-icons@1.3.2 is compatible with the project's React version (^18.2.0) as it supports React ^16.x through ^19.x. This is the only Radix UI dependency in the project, and its peer dependencies are satisfied.
Note: While npm audit shows some vulnerabilities, they are in other dependencies (markdown-to-jsx and rollup) and not in @radix-ui/react-icons itself.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential conflicts with existing dependencies
npm info @radix-ui/react-icons@^1.3.2 peerDependencies
# Check for known vulnerabilities
npm audit @radix-ui/react-icons@^1.3.2
# Verify compatibility with React version
npm info @radix-ui/react-icons@^1.3.2 peerDependencies.react
Length of output: 1145
Script:
#!/bin/bash
# Check React version in package.json
grep -A 1 '"react":' package.json
# Check for any other Radix UI dependencies
grep "@radix-ui/" package.json
Length of output: 210
| const [isCopied, setIsCopied] = useState(false); | ||
|
|
||
| const handleCopy = async() => { | ||
| try { | ||
| await navigator.clipboard.writeText(children as string); | ||
| setIsCopied(true); | ||
|
|
||
| setTimeout(() => { | ||
| setIsCopied(false); | ||
| }, 2000); | ||
| } catch (error) { | ||
| console.error('Failed to copy text: ', error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type checking and improve error handling
Several improvements could enhance the robustness of the copy functionality:
- Add type checking before casting children to string
- Provide user feedback for copy failures
- Consider extracting the timeout duration to a constant
Consider this implementation:
const [isCopied, setIsCopied] = useState(false);
+const COPY_FEEDBACK_DURATION = 2000;
+
+const getTextContent = (children: React.ReactNode): string => {
+ if (typeof children === 'string') return children;
+ if (Array.isArray(children)) return children.map(getTextContent).join('');
+ if (React.isValidElement(children)) return getTextContent(children.props.children);
+ return '';
+};
const handleCopy = async() => {
try {
- await navigator.clipboard.writeText(children as string);
+ const text = getTextContent(children);
+ await navigator.clipboard.writeText(text);
setIsCopied(true);
setTimeout(() => {
setIsCopied(false);
- }, 2000);
+ }, COPY_FEEDBACK_DURATION);
} catch (error) {
console.error('Failed to copy text: ', error);
+ // Consider using a toast or other UI feedback mechanism
+ alert('Failed to copy text to clipboard');
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [isCopied, setIsCopied] = useState(false); | |
| const handleCopy = async() => { | |
| try { | |
| await navigator.clipboard.writeText(children as string); | |
| setIsCopied(true); | |
| setTimeout(() => { | |
| setIsCopied(false); | |
| }, 2000); | |
| } catch (error) { | |
| console.error('Failed to copy text: ', error); | |
| } | |
| }; | |
| const [isCopied, setIsCopied] = useState(false); | |
| const COPY_FEEDBACK_DURATION = 2000; | |
| const getTextContent = (children: React.ReactNode): string => { | |
| if (typeof children === 'string') return children; | |
| if (Array.isArray(children)) return children.map(getTextContent).join(''); | |
| if (React.isValidElement(children)) return getTextContent(children.props.children); | |
| return ''; | |
| }; | |
| const handleCopy = async() => { | |
| try { | |
| const text = getTextContent(children); | |
| await navigator.clipboard.writeText(text); | |
| setIsCopied(true); | |
| setTimeout(() => { | |
| setIsCopied(false); | |
| }, COPY_FEEDBACK_DURATION); | |
| } catch (error) { | |
| console.error('Failed to copy text: ', error); | |
| // Consider using a toast or other UI feedback mechanism | |
| alert('Failed to copy text to clipboard'); | |
| } | |
| }; |
|
Please take a look at the issue, we are supposed to add the copy code functionality for the docs website - not the core library - we can ofcourse make use of this component to build it- but this isn't released in the npm package yet |
2 participants
This pull request implements the copy-code functionality in the project. Users can now easily copy code snippets by clicking on the copy icon next to the code block. The icon changes to a checkmark once the code is successfully copied, providing clear feedback to the user.
Changes made:
Summary by CodeRabbit
New Features
Chores
@radix-ui/react-icons.