feat: add repository URL copy action in repo modal#840
Conversation
📝 WalkthroughWalkthroughRepoDiscoveryPage adds copy-to-clipboard functionality for repository URLs in the detail modal. Icon imports and state tracking are added, a copy function manages clipboard interaction with visual feedback, and the modal's action footer is refactored to display a new copy button alongside the existing "View on GitHub" link. ChangesRepository URL Copy Feature
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a "copy repository URL" button next to the "View on GitHub" link in the repo detail panel, providing quick clipboard copying with transient visual feedback.
Changes:
- Introduces
repoUrlCopiedstate and acopySelectedRepoUrlhandler using the Clipboard API. - Resets the copied state when the selected repo changes.
- Restructures the action area to render the GitHub link alongside the new copy button with a Check/Copy icon and tooltip.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const copySelectedRepoUrl = () => { | ||
| if (!selectedRepo) return; | ||
| navigator.clipboard.writeText(selectedRepo.url); | ||
| setRepoUrlCopied(true); | ||
| setTimeout(() => setRepoUrlCopied(false), 1500); | ||
| }; |
|
@sachin-mahato25 , Kindly review the PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/src/module/student/opensource/RepoDiscoveryPage.tsx (1)
632-632: ⚡ Quick winReplace arbitrary text size utility with a standard scale class.
text-[10px]is non-canonical per project Tailwind rules; use a standard text scale token.As per coding guidelines, "Do not use arbitrary bracket sizes like
text-[17px], use standard scale classes instead".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/module/student/opensource/RepoDiscoveryPage.tsx` at line 632, In RepoDiscoveryPage.tsx replace the non-canonical utility "text-[10px]" on the span (the element with className beginning "absolute -top-8 right-0 ...") with a standard Tailwind text scale token (e.g. use "text-xs" as the closest standard size); remove the bracketed arbitrary size and ensure the rest of the className (bg, padding, font-bold, dark:text, shadow) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/module/student/opensource/RepoDiscoveryPage.tsx`:
- Around line 619-623: Replace the raw <button> in RepoDiscoveryPage with the
shared Button component: import the shared Button from the UI button module,
pass onClick={copySelectedRepoUrl} and the aria-label (use repoUrlCopied to
toggle the label), and convert the className styling into the Button's props
(e.g., mode="icon" and an appropriate size and variant like size="md"
variant="ghost") while rendering the same icon/children; ensure the Button
receives any needed accessibility attributes and retains the hover/focus
behavior.
- Around line 107-112: The copySelectedRepoUrl handler needs to await
navigator.clipboard.writeText and handle rejections, only setting
setRepoUrlCopied(true) on success; add a persistent timer ref (e.g.,
repoCopyTimerRef) and clearTimeout(repoCopyTimerRef.current) before setting a
new timeout to reset setRepoUrlCopied(false) to avoid overlapping timers; also
update the copy control UI to use the shared Button component instead of a raw
<button> (replace the element used in the copy action around lines 619-636) and
change the “Copied!” tooltip/label to use a theme-safe small text class (avoid
text-[10px], use an existing small text token such as text-sm or a design-system
utility).
---
Nitpick comments:
In `@client/src/module/student/opensource/RepoDiscoveryPage.tsx`:
- Line 632: In RepoDiscoveryPage.tsx replace the non-canonical utility
"text-[10px]" on the span (the element with className beginning "absolute -top-8
right-0 ...") with a standard Tailwind text scale token (e.g. use "text-xs" as
the closest standard size); remove the bracketed arbitrary size and ensure the
rest of the className (bg, padding, font-bold, dark:text, shadow) remains
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b57d3278-f507-4071-be63-7b6bc5b8fa0b
📒 Files selected for processing (1)
client/src/module/student/opensource/RepoDiscoveryPage.tsx
| const copySelectedRepoUrl = () => { | ||
| if (!selectedRepo) return; | ||
| navigator.clipboard.writeText(selectedRepo.url); | ||
| setRepoUrlCopied(true); | ||
| setTimeout(() => setRepoUrlCopied(false), 1500); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
MDN Web Clipboard API: does navigator.clipboard.writeText return a Promise that can reject (e.g., permission denied / insecure context), and what failure handling is recommended?
💡 Result:
Yes, navigator.clipboard.writeText returns a Promise that can reject [1][2]. If the operation fails—such as due to permission denial, lack of a secure context (HTTPS), or the document losing focus—the Promise will reject, throwing an error [1][3][4][2]. Recommended failure handling includes: 1. Always use try...catch blocks: Because the Promise rejects silently if unhandled, you should wrap calls in a try...catch block to manage errors and provide user feedback [1][3][4]. 2. Verify context and state: Ensure the call occurs within a secure context (window.isSecureContext) and that the document is focused (document.hasFocus) before attempting the operation [4][5]. 3. Trigger from user interaction: To satisfy browser security requirements (transient user activation), call writeText directly within a synchronous event handler, such as a click listener [3][6]. Performing asynchronous operations between the user gesture and the clipboard call can cause the gesture context to be lost, particularly in some browsers like Safari [3]. 4. Consider a fallback: While document.execCommand('copy') is deprecated, it remains a common fallback for non-secure contexts or environments where the Clipboard API is unsupported or restricted, though its reliability is decreasing [5]. Example implementation: async function writeClipboardText(text) { try { await navigator.clipboard.writeText(text); console.log('Text copied to clipboard'); } catch (err) { console.error('Failed to copy: ', err); } }
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard
- 3: https://juanchi.dev/en/blog/clipboard-api-typescript-fails-undocumented-cases-copytext
- 4: https://stackoverflow.com/questions/75656677/unable-to-call-navigator-clipboard-writetext-from-within-promise
- 5: https://medium.com/@seeranjeeviramavel/the-pitfall-of-using-navigator-clipboard-in-non-https-web-apps-b47e3f065ab6
- 6: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard_API
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/src/module/student/opensource/RepoDiscoveryPage.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE" >&2
exit 1
fi
echo "=== copySelectedRepoUrl ==="
rg -n "copySelectedRepoUrl|writeText\\(" "$FILE" || true
echo
echo "=== Around copy handler (approx. lines 90-140) ==="
nl -ba "$FILE" | sed -n '90,150p'
echo
echo "=== Around tooltip / modal actions (approx. lines 560-700) ==="
nl -ba "$FILE" | sed -n '560,700'Repository: Sachinchaurasiya360/InternHack
Length of output: 369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/src/module/student/opensource/RepoDiscoveryPage.tsx"
echo "=== copySelectedRepoUrl & reset (lines 95-140) ==="
cat -n "$FILE" | sed -n '95,150p'
echo
echo "=== Search for raw <button> usage and tooltip text-[...] ==="
rg -n "<button\b|text-\[[0-9]+px\]" "$FILE"
echo
echo "=== Around occurrences (line 580-700) ==="
cat -n "$FILE" | sed -n '580,700p'Repository: Sachinchaurasiya360/InternHack
Length of output: 10023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/src/module/student/opensource/RepoDiscoveryPage.tsx"
echo "=== Imports (top of file) ==="
sed -n '1,90p' "$FILE"
echo
echo "=== Search for Button component usage/import ==="
rg -n "components/ui/button|\\bButton\\b" "$FILE" || true
echo
echo "=== Search for useRef/timeout patterns ==="
rg -n "\\buseRef\\b|setTimeout\\(" "$FILE"Repository: Sachinchaurasiya360/InternHack
Length of output: 3981
Fix clipboard copy handling and align copy control UI to guidelines (client/src/module/student/opensource/RepoDiscoveryPage.tsx)
- Lines 107-112:
navigator.clipboard.writeText(...)is async and can reject; also overlappingsetTimeouts can clear the “Copied!” state too early—debounce/cancel the previous timer and handle failures. - Lines 619-636: the copy action uses a raw
<button>(should use the sharedButtoncomponent), and the “Copied!” tooltip usestext-[10px](avoid arbitrary bracket text sizes).
💡 Suggested fix (clipboard + debounced copied timer)
+ const copyResetTimerRef = useRef<number | null>(null);
+
const copySelectedRepoUrl = () => {
if (!selectedRepo) return;
- navigator.clipboard.writeText(selectedRepo.url);
- setRepoUrlCopied(true);
- setTimeout(() => setRepoUrlCopied(false), 1500);
+ navigator.clipboard
+ .writeText(selectedRepo.url)
+ .then(() => {
+ setRepoUrlCopied(true);
+ if (copyResetTimerRef.current) {
+ window.clearTimeout(copyResetTimerRef.current);
+ }
+ copyResetTimerRef.current = window.setTimeout(() => {
+ setRepoUrlCopied(false);
+ copyResetTimerRef.current = null;
+ }, 1500);
+ })
+ .catch(() => {
+ setRepoUrlCopied(false);
+ });
};📝 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 copySelectedRepoUrl = () => { | |
| if (!selectedRepo) return; | |
| navigator.clipboard.writeText(selectedRepo.url); | |
| setRepoUrlCopied(true); | |
| setTimeout(() => setRepoUrlCopied(false), 1500); | |
| }; | |
| const copyResetTimerRef = useRef<number | null>(null); | |
| const copySelectedRepoUrl = () => { | |
| if (!selectedRepo) return; | |
| navigator.clipboard | |
| .writeText(selectedRepo.url) | |
| .then(() => { | |
| setRepoUrlCopied(true); | |
| if (copyResetTimerRef.current) { | |
| window.clearTimeout(copyResetTimerRef.current); | |
| } | |
| copyResetTimerRef.current = window.setTimeout(() => { | |
| setRepoUrlCopied(false); | |
| copyResetTimerRef.current = null; | |
| }, 1500); | |
| }) | |
| .catch(() => { | |
| setRepoUrlCopied(false); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/opensource/RepoDiscoveryPage.tsx` around lines 107
- 112, The copySelectedRepoUrl handler needs to await
navigator.clipboard.writeText and handle rejections, only setting
setRepoUrlCopied(true) on success; add a persistent timer ref (e.g.,
repoCopyTimerRef) and clearTimeout(repoCopyTimerRef.current) before setting a
new timeout to reset setRepoUrlCopied(false) to avoid overlapping timers; also
update the copy control UI to use the shared Button component instead of a raw
<button> (replace the element used in the copy action around lines 619-636) and
change the “Copied!” tooltip/label to use a theme-safe small text class (avoid
text-[10px], use an existing small text token such as text-sm or a design-system
utility).
| <button | ||
| type="button" | ||
| onClick={copySelectedRepoUrl} | ||
| className="relative flex h-11 w-11 shrink-0 items-center justify-center rounded-md border border-stone-200 dark:border-white/10 bg-white dark:bg-stone-900 text-stone-700 dark:text-stone-300 transition-colors hover:bg-stone-100 dark:hover:bg-white/5 cursor-pointer" | ||
| aria-label={repoUrlCopied ? "Copied repository URL" : "Copy repository URL"} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the shared Button component for the new copy action.
This new action is introduced as a raw <button> instead of the reusable Button component used for standardized variants/sizes/modes.
As per coding guidelines, "Use the reusable Button component from client/src/components/ui/button.tsx for all new buttons with variants (primary, secondary, mono, ghost, danger), modes (button, icon, link), and sizes (sm, md, lg)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/opensource/RepoDiscoveryPage.tsx` around lines 619
- 623, Replace the raw <button> in RepoDiscoveryPage with the shared Button
component: import the shared Button from the UI button module, pass
onClick={copySelectedRepoUrl} and the aria-label (use repoUrlCopied to toggle
the label), and convert the className styling into the Button's props (e.g.,
mode="icon" and an appropriate size and variant like size="md" variant="ghost")
while rendering the same icon/children; ensure the Button receives any needed
accessibility attributes and retains the hover/focus behavior.
Sachinchaurasiya360
added
level:beginner
Sachinchaurasiya360
left a comment
Sachinchaurasiya360
left a comment
There was a problem hiding this comment.
Review: Changes Requested
The feature concept is correct but there are three project convention violations that must be fixed before this can merge.
1. Use the shared Button component (required)
The copy button is a raw <button> element. Per CLAUDE.md, all new buttons must use the Button component from client/src/components/ui/button.tsx. Replace with:
import { Button } from "../../components/ui/button";
// replace the raw <button> around line 619 with:
<Button
mode="icon"
variant="ghost"
size="sm"
onClick={copySelectedRepoUrl}
aria-label={repoUrlCopied ? "Copied" : "Copy repository URL"}
>
{repoUrlCopied ? <Check className="h-4 w-4" /> : <Copy className="h-4 w-4" />}
</Button>2. Replace text-[10px] with a standard Tailwind class (required)
text-[10px] is an arbitrary bracket size, which is explicitly banned by the project's TailwindCSS v4 rules. Use text-xs instead on the tooltip span (around line 632).
3. Handle clipboard API rejection (required)
navigator.clipboard.writeText returns a Promise that can reject (e.g., when the page loses focus, or in non-secure contexts). The current handler sets the copied state unconditionally. Add error handling:
const copyTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const copySelectedRepoUrl = async () => {
if (!selectedRepo?.url) return;
try {
await navigator.clipboard.writeText(selectedRepo.url);
if (copyTimerRef.current) clearTimeout(copyTimerRef.current);
setRepoUrlCopied(true);
copyTimerRef.current = setTimeout(() => setRepoUrlCopied(false), 1500);
} catch {
// clipboard write failed silently
}
};The timer ref also prevents a race condition if the user clicks copy multiple times rapidly.
Please address all three before requesting re-review.
Pull Request
Description
Adds a copy-to-clipboard action to the repository detail modal, allowing users to quickly copy a repository's GitHub URL without leaving the page.
What changed
selectedRepo.urlusingnavigator.clipboard.writeTextRelated Issue
Fixes #675
Type of Change
Testing
Manual Testing
Checklist
.env, credentials, ornode_modulescommittedSummary by CodeRabbit