feat: add social sharing to catalog valuation results#47
Open
sidneyswift wants to merge 1 commit into
Open
Conversation
Adds copy-link, X, and LinkedIn share buttons below the 'Get full report' CTA on valuation result cards. Each share pre-fills with the artist name and valuation figure, linking back to /valuation to drive organic traffic. Design matches existing system: font-pixel labels, shadow-as-border, foreground opacity modifiers, muted hover states.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new ChangesShare Valuation Feature
Estimated code review effort: 1 (Trivial) | ~5 minutes Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
🤖 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 `@components/valuation/ShareValuation.tsx`:
- Around line 55-63: The copyLink function in ShareValuation should handle
clipboard write failures by updating the UI with a brief error state or “Failed”
label instead of silently swallowing the exception, so users get feedback when
navigator.clipboard.writeText(url) fails. Also store the timeout handle used by
setTimeout(() => setCopied(false), 2000) and clear it during unmount/cleanup to
avoid leaving an active timer behind; keep the logic localized around copyLink
and the copied state handling.
- Around line 47-51: The ShareValuation component is hardcoding brand-specific
values in the tweet text. Update ShareValuation to import the base URL and brand
hashtag from lib/config.ts instead of defining the URL string and `#recoup`
inline, and then build tweetText using those imported values so branding stays
centralized and consistent.
- Around line 70-81: The social share controls in ShareValuation are not
distinguishable to screen readers because both links use the same visible label
and the icon components are exposed to assistive tech. Update the ShareValuation
button/link markup so the Twitter/X and LinkedIn actions each have unique
accessible names (for example via distinct link text or aria-labels) and mark
the icon components used by CopyIcon, CheckIcon, XIcon, and LinkedInIcon as
aria-hidden="true" so they are not announced redundantly.
🪄 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: f069dce4-d063-45b2-82bc-c4890de84608
📒 Files selected for processing (2)
components/valuation/ShareValuation.tsxcomponents/valuation/ValuationResult.tsx
Comment on lines
+47
to
+51
| const url = "https://recoupable.dev/valuation"; | ||
| const value = formatUsd(centralValue); | ||
| const name = artistName ?? "My artist"; | ||
|
|
||
| const tweetText = `${name} catalog just got valued at ${value}. What's yours worth?\n\n${url}\n\n#recoup`; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Hardcoded brand URL and hashtag violate coding guidelines.
The URL https://recoupable.dev/valuation and the #recoup hashtag are brand values hardcoded directly in the component. As per coding guidelines, brand values should be imported from lib/config.ts.
🔧 Proposed fix
- const url = "https://recoupable.dev/valuation";
+ const url = `${siteUrl}/valuation`;
const value = formatUsd(centralValue);
const name = artistName ?? "My artist";
- const tweetText = `${name} catalog just got valued at ${value}. What's yours worth?\n\n${url}\n\n#recoup`;
+ const tweetText = `${name} catalog just got valued at ${value}. What's yours worth?\n\n${url}\n\n#${brandHashtag}`;Import the base URL and brand hashtag from lib/config.ts at the top of the file:
+import { siteUrl, brandHashtag } from "`@/lib/config`";📝 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.
Suggested change
| const url = "https://recoupable.dev/valuation"; | |
| const value = formatUsd(centralValue); | |
| const name = artistName ?? "My artist"; | |
| const tweetText = `${name} catalog just got valued at ${value}. What's yours worth?\n\n${url}\n\n#recoup`; | |
| import { siteUrl, brandHashtag } from "`@/lib/config`"; | |
| const url = `${siteUrl}/valuation`; | |
| const value = formatUsd(centralValue); | |
| const name = artistName ?? "My artist"; | |
| const tweetText = `${name} catalog just got valued at ${value}. What's yours worth?\n\n${url}\n\n#${brandHashtag}`; |
🤖 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 `@components/valuation/ShareValuation.tsx` around lines 47 - 51, The
ShareValuation component is hardcoding brand-specific values in the tweet text.
Update ShareValuation to import the base URL and brand hashtag from
lib/config.ts instead of defining the URL string and `#recoup` inline, and then
build tweetText using those imported values so branding stays centralized and
consistent.
Source: Coding guidelines
Comment on lines
+55
to
+63
| async function copyLink() { | ||
| try { | ||
| await navigator.clipboard.writeText(url); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| } catch { | ||
| // Fallback — noop | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Silent clipboard failure and missing timeout cleanup.
Two issues:
- If
navigator.clipboard.writeTextfails (e.g., non-secure context, permissions denied), the user gets no feedback — the button doesn't change. Consider showing an error state or a brief "Failed" label. - The
setTimeoutis never cleared on unmount. While React 18+ no-ops on unmounted state updates, clearing the timeout is good practice.
🛡️ Proposed fix
+ useEffect(() => {
+ return () => clearTimeout(timerRef.current);
+ }, []);
+
async function copyLink() {
try {
await navigator.clipboard.writeText(url);
setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ timerRef.current = setTimeout(() => setCopied(false), 2000);
} catch {
- // Fallback — noop
+ setCopied(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 `@components/valuation/ShareValuation.tsx` around lines 55 - 63, The copyLink
function in ShareValuation should handle clipboard write failures by updating
the UI with a brief error state or “Failed” label instead of silently swallowing
the exception, so users get feedback when navigator.clipboard.writeText(url)
fails. Also store the timeout handle used by setTimeout(() => setCopied(false),
2000) and clear it during unmount/cleanup to avoid leaving an active timer
behind; keep the logic localized around copyLink and the copied state handling.
Comment on lines
+70
to
+81
| <button type="button" onClick={copyLink} className={btnClass}> | ||
| {copied ? <CheckIcon /> : <CopyIcon />} | ||
| {copied ? "Copied" : "Copy link"} | ||
| </button> | ||
| <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}> | ||
| <XIcon /> | ||
| Share | ||
| </a> | ||
| <a href={linkedInUrl} target="_blank" rel="noopener noreferrer" className={btnClass}> | ||
| <LinkedInIcon /> | ||
| Share | ||
| </a> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Share buttons are indistinguishable for screen readers.
Both social links render the text "Share" with no distinguishing label. Screen reader users hear "Share" twice with no context. Additionally, the inline SVGs lack aria-hidden="true", so they may be announced redundantly.
♿ Proposed fix
<button type="button" onClick={copyLink} className={btnClass} aria-label="Copy valuation link">
{copied ? <CheckIcon /> : <CopyIcon />}
{copied ? "Copied" : "Copy link"}
</button>
- <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>
+ <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass} aria-label="Share on X">
<XIcon />
Share
</a>
- <a href={linkedInUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>
+ <a href={linkedInUrl} target="_blank" rel="noopener noreferrer" className={btnClass} aria-label="Share on LinkedIn">
<LinkedInIcon />
Share
</a>Also add aria-hidden="true" to all four icon SVG components, e.g.:
- <svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
+ <svg aria-hidden="true" width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">🤖 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 `@components/valuation/ShareValuation.tsx` around lines 70 - 81, The social
share controls in ShareValuation are not distinguishable to screen readers
because both links use the same visible label and the icon components are
exposed to assistive tech. Update the ShareValuation button/link markup so the
Twitter/X and LinkedIn actions each have unique accessible names (for example
via distinct link text or aria-labels) and mark the icon components used by
CopyIcon, CheckIcon, XIcon, and LinkedInIcon as aria-hidden="true" so they are
not announced redundantly.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/valuation/ShareValuation.tsx">
<violation number="1" location="components/valuation/ShareValuation.tsx:28">
P3: Local `XIcon` component collides with `lucide-react`'s `XIcon` used elsewhere in the codebase. This creates ambiguity when grepping for the close icon. Rename to something specific like `XLogoIcon` to avoid confusion with the dismiss icon.</violation>
<violation number="2" location="components/valuation/ShareValuation.tsx:47">
P2: The share link URL `https://recoupable.dev/valuation` is hardcoded instead of importing from the site config. The project convention (stated in both AGENTS.md and lib/config.ts) is to import `siteConfig` from `@/lib/config` and use `siteConfig.url` as the single source of truth for the domain. Using the config here prevents the share URL from going stale if the domain ever changes.</violation>
<violation number="3" location="components/valuation/ShareValuation.tsx:59">
P2: Two minor stability issues with `copyLink`:
1. If `navigator.clipboard.writeText` rejects (non-HTTPS context, permission denied), the catch block silently swallows the error and provides no user feedback — the button stays unchanged. Consider briefly showing a "Failed" state.
2. The `setTimeout` reference isn't stored or cleared on unmount. While React 18+ won't crash on a stale `setCopied`, clearing it avoids the no-op state update and is straightforward with a ref + cleanup effect.</violation>
<violation number="4" location="components/valuation/ShareValuation.tsx:74">
P2: The X and LinkedIn share buttons both display "Share" with identical visible text and no `aria-label`. This creates an accessibility gap — screen reader users cannot distinguish which platform each button shares to when navigating by control. Consider adding `aria-label="Share on X"` and `aria-label="Share on LinkedIn"` to give each anchor a unique accessible name.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export function ShareValuation({ artistName, centralValue }: ShareValuationProps) { | ||
| const [copied, setCopied] = useState(false); | ||
|
|
||
| const url = "https://recoupable.dev/valuation"; |
There was a problem hiding this comment.
P2: The share link URL https://recoupable.dev/valuation is hardcoded instead of importing from the site config. The project convention (stated in both AGENTS.md and lib/config.ts) is to import siteConfig from @/lib/config and use siteConfig.url as the single source of truth for the domain. Using the config here prevents the share URL from going stale if the domain ever changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/valuation/ShareValuation.tsx, line 47:
<comment>The share link URL `https://recoupable.dev/valuation` is hardcoded instead of importing from the site config. The project convention (stated in both AGENTS.md and lib/config.ts) is to import `siteConfig` from `@/lib/config` and use `siteConfig.url` as the single source of truth for the domain. Using the config here prevents the share URL from going stale if the domain ever changes.</comment>
<file context>
@@ -0,0 +1,84 @@
+export function ShareValuation({ artistName, centralValue }: ShareValuationProps) {
+ const [copied, setCopied] = useState(false);
+
+ const url = "https://recoupable.dev/valuation";
+ const value = formatUsd(centralValue);
+ const name = artistName ?? "My artist";
</file context>
| {copied ? <CheckIcon /> : <CopyIcon />} | ||
| {copied ? "Copied" : "Copy link"} | ||
| </button> | ||
| <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}> |
There was a problem hiding this comment.
P2: The X and LinkedIn share buttons both display "Share" with identical visible text and no aria-label. This creates an accessibility gap — screen reader users cannot distinguish which platform each button shares to when navigating by control. Consider adding aria-label="Share on X" and aria-label="Share on LinkedIn" to give each anchor a unique accessible name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/valuation/ShareValuation.tsx, line 74:
<comment>The X and LinkedIn share buttons both display "Share" with identical visible text and no `aria-label`. This creates an accessibility gap — screen reader users cannot distinguish which platform each button shares to when navigating by control. Consider adding `aria-label="Share on X"` and `aria-label="Share on LinkedIn"` to give each anchor a unique accessible name.</comment>
<file context>
@@ -0,0 +1,84 @@
+ {copied ? <CheckIcon /> : <CopyIcon />}
+ {copied ? "Copied" : "Copy link"}
+ </button>
+ <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>
+ <XIcon />
+ Share
</file context>
| try { | ||
| await navigator.clipboard.writeText(url); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); |
There was a problem hiding this comment.
P2: Two minor stability issues with copyLink:
- If
navigator.clipboard.writeTextrejects (non-HTTPS context, permission denied), the catch block silently swallows the error and provides no user feedback — the button stays unchanged. Consider briefly showing a "Failed" state. - The
setTimeoutreference isn't stored or cleared on unmount. While React 18+ won't crash on a stalesetCopied, clearing it avoids the no-op state update and is straightforward with a ref + cleanup effect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/valuation/ShareValuation.tsx, line 59:
<comment>Two minor stability issues with `copyLink`:
1. If `navigator.clipboard.writeText` rejects (non-HTTPS context, permission denied), the catch block silently swallows the error and provides no user feedback — the button stays unchanged. Consider briefly showing a "Failed" state.
2. The `setTimeout` reference isn't stored or cleared on unmount. While React 18+ won't crash on a stale `setCopied`, clearing it avoids the no-op state update and is straightforward with a ref + cleanup effect.</comment>
<file context>
@@ -0,0 +1,84 @@
+ try {
+ await navigator.clipboard.writeText(url);
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ } catch {
+ // Fallback — noop
</file context>
| ); | ||
| } | ||
|
|
||
| function XIcon() { |
There was a problem hiding this comment.
P3: Local XIcon component collides with lucide-react's XIcon used elsewhere in the codebase. This creates ambiguity when grepping for the close icon. Rename to something specific like XLogoIcon to avoid confusion with the dismiss icon.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/valuation/ShareValuation.tsx, line 28:
<comment>Local `XIcon` component collides with `lucide-react`'s `XIcon` used elsewhere in the codebase. This creates ambiguity when grepping for the close icon. Rename to something specific like `XLogoIcon` to avoid confusion with the dismiss icon.</comment>
<file context>
@@ -0,0 +1,84 @@
+ );
+}
+
+function XIcon() {
+ return (
+ <svg width="14" height="14" viewBox="0 0 24 24" fill="currentColor">
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds copy-link, X, and LinkedIn share buttons to valuation result cards. Each share pre-fills with artist name + valuation figure, linking back to /valuation for organic traffic.
Summary by cubic
Adds copy-link, X, and LinkedIn share buttons below the Get full report CTA on valuation result cards to make sharing easy and drive traffic to /valuation. Shares prefill the artist name and formatted central valuation; the copy action confirms with a brief "Copied" state.
Written for commit 69b70c8. Summary will update on new commits.
Summary by CodeRabbit