Skip to content

Conversation

@wamimi
Copy link

@wamimi wamimi commented Nov 15, 2025

Notes for the Reviewer

This PR adds a global "Copy Page" button to all documentation pages, allowing users to copy entire page content with one click instead of manually selecting text.

Key Implementation Details

  • Created new CopyPageButton client component using existing UI primitives and lucide-react icons
  • Integrated into DocLayout.tsx so it automatically appears on all documentation pages
  • No changes to individual MDX files required, works through layout injection

Files Changed

  • apps/portal/src/components/others/CopyPageButton.tsxnew (53 lines)
  • apps/portal/src/components/Layouts/DocLayout.tsxmodified (7 insertions, 3 deletions)

Technical Notes

  • Uses innerText to preserve readable text formatting
  • Follows existing patterns in codebase (similar to CopyButton.tsx)

How to Test

  1. Run the portal locally:
    cd apps/portal
    pnpm dev
    
    

PR-Codex overview

This PR introduces a new CopyPageButton component to the DocLayout, allowing users to copy the content of the page. It modifies the layout to include this button and enhances the user interface by adjusting the structure of the sidebar.

Detailed summary

  • Added CopyPageButton import to DocLayout.
  • Replaced the sidebar layout with a flex container for better alignment.
  • Introduced the CopyPageButton component in CopyPageButton.tsx.
  • Implemented clipboard functionality in CopyPageButton to copy page content.
  • Added visual feedback for the copy action (changes button text and icon).

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features
    • Added a "Copy Page" button to documentation pages, enabling users to quickly copy page content to their clipboard with visual confirmation when the action completes.

Add a global "Copy Page" button to all documentation pages, allowing users to copy the entire page content with one click instead of manually selecting text from top to bottom.
@wamimi wamimi requested review from a team as code owners November 15, 2025 18:15
@changeset-bot
Copy link

changeset-bot bot commented Nov 15, 2025

⚠️ No Changeset found

Latest commit: e5e5dcc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Nov 15, 2025

@wamimi is attempting to deploy a commit to the thirdweb Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 15, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added the Portal Involves changes to the Portal (docs) codebase. label Nov 15, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 15, 2025

Walkthrough

A new CopyPageButton component is created and integrated into DocLayout. The button extracts and copies the main page's text content to the clipboard, displaying visual feedback with a state toggle that switches between "Copy Page" and "Copied!" states.

Changes

Cohort / File(s) Summary
New CopyPageButton component
apps/portal/src/components/others/CopyPageButton.tsx
New React component that renders a button to copy page content to clipboard with a 2-second confirmation state, includes error handling
DocLayout integration
apps/portal/src/components/Layouts/DocLayout.tsx
Imports and renders CopyPageButton alongside sidebar title in a flex container with justify-between alignment

Sequence Diagram

sequenceDiagram
    User->>CopyPageButton: Click "Copy Page"
    CopyPageButton->>DOM: Find main element
    CopyPageButton->>DOM: Extract text content
    CopyPageButton->>Clipboard API: writeText(content)
    alt Success
        Clipboard API-->>CopyPageButton: Success
        CopyPageButton->>CopyPageButton: Set copied = true
        CopyPageButton->>UI: Show "Copied!"
        Note over CopyPageButton: Wait 2 seconds
        CopyPageButton->>CopyPageButton: Set copied = false
        CopyPageButton->>UI: Show "Copy Page"
    else Error
        Clipboard API-->>CopyPageButton: Error
        CopyPageButton->>Console: Log error
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Straightforward clipboard API usage with basic state management
  • Component follows a single-responsibility pattern with clear control flow
  • Error handling is minimal (console logging only)

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main change: adding a Copy Page button to documentation pages in the Portal.
Description check ✅ Passed The description provides comprehensive information including purpose, implementation details, files changed, technical notes, and testing instructions that align well with the template requirements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
apps/portal/src/components/others/CopyPageButton.tsx (1)

7-31: Recommended: Use existing useClipboard hook pattern.

The codebase already has a useClipboard hook pattern (as seen in packages/thirdweb/src/react/web/ui/components/CopyIcon.tsx). Consider extracting the clipboard logic into a similar reusable hook to maintain consistency and reduce code duplication.

This would:

  • Centralize clipboard logic and error handling
  • Make the component simpler and more focused on rendering
  • Allow reuse across other components that need clipboard functionality
  • Automatically handle cleanup and edge cases
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c7ff3b1 and e5e5dcc.

📒 Files selected for processing (2)
  • apps/portal/src/components/Layouts/DocLayout.tsx (2 hunks)
  • apps/portal/src/components/others/CopyPageButton.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T23:44:47.512Z
Learnt from: MananTank
Repo: thirdweb-dev/js PR: 7951
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/_layout/contract-page-layout.tsx:38-38
Timestamp: 2025-08-29T23:44:47.512Z
Learning: The ContractPageLayout component in apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/_layout/contract-page-layout.tsx is not the root layout - it's nested within the dashboard layout which already handles footer positioning with min-h-dvh and AppFooter placement. The ContractPageLayout needs flex flex-col grow to properly participate in the parent's flex layout.

Applied to files:

  • apps/portal/src/components/Layouts/DocLayout.tsx
📚 Learning: 2025-07-31T16:17:42.753Z
Learnt from: MananTank
Repo: thirdweb-dev/js PR: 7768
File: apps/playground-web/src/app/navLinks.ts:1-1
Timestamp: 2025-07-31T16:17:42.753Z
Learning: Configuration files that import and reference React components (like icon components from lucide-react) need the "use client" directive, even if they primarily export static data, because the referenced components need to be executed in a client context when used by other client components.

Applied to files:

  • apps/portal/src/components/others/CopyPageButton.tsx
🧬 Code graph analysis (2)
apps/portal/src/components/Layouts/DocLayout.tsx (1)
apps/portal/src/components/others/CopyPageButton.tsx (1)
  • CopyPageButton (7-53)
apps/portal/src/components/others/CopyPageButton.tsx (1)
packages/thirdweb/src/react/web/ui/components/CopyIcon.tsx (1)
  • CopyIcon (12-60)
🔇 Additional comments (2)
apps/portal/src/components/others/CopyPageButton.tsx (1)

33-52: LGTM!

The button rendering logic is clean and follows good React patterns with conditional rendering based on the copied state. The use of lucide-react icons and the Button component is consistent with the codebase.

apps/portal/src/components/Layouts/DocLayout.tsx (1)

62-67: Verify responsive layout on mobile devices.

The button is now positioned alongside the title using justify-between. On smaller screens or with longer documentation section names, this layout might cause text truncation or button wrapping issues.

Please test the layout with:

  1. Long section names (e.g., "Getting Started with Advanced Configuration Options")
  2. Various mobile viewport widths (320px, 375px, 425px)
  3. Different zoom levels

Consider adding responsive classes if issues occur:

<div className="flex items-center justify-between mb-2 flex-wrap gap-2">
  <h5 className="text-sm text-muted-foreground min-w-0 flex-shrink">
    {props.sideBar.name}
  </h5>
  <CopyPageButton />
</div>

The flex-wrap and gap-2 would allow the button to wrap below the title on very small screens, while min-w-0 allows the title to shrink when needed.

Comment on lines +10 to +31
const handleCopy = async () => {
try {
// Get the main content element
const mainElement = document.querySelector("main");
if (!mainElement) {
return;
}

// Get text content, preserving some structure
const textContent = mainElement.innerText || mainElement.textContent || "";

await navigator.clipboard.writeText(textContent);
setCopied(true);

// Reset after 2 seconds
setTimeout(() => {
setCopied(false);
}, 2000);
} catch (error) {
console.error("Failed to copy page content:", error);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Clean up setTimeout to prevent memory leaks.

The setTimeout on lines 25-27 is not cleaned up if the component unmounts before the 2-second delay completes. This will cause React warnings about state updates on unmounted components and potential memory leaks.

Apply this diff to fix the cleanup issue:

 export function CopyPageButton() {
   const [copied, setCopied] = useState(false);
+  const timeoutRef = useRef<NodeJS.Timeout>();

   const handleCopy = async () => {
     try {
+      // Clear any existing timeout
+      if (timeoutRef.current) {
+        clearTimeout(timeoutRef.current);
+      }
+
       // Get the main content element
       const mainElement = document.querySelector("main");
       if (!mainElement) {
         return;
       }

       // Get text content, preserving some structure
       const textContent = mainElement.innerText || mainElement.textContent || "";

       await navigator.clipboard.writeText(textContent);
       setCopied(true);

       // Reset after 2 seconds
-      setTimeout(() => {
+      timeoutRef.current = setTimeout(() => {
         setCopied(false);
       }, 2000);
     } catch (error) {
       console.error("Failed to copy page content:", error);
     }
   };

+  useEffect(() => {
+    return () => {
+      if (timeoutRef.current) {
+        clearTimeout(timeoutRef.current);
+      }
+    };
+  }, []);

Don't forget to import useRef and useEffect:

-import { useState } from "react";
+import { useState, useRef, useEffect } from "react";

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/portal/src/components/others/CopyPageButton.tsx around lines 10 to 31,
the setTimeout used to reset copied state isn't cleared on unmount which can
trigger React warnings and memory leaks; fix by creating a ref (timerRef) to
hold the timeout id, assign the id when calling setTimeout, clear any existing
timeout before setting a new one, and add a useEffect cleanup that calls
clearTimeout(timerRef.current) on unmount; also add imports for useRef and
useEffect at the top of the file.

Comment on lines +13 to +16
const mainElement = document.querySelector("main");
if (!mainElement) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Minor: Provide user feedback when main element is not found.

If the main element is not found, the function returns silently. Users won't know why the copy operation didn't work, which creates a poor user experience.

Consider showing a toast notification or brief error state:

     const mainElement = document.querySelector("main");
     if (!mainElement) {
+      console.error("Main content element not found");
+      // Consider showing user feedback here (e.g., toast notification)
       return;
     }
📝 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 mainElement = document.querySelector("main");
if (!mainElement) {
return;
}
const mainElement = document.querySelector("main");
if (!mainElement) {
console.error("Main content element not found");
// Consider showing user feedback here (e.g., toast notification)
return;
}
🤖 Prompt for AI Agents
In apps/portal/src/components/others/CopyPageButton.tsx around lines 13 to 16,
the code silently returns when document.querySelector("main") is null; update
this to notify the user before returning — call the application's
toast/notification utility (or fallback to alert if no toast exists) with a
brief message like "Unable to copy: page content not found", then return; keep
the early exit behavior but ensure the UI feedback is shown and accessible (use
aria-live or the app's toast API) so users know why the copy failed.

// Get text content, preserving some structure
const textContent = mainElement.innerText || mainElement.textContent || "";

await navigator.clipboard.writeText(textContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Major: Check clipboard API availability before using.

The navigator.clipboard API may not be available in non-HTTPS contexts or older browsers. This could cause runtime errors when users click the button.

Apply this diff to add a feature check:

+    // Check if clipboard API is available
+    if (!navigator.clipboard) {
+      console.error("Clipboard API not available");
+      return;
+    }
+
     await navigator.clipboard.writeText(textContent);
📝 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
await navigator.clipboard.writeText(textContent);
// Check if clipboard API is available
if (!navigator.clipboard) {
console.error("Clipboard API not available");
return;
}
await navigator.clipboard.writeText(textContent);
🤖 Prompt for AI Agents
In apps/portal/src/components/others/CopyPageButton.tsx around line 21, the code
calls navigator.clipboard.writeText(textContent) without checking for clipboard
API availability which can throw in non-HTTPS or older browsers; update the
handler to first check if navigator.clipboard and navigator.clipboard.writeText
exist and only call it when available, otherwise fall back to a safe alternative
(e.g., create a temporary textarea, select its content and use
document.execCommand('copy'), or gracefully notify the user that copy is not
supported). Ensure the function handles and logs promise rejections from
writeText and cleans up any temporary DOM elements used by the fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Portal Involves changes to the Portal (docs) codebase.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant