-
Notifications
You must be signed in to change notification settings - Fork 0
feat: ReplayWeb.page URL link #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
export const REPLAY_BASE_URL = | ||
"https://replayweb-page-m-git-nikita-webtorrent-integration-monadical.vercel.app"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The hardcoded URL appears to be a development or staging environment. For production code, consider using a configuration variable or environment variable to make the base URL configurable across different environments. [general, importance: 7]
export const REPLAY_BASE_URL = | |
"https://replayweb-page-m-git-nikita-webtorrent-integration-monadical.vercel.app"; | |
export const REPLAY_BASE_URL = | |
process.env.REPLAY_BASE_URL || "https://replayweb.page"; |
src/sidepanel.ts
Outdated
// Copy to clipboard | ||
navigator.clipboard | ||
.writeText(magnetURI) | ||
.writeText(replayLink) | ||
.then(() => { | ||
alert(`Magnet link copied to clipboard:\n${magnetURI}`); | ||
alert(`Magnet link copied to clipboard:\n${replayLink}`); | ||
}) | ||
.catch((err) => { | ||
console.error("Failed to copy magnet link:", err); | ||
alert(`Magnet Link Ready:\n${magnetURI}`); | ||
alert(`Magnet Link Ready:\n${replayLink}`); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The alert message says "Magnet link copied" but you're actually copying a ReplayWeb.page URL. Update the message to accurately reflect what's being copied to avoid user confusion. [general, importance: 8]
// Copy to clipboard | |
navigator.clipboard | |
.writeText(magnetURI) | |
.writeText(replayLink) | |
.then(() => { | |
alert(`Magnet link copied to clipboard:\n${magnetURI}`); | |
alert(`Magnet link copied to clipboard:\n${replayLink}`); | |
}) | |
.catch((err) => { | |
console.error("Failed to copy magnet link:", err); | |
alert(`Magnet Link Ready:\n${magnetURI}`); | |
alert(`Magnet Link Ready:\n${replayLink}`); | |
}); | |
// Copy to clipboard | |
navigator.clipboard | |
.writeText(replayLink) | |
.then(() => { | |
alert(`ReplayWeb.page link copied to clipboard:\n${replayLink}`); | |
}) | |
.catch((err) => { | |
console.error("Failed to copy ReplayWeb.page link:", err); | |
alert(`ReplayWeb.page Link Ready:\n${replayLink}`); | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Added the tooltips to all relevant icon buttons in this PR too.
User description
Now, Copy Link and the share button copy the ReplayWeb.page URL link instead of a simple magnet url. #18
PR Type
Enhancement
Description
Replace magnet URI with ReplayWeb.page URL
Add REPLAY_BASE_URL constant for link generation
Update copy link functionality in multiple components
Changes walkthrough 📝
argo-shared-archive-list.ts
Update archive list to use ReplayWeb.page URLs
src/argo-shared-archive-list.ts
URI
sidepanel.ts
Update sidepanel to use ReplayWeb.page URLs
src/sidepanel.ts
consts.ts
Add ReplayWeb.page base URL constant
src/consts.ts
"https://replayweb-page-m-git-nikita-webtorrent-integration-monadical.vercel.app"