feat: add shareable deep-link for individual repo modals#822
Conversation
- Update URL with ?repo=<id> when modal opens (replace: true) - Remove repo param when modal closes - On mount read ?repo= param and open correct modal - Fetch repo by ID on mount if param present - Silently ignore invalid or 404 repo IDs - Add Share button in modal header copies URL to clipboard - Share button shows Copied! for 1.5s then resets - replace: true keeps back button behavior natural - Resolves Sachinchaurasiya360#703
📝 WalkthroughWalkthroughRepoDiscoveryPage now supports deep-linking to individual repos via URL query parameters ( ChangesRepo Deep-Linking and Share Functionality
Sequence DiagramsequenceDiagram
participant User
participant RepoCard
participant RepoDiscoveryPage
participant URLSearchParams
participant useQuery
participant Clipboard
Note over User,Clipboard: User opens repo from card
User->>RepoCard: clicks repo card
RepoCard->>RepoDiscoveryPage: onSelect(repo)
RepoDiscoveryPage->>RepoDiscoveryPage: handleOpenRepo(repo.id)
RepoDiscoveryPage->>URLSearchParams: setSearchParams({repo: id})
RepoDiscoveryPage->>RepoDiscoveryPage: setSelectedRepo(repo)
Note over User,Clipboard: Deep-link: user visits URL with ?repo=id
User->>RepoDiscoveryPage: page load with ?repo=id
RepoDiscoveryPage->>useQuery: fetch /api/opensource/:id
useQuery-->>RepoDiscoveryPage: repo data
RepoDiscoveryPage->>RepoDiscoveryPage: setSelectedRepo(repo)
Note over User,Clipboard: User shares repo
User->>RepoDiscoveryPage: clicks Share button
RepoDiscoveryPage->>RepoDiscoveryPage: handleShare()
RepoDiscoveryPage->>Clipboard: copy window.location.href
RepoDiscoveryPage->>RepoDiscoveryPage: setCopiedShareUrl(true)
Note right of RepoDiscoveryPage: Button shows "Copied!" for ~2s
Note over User,Clipboard: User closes modal
User->>RepoDiscoveryPage: backdrop click or close button
RepoDiscoveryPage->>RepoDiscoveryPage: handleCloseRepo()
RepoDiscoveryPage->>URLSearchParams: clearSearchParams()
RepoDiscoveryPage->>RepoDiscoveryPage: setSelectedRepo(null)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 |
![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
🧹 Nitpick comments (1)
client/src/module/student/opensource/RepoDiscoveryPage.tsx (1)
496-524: 💤 Low valueConsider using the reusable
Buttoncomponent for new buttons.The close button at lines 517-524 could use
Buttonwithvariant="ghost"andmode="icon". The share button has state-dependent styling that may require custom classes, but the close button is a good candidate.As per coding guidelines: "Use the reusable
Buttoncomponent fromclient/src/components/ui/button.tsxfor all new buttons".🤖 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 496 - 524, Replace the plain close <button> with the shared Button component to follow UI guidelines: locate the close handler usage (onClick={handleCloseRepo}) and the X icon markup in RepoDiscoveryPage and swap the raw <button> for the reusable Button from client/src/components/ui/button.tsx, passing variant="ghost", mode="icon", aria-label="Close", and keep the onClick={handleCloseRepo} and the <X /> child so behavior and accessibility remain identical; leave the share button as-is since it has custom state styling.
🤖 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 101-105: The handleShare function currently calls
navigator.clipboard.writeText without handling Promise rejections; update
handleShare to be async and follow the existing ShareButton pattern: if
navigator.share is available, call it first inside try/catch, otherwise await
navigator.clipboard.writeText(window.location.href) inside try/catch, and on
success call setCopiedShareUrl(true) with the existing timeout; on failure catch
and log/report the error and optionally show a fallback UI message. Reference
the handleShare function and the setCopiedShareUrl state to implement these
changes.
---
Nitpick comments:
In `@client/src/module/student/opensource/RepoDiscoveryPage.tsx`:
- Around line 496-524: Replace the plain close <button> with the shared Button
component to follow UI guidelines: locate the close handler usage
(onClick={handleCloseRepo}) and the X icon markup in RepoDiscoveryPage and swap
the raw <button> for the reusable Button from
client/src/components/ui/button.tsx, passing variant="ghost", mode="icon",
aria-label="Close", and keep the onClick={handleCloseRepo} and the <X /> child
so behavior and accessibility remain identical; leave the share button as-is
since it has custom state styling.
🪄 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: 510ce5c0-d6ec-4031-9f0e-712c0b942127
📒 Files selected for processing (1)
client/src/module/student/opensource/RepoDiscoveryPage.tsx
| const handleShare = () => { | ||
| navigator.clipboard.writeText(window.location.href); | ||
| setCopiedShareUrl(true); | ||
| setTimeout(() => setCopiedShareUrl(false), 1500); | ||
| }; |
There was a problem hiding this comment.
Add error handling for clipboard API.
navigator.clipboard.writeText() returns a Promise that can reject (e.g., permission denied, non-secure context). The existing ShareButton.tsx in the codebase uses async/await with try/catch and also leverages navigator.share as the primary method when available.
🛡️ Proposed fix following codebase pattern
- const handleShare = () => {
- navigator.clipboard.writeText(window.location.href);
- setCopiedShareUrl(true);
- setTimeout(() => setCopiedShareUrl(false), 1500);
- };
+ const handleShare = async () => {
+ try {
+ await navigator.clipboard.writeText(window.location.href);
+ setCopiedShareUrl(true);
+ setTimeout(() => setCopiedShareUrl(false), 1500);
+ } catch (error) {
+ console.error("Failed to copy URL:", error);
+ }
+ };🤖 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 101
- 105, The handleShare function currently calls navigator.clipboard.writeText
without handling Promise rejections; update handleShare to be async and follow
the existing ShareButton pattern: if navigator.share is available, call it first
inside try/catch, otherwise await
navigator.clipboard.writeText(window.location.href) inside try/catch, and on
success call setCopiedShareUrl(true) with the existing timeout; on failure catch
and log/report the error and optionally show a fallback UI message. Reference
the handleShare function and the setCopiedShareUrl state to implement these
changes.
Sachinchaurasiya360
added
level:beginner
Sachinchaurasiya360
left a comment
Sachinchaurasiya360
left a comment
There was a problem hiding this comment.
Review: Changes Requested
Deep-linking for repo modals is a great UX improvement — bookmarking and sharing individual repos are real needs. Two issues must be fixed before merging.
1. Clipboard API rejection not handled
handleShare calls navigator.clipboard.writeText() without awaiting the Promise or handling rejection. If the clipboard write fails (e.g., page loses focus, non-secure context, permissions denied), the user still sees the "Copied!" confirmation even though nothing was copied. Fix:
const copyTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const handleShare = async () => {
try {
await navigator.clipboard.writeText(window.location.href);
if (copyTimerRef.current) clearTimeout(copyTimerRef.current);
setCopiedShareUrl(true);
copyTimerRef.current = setTimeout(() => setCopiedShareUrl(false), 1500);
} catch {
// silently fail — user's clipboard is unavailable
}
};2. Close button should use the shared Button component
The close button (around lines 517-524) is a raw <button> element. Per CLAUDE.md, all new buttons must use the shared Button from client/src/components/ui/button.tsx:
import { Button } from "../../components/ui/button";
<Button
mode="icon"
variant="ghost"
onClick={handleCloseRepo}
aria-label="Close"
>
<X className="h-4 w-4" />
</Button>Please fix both items. The deep-link logic itself (URL param on open, clean removal on close, auto-open on page load) is correct and well-implemented.
Sachinchaurasiya360
merged commit 85e9bde
into
Sachinchaurasiya360:main
|
Hey @Sachinchaurasiya360! 👋 Thanks! 🙌 |
Sachinchaurasiya360
added
good first issue
2 participants
Description
Repo detail modals had no URL — closing the tab or sharing a link lost the specific repo entirely. Students could not bookmark or share individual repos with peers.
Changes:
Related Issue
Fixes #703
Type of Change
Testing
Screenshots
Checklist
Summary by CodeRabbit