feat: add copy dashboard URL button to header#66
Conversation
|
@MansimranKaurBedi0 is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @Priyanshu-byte-coder, it looks like the workflow is failing due to a GitHub Actions permission issue ( |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Clean implementation of the core feature — Clipboard API usage is correct, aria-label is good, CSS variables handle theming, and the copied state feedback is nice. Two things to fix.
Blockers
1. alert("Copy failed") — bad UX
An alert() dialog is jarring and inconsistent with the rest of the UI. Use an inline error state the same way you handle the success case:
const [copyError, setCopyError] = useState(false);
const handleCopy = async () => {
try {
await navigator.clipboard.writeText(window.location.href);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch {
setCopyError(true);
setTimeout(() => setCopyError(false), 2000);
}
};Then in JSX:
{copied ? "✓ Copied!" : copyError ? "Failed" : "📋"}2. Missing newline at end of file
DashboardHeader.tsx is missing a trailing newline.
Minor
Extra blank lines between JSX elements inside the <div> (between <h1> and <p>, and between buttons) — not idiomatic, please remove.
Fix those two and this is good to merge.
|
Implemented all requested changes: ✅ Removed alert-based error handling and added inline error state Kindly review again. Thanks! |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Two issues to fix before this can merge:
1. Regression — removes the Share Profile link
PR #102 (already merged) added a 'Share Profile' link to DashboardHeader.tsx. Your branch is based on an older commit so that change is missing. Please rebase on main and keep the Share Profile link alongside your copy button.
2. Indentation
The rest of the file uses 2-space indent; your additions use 4-space. Please match the existing style.
Once those are fixed this will be ready to go.
|
Hi @MansimranKaurBedi0 — this PR has a merge conflict with git fetch upstream
git rebase upstream/main
# resolve conflicts, then:
git push --force-with-leaseOnce rebased, we'll review and merge. |
e203bbf to
1852dcf
Compare
|
@Priyanshu-byte-coder done ,plz review |
|
Good fixes — CSS vars look correct. One remaining issue: Indentation changed from 2-space to 4-space throughout the file The PR reformatted the entire Please reformat back to 2-space indentation. You can run |
2e45a45 to
ce1843a
Compare
done @Priyanshu-byte-coder review plz |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
- Missing EOF newline on DashboardHeader.tsx. 2. No button background/text color — the copy button has no bg- or text- class, inherits browser defaults, looks inconsistent with adjacent buttons. Add bg-[var(--control)] text-[var(--card-foreground)]. 3. Emoji-only label — the clipboard emoji with no text fallback is inconsistent across platforms. Replace with a text label or SVG icon. 4. Wholesale indentation reformat — the entire component was re-indented from 2-space to 4-space, bloating the diff. Revert to 2-space to match the codebase. 5. Copies current URL, not dashboard URL — window.location.href copies wherever the user is. Use window.location.origin + /dashboard for a predictable link.
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Missing EOF newline — DashboardHeader.tsx ends without a trailing newline. Please add one.
|
This PR has merge conflicts with |
|
This PR has conflicts with the current |
|
This PR has merge conflicts with the current main branch. Please rebase on the latest main to resolve them — your contribution is labeled for GSSoC scoring. |
|
Hi! This PR has merge conflicts with the git fetch upstream
git rebase upstream/main
# resolve any conflicts
git push --force-with-leaseIf you're no longer working on this, let us know and we can close it. Thanks! |
|
This PR has merge conflicts with the main branch. Please rebase your branch on latest main and resolve the conflicts so it can be reviewed and merged. |
|
This PR has been open for a long time and has merge conflicts with the current If you're still interested in contributing this change, please:
If no update is made within 2 weeks, this PR will be closed as stale. |
Summary
This PR adds a copy dashboard URL button to the dashboard header, making it easier for users to quickly share their dashboard link. On clicking the button, the current dashboard URL gets copied and the user sees a short "Copied!" confirmation.
Closes #52
Type of Change
Changes Made
DashboardHeader.tsxaria-labelHow to Test
npm run devScreenshots (if UI change)
Tested locally.
Checklist