Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions components/valuation/ShareValuation.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
"use client";

import { useState } from "react";
import { formatUsd } from "@/lib/valuation/formatUsd";

type ShareValuationProps = {
artistName: string | null;
centralValue: number;
};

function CopyIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
<rect x="9" y="9" width="13" height="13" rx="2" ry="2" />
<path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1" />
</svg>
);
}

function CheckIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
<polyline points="20 6 9 17 4 12" />
</svg>
);
}

function XIcon() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="currentColor">
<path d="M18.244 2.25h3.308l-7.227 8.26 8.502 11.24H16.17l-5.214-6.817L4.99 21.75H1.68l7.73-8.835L1.254 2.25H8.08l4.713 6.231zm-1.161 17.52h1.833L7.084 4.126H5.117z" />
</svg>
);
}

function LinkedInIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="currentColor">
<path d="M20.447 20.452h-3.554v-5.569c0-1.328-.027-3.037-1.852-3.037-1.853 0-2.136 1.445-2.136 2.939v5.667H9.351V9h3.414v1.561h.046c.477-.9 1.637-1.85 3.37-1.85 3.601 0 4.267 2.37 4.267 5.455v6.286zM5.337 7.433a2.062 2.062 0 0 1-2.063-2.065 2.064 2.064 0 1 1 2.063 2.065zm1.782 13.019H3.555V9h3.564v11.452zM22.225 0H1.771C.792 0 0 .774 0 1.729v20.542C0 23.227.792 24 1.771 24h20.451C23.2 24 24 23.227 24 22.271V1.729C24 .774 23.2 0 22.222 0h.003z" />
</svg>
);
}

export function ShareValuation({ artistName, centralValue }: ShareValuationProps) {
const [copied, setCopied] = useState(false);

const url = "https://recoupable.dev/valuation";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

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`;
Comment on lines +47 to +51

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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

const tweetUrl = `https://twitter.com/intent/tweet?text=${encodeURIComponent(tweetText)}`;
const linkedInUrl = `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent(url)}`;

async function copyLink() {
try {
await navigator.clipboard.writeText(url);
setCopied(true);
setTimeout(() => setCopied(false), 2000);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
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>

} catch {
// Fallback — noop
}
}
Comment on lines +55 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Silent clipboard failure and missing timeout cleanup.

Two issues:

  1. If navigator.clipboard.writeText fails (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.
  2. The setTimeout is 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.


const btnClass =
"inline-flex items-center gap-2 px-4 py-2.5 rounded-lg text-[12px] font-pixel uppercase tracking-[0.12em] text-(--foreground)/45 transition-all duration-200 hover:text-(--foreground)/70 hover:bg-(--foreground)/[0.04]";

return (
<div className="mt-6 flex items-center justify-center gap-1">
<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}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

<XIcon />
Share
</a>
<a href={linkedInUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>
<LinkedInIcon />
Share
</a>
Comment on lines +70 to +81

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

</div>
);
}
2 changes: 2 additions & 0 deletions components/valuation/ValuationResult.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ArtistHeader } from "@/components/valuation/ArtistHeader";
import { ValuationStats } from "@/components/valuation/ValuationStats";
import { MeasuredCatalog } from "@/components/valuation/MeasuredCatalog";
import { GetFullReportCta } from "@/components/valuation/GetFullReportCta";
import { ShareValuation } from "@/components/valuation/ShareValuation";
import { formatUsd } from "@/lib/valuation/formatUsd";

type ValuationResultProps = {
Expand Down Expand Up @@ -49,6 +50,7 @@ export function ValuationResult({ artist, result, catalogAlbums }: ValuationResu
totalStreams={result.totalStreams}
/>
<GetFullReportCta snapshotId={result.snapshotId} artistName={artist?.name} />
<ShareValuation artistName={artist?.name ?? null} centralValue={result.valueBand.central} />
</div>
);
}