-
Notifications
You must be signed in to change notification settings - Fork 619
[Portal] Feature: Add Copy Page button to documentation pages #8421
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
base: main
Are you sure you want to change the base?
Conversation
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 is attempting to deploy a commit to the thirdweb Team on Vercel. A member of the Team first needs to authorize it. |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
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. |
WalkthroughA 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
Sequence DiagramsequenceDiagram
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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
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.
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
useClipboardhook pattern (as seen inpackages/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.
📒 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:
- Long section names (e.g., "Getting Started with Advanced Configuration Options")
- Various mobile viewport widths (320px, 375px, 425px)
- 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-wrapandgap-2would allow the button to wrap below the title on very small screens, whilemin-w-0allows the title to shrink when needed.
| 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); | ||
| } | ||
| }; |
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.
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.
| const mainElement = document.querySelector("main"); | ||
| if (!mainElement) { | ||
| return; | ||
| } |
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.
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.
| 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); |
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.
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.
| 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.
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
CopyPageButtonclient component using existing UI primitives and lucide-react iconsDocLayout.tsxso it automatically appears on all documentation pagesFiles Changed
apps/portal/src/components/others/CopyPageButton.tsx— new (53 lines)apps/portal/src/components/Layouts/DocLayout.tsx— modified (7 insertions, 3 deletions)Technical Notes
innerTextto preserve readable text formattingCopyButton.tsx)How to Test
cd apps/portal pnpm devPR-Codex overview
This PR introduces a new
CopyPageButtoncomponent to theDocLayout, 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
CopyPageButtonimport toDocLayout.CopyPageButtoncomponent inCopyPageButton.tsx.CopyPageButtonto copy page content.Summary by CodeRabbit