-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
docs(feat): Add copy as md button to doc pages #3738
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change introduces a "Copy as Markdown" button to documentation pages. It adds a new React component for copying main content as Markdown, integrates it into the breadcrumb navigation, and applies new CSS modules for styling. Wrapper components are created for breadcrumbs, layout, and content to facilitate these UI enhancements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CopyMarkdownButton
participant DocumentContent
participant Clipboard
User->>CopyMarkdownButton: Click "Copy as Markdown"
CopyMarkdownButton->>DocumentContent: Query main content area
alt Content found
CopyMarkdownButton->>CopyMarkdownButton: Convert DOM to Markdown
CopyMarkdownButton->>Clipboard: Write Markdown text
Clipboard-->>CopyMarkdownButton: Success
CopyMarkdownButton-->>User: Show success icon/tooltip
else Content not found or error
CopyMarkdownButton-->>User: Show error alert
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Can we add a label to it |
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: 2
🧹 Nitpick comments (5)
docs/src/theme/DocItem/Content/index.js (1)
4-10: Consider simplifying or documenting the purpose of this wrapper.This wrapper component currently adds no functionality beyond forwarding props to the original component. The React fragment is also unnecessary.
If this wrapper is intended for future extensibility, consider adding a comment to clarify its purpose. Otherwise, you could simplify it to:
export default function ContentWrapper(props) { - return ( - <> - <Content {...props} /> - </> - ); + return <Content {...props} />; }docs/src/theme/DocItem/Layout/index.js (2)
3-3: Remove unused import.The
CopyMarkdownButtonis imported but never used in this component.-import CopyMarkdownButton from '@site/src/components/CopyMarkdownButton';
5-11: Simplify the wrapper component.Similar to the ContentWrapper, this component adds no functionality and uses an unnecessary React fragment.
export default function LayoutWrapper(props) { - return ( - <> - <Layout {...props} /> - </> - ); + return <Layout {...props} />; }docs/src/components/CopyMarkdownButton/index.js (2)
9-46: Well-implemented copy functionality with good error handling.The fallback selector strategy and modern clipboard API usage are appropriate choices. Consider adding a fallback for older browsers that don't support
navigator.clipboard.For broader browser support, consider this enhancement:
// Copy to clipboard using modern API -await navigator.clipboard.writeText(content); +if (navigator.clipboard) { + await navigator.clipboard.writeText(content); +} else { + // Fallback for older browsers + const textArea = document.createElement('textarea'); + textArea.value = content; + document.body.appendChild(textArea); + textArea.select(); + document.execCommand('copy'); + document.body.removeChild(textArea); +}
132-159: Remove unused table processing functions or add table support.The
processTableandprocessTableRowfunctions are defined but never used since 'table' is not included in the TreeWalker filter (line 56).Either remove these unused functions:
- const processTable = tableElement => { - let result = ''; - const rows = tableElement.querySelectorAll('tr'); - - rows.forEach((row, index) => { - if (index === 1) { - // Add separator row after header - const cells = row.querySelectorAll('th, td'); - result += '|'; - cells.forEach(() => { - result += ' --- |'; - }); - result += '\n'; - } - result += processTableRow(row); - }); - - return result + '\n'; - }; - - const processTableRow = rowElement => { - let result = '|'; - const cells = rowElement.querySelectorAll('th, td'); - cells.forEach(cell => { - result += ` ${cell.textContent.trim()} |`; - }); - return result + '\n'; - };Or add table support by including 'table' in the filter and adding a table case to the switch statement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/src/components/CopyMarkdownButton/index.js(1 hunks)docs/src/components/CopyMarkdownButton/styles.module.css(1 hunks)docs/src/theme/DocBreadcrumbs/index.js(1 hunks)docs/src/theme/DocBreadcrumbs/styles.module.css(1 hunks)docs/src/theme/DocItem/Content/index.js(1 hunks)docs/src/theme/DocItem/Layout/index.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : prefer using the formfieldwrapper component for f...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
Applied to files:
docs/src/theme/DocItem/Content/index.jsdocs/src/theme/DocItem/Layout/index.jsdocs/src/theme/DocBreadcrumbs/index.js
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : use react for all ui components in the dashboard ...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Use React for all UI components in the dashboard package.
Applied to files:
docs/src/theme/DocItem/Layout/index.jsdocs/src/components/CopyMarkdownButton/index.js
📚 Learning: applies to packages/dashboard/src/lib/components/**/*.{tsx,jsx} : use shadcn ui and tailwind css for...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/lib/components/**/*.{tsx,jsx} : Use Shadcn UI and Tailwind CSS for UI components.
Applied to files:
docs/src/theme/DocItem/Layout/index.js
🧬 Code Graph Analysis (1)
docs/src/theme/DocBreadcrumbs/index.js (1)
docs/src/components/CopyMarkdownButton/index.js (1)
CopyMarkdownButton(6-182)
🪛 Biome (2.1.2)
docs/src/components/CopyMarkdownButton/index.js
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 110-110: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
docs/src/theme/DocBreadcrumbs/styles.module.css (1)
1-6: LGTM! Clean and semantic CSS implementation.The flexbox layout with
space-betweenjustification properly separates the breadcrumbs from the copy button, and the vertical centering ensures good alignment. The styling is minimal and follows CSS module conventions.docs/src/theme/DocBreadcrumbs/index.js (1)
6-13: LGTM! Well-structured integration of the copy button.The component properly integrates the
CopyMarkdownButtonwith the existing breadcrumbs using flexbox layout. All imports are used, props are forwarded correctly, and the CSS module is applied appropriately.docs/src/components/CopyMarkdownButton/styles.module.css (1)
1-38: LGTM! Comprehensive styling with good interactive feedback.The button styling is well-implemented with proper use of CSS custom properties, smooth transitions, and good visual hierarchy. The flexbox layout, hover effects, and icon sizing all look appropriate.
docs/src/components/CopyMarkdownButton/index.js (2)
1-8: LGTM!Clean component setup with proper imports and React hooks usage.
168-182: LGTM!Clean render implementation with good accessibility (title attribute) and user feedback through conditional rendering.
| case 'pre': | ||
| const codeElement = currentNode.querySelector('code'); | ||
| const codeText = codeElement ? codeElement.textContent : textContent; | ||
| const language = getCodeLanguage(codeElement); | ||
| markdown += `\n\`\`\`${language}\n${codeText}\n\`\`\`\n\n`; | ||
| break; | ||
| case 'blockquote': | ||
| markdown += `> ${textContent}\n\n`; | ||
| break; | ||
| case 'a': | ||
| const href = currentNode.getAttribute('href'); | ||
| const linkText = textContent; | ||
| if (href && linkText) { | ||
| markdown += `[${linkText}](${href})`; | ||
| } else if (linkText) { | ||
| markdown += linkText; | ||
| } | ||
| break; |
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.
Fix variable declaration scope issues in switch statement.
Variable declarations in switch cases can be accessed by other cases, leading to potential bugs. Wrap declarations in blocks to restrict their scope.
Apply this diff to fix the scope issues:
case 'pre':
- const codeElement = currentNode.querySelector('code');
- const codeText = codeElement ? codeElement.textContent : textContent;
- const language = getCodeLanguage(codeElement);
- markdown += `\n\`\`\`${language}\n${codeText}\n\`\`\`\n\n`;
- break;
+ {
+ const codeElement = currentNode.querySelector('code');
+ const codeText = codeElement ? codeElement.textContent : textContent;
+ const language = getCodeLanguage(codeElement);
+ markdown += `\n\`\`\`${language}\n${codeText}\n\`\`\`\n\n`;
+ break;
+ }
case 'blockquote':
markdown += `> ${textContent}\n\n`;
break;
case 'a':
- const href = currentNode.getAttribute('href');
- const linkText = textContent;
- if (href && linkText) {
- markdown += `[${linkText}](${href})`;
- } else if (linkText) {
- markdown += linkText;
- }
- break;
+ {
+ const href = currentNode.getAttribute('href');
+ const linkText = textContent;
+ if (href && linkText) {
+ markdown += `[${linkText}](${href})`;
+ } else if (linkText) {
+ markdown += linkText;
+ }
+ break;
+ }📝 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.
| case 'pre': | |
| const codeElement = currentNode.querySelector('code'); | |
| const codeText = codeElement ? codeElement.textContent : textContent; | |
| const language = getCodeLanguage(codeElement); | |
| markdown += `\n\`\`\`${language}\n${codeText}\n\`\`\`\n\n`; | |
| break; | |
| case 'blockquote': | |
| markdown += `> ${textContent}\n\n`; | |
| break; | |
| case 'a': | |
| const href = currentNode.getAttribute('href'); | |
| const linkText = textContent; | |
| if (href && linkText) { | |
| markdown += `[${linkText}](${href})`; | |
| } else if (linkText) { | |
| markdown += linkText; | |
| } | |
| break; | |
| case 'pre': { | |
| const codeElement = currentNode.querySelector('code'); | |
| const codeText = codeElement ? codeElement.textContent : textContent; | |
| const language = getCodeLanguage(codeElement); | |
| markdown += `\n\`\`\`${language}\n${codeText}\n\`\`\`\n\n`; | |
| break; | |
| } | |
| case 'blockquote': | |
| markdown += `> ${textContent}\n\n`; | |
| break; | |
| case 'a': { | |
| const href = currentNode.getAttribute('href'); | |
| const linkText = textContent; | |
| if (href && linkText) { | |
| markdown += `[${linkText}](${href})`; | |
| } else if (linkText) { | |
| markdown += linkText; | |
| } | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 110-110: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In docs/src/components/CopyMarkdownButton/index.js between lines 99 and 116,
variable declarations inside switch cases are not scoped properly, which can
cause variables to leak into other cases and lead to bugs. To fix this, wrap the
code inside each case with curly braces {} to create a block scope, ensuring
variables like codeElement, codeText, language, href, and linkText are limited
to their respective cases.
| @media (max-width: 768px) { | ||
| .copyButton { | ||
| top: 70px; | ||
| right: 10px; | ||
| padding: 6px 10px; | ||
| font-size: 11px; | ||
| } | ||
| } |
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.
🛠️ Refactor suggestion
Fix mobile positioning properties.
The mobile media query uses top and right properties, but the button doesn't have position: absolute or fixed set. These properties will have no effect on a statically positioned element.
Either remove the positioning properties if not needed:
@media (max-width: 768px) {
.copyButton {
- top: 70px;
- right: 10px;
padding: 6px 10px;
font-size: 11px;
}
}Or add positioning if the button should be absolutely positioned on mobile:
@media (max-width: 768px) {
.copyButton {
+ position: absolute;
top: 70px;
right: 10px;
padding: 6px 10px;
font-size: 11px;
}
}🤖 Prompt for AI Agents
In docs/src/components/CopyMarkdownButton/styles.module.css around lines 53 to
60, the mobile media query sets top and right properties on the .copyButton
class without specifying position, so these properties have no effect. To fix
this, either remove the top and right properties if positioning is not needed,
or add position: absolute (or fixed) to .copyButton within the media query to
enable the positioning to work as intended.
Done |
|
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: 2
♻️ Duplicate comments (1)
docs/src/components/CopyMarkdownButton/index.js (1)
99-116: Variable declaration scope issues still present.The switch statement variable declarations identified in previous reviews still need to be addressed. These variables can leak between cases and cause runtime errors.
Please apply the previously suggested fix to wrap the declarations in block scope as shown in the earlier review comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/components/CopyMarkdownButton/index.js(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
docs/src/components/CopyMarkdownButton/index.js
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 110-110: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (5)
docs/src/components/CopyMarkdownButton/index.js (5)
1-7: LGTM!The imports are well-organized and the component setup with state management is appropriate for this functionality.
9-46: Well-implemented error handling and content selection.The function demonstrates good practices with try-catch error handling, multiple selector fallbacks for content detection, and appropriate user feedback. The use of modern clipboard API with proper state management is excellent.
48-70: Efficient DOM traversal implementation.The use of TreeWalker with proper element filtering is an excellent approach for processing documentation content. The selected elements cover all the major content types needed for Markdown conversion.
120-121: Good content normalization.The regex-based cleanup to normalize excessive newlines and trim the result is a clean approach to ensure consistent Markdown formatting.
161-183: Excellent UI implementation with good accessibility.The render logic properly handles state transitions, includes accessibility attributes, and provides clear user feedback. The code language detection is also well-implemented.
| const processList = listElement => { | ||
| let result = ''; | ||
| const items = listElement.querySelectorAll('li'); | ||
| items.forEach(item => { | ||
| result += `- ${item.textContent.trim()}\n`; | ||
| }); | ||
| return result + '\n'; | ||
| }; |
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.
Fix list processing to handle ordered lists correctly.
The processList function always uses unordered list syntax (-) regardless of whether the source is an <ol> or <ul> element. This loses important formatting information.
Apply this diff to handle ordered lists correctly:
-const processList = listElement => {
+const processList = listElement => {
let result = '';
const items = listElement.querySelectorAll('li');
+ const isOrdered = listElement.tagName.toLowerCase() === 'ol';
items.forEach(item => {
- result += `- ${item.textContent.trim()}\n`;
+ if (isOrdered) {
+ result += `1. ${item.textContent.trim()}\n`;
+ } else {
+ result += `- ${item.textContent.trim()}\n`;
+ }
});
return result + '\n';
};📝 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 processList = listElement => { | |
| let result = ''; | |
| const items = listElement.querySelectorAll('li'); | |
| items.forEach(item => { | |
| result += `- ${item.textContent.trim()}\n`; | |
| }); | |
| return result + '\n'; | |
| }; | |
| const processList = listElement => { | |
| let result = ''; | |
| const items = listElement.querySelectorAll('li'); | |
| const isOrdered = listElement.tagName.toLowerCase() === 'ol'; | |
| items.forEach(item => { | |
| if (isOrdered) { | |
| result += `1. ${item.textContent.trim()}\n`; | |
| } else { | |
| result += `- ${item.textContent.trim()}\n`; | |
| } | |
| }); | |
| return result + '\n'; | |
| }; |
🤖 Prompt for AI Agents
In docs/src/components/CopyMarkdownButton/index.js around lines 123 to 130, the
processList function currently formats all lists as unordered using '-' which is
incorrect for ordered lists. Modify the function to detect if the listElement is
an ordered list (<ol>) or unordered list (<ul>) and use numbered prefixes (e.g.,
'1.', '2.', etc.) for ordered lists while keeping '-' for unordered lists.
Adjust the iteration to number each list item accordingly for ordered lists.
| const processTable = tableElement => { | ||
| let result = ''; | ||
| const rows = tableElement.querySelectorAll('tr'); | ||
|
|
||
| rows.forEach((row, index) => { | ||
| if (index === 1) { | ||
| // Add separator row after header | ||
| const cells = row.querySelectorAll('th, td'); | ||
| result += '|'; | ||
| cells.forEach(() => { | ||
| result += ' --- |'; | ||
| }); | ||
| result += '\n'; | ||
| } | ||
| result += processTableRow(row); | ||
| }); | ||
|
|
||
| return result + '\n'; | ||
| }; | ||
|
|
||
| const processTableRow = rowElement => { | ||
| let result = '|'; | ||
| const cells = rowElement.querySelectorAll('th, td'); | ||
| cells.forEach(cell => { | ||
| result += ` ${cell.textContent.trim()} |`; | ||
| }); | ||
| return result + '\n'; | ||
| }; |
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.
🛠️ Refactor suggestion
Table processing functions are not integrated.
The processTable and processTableRow functions are implemented but not used. The main switch statement doesn't handle table elements, and the TreeWalker filter excludes them.
Either integrate table support by:
- Adding
'table'to the TreeWalker filter (line 56) - Adding a
case 'table':in the switch statement (line 71)
Or remove these unused functions if table support isn't needed for this release.
🤖 Prompt for AI Agents
In docs/src/components/CopyMarkdownButton/index.js between lines 56 and 71, the
processTable and processTableRow functions are defined but never used because
'table' elements are excluded from the TreeWalker filter and not handled in the
main switch statement. To fix this, add 'table' to the TreeWalker filter around
line 56 and include a case 'table': in the switch statement around line 71 that
calls processTable with the current node. This will integrate table processing
properly. Alternatively, if table support is not required, remove these unused
functions to clean up the code.




Description
This adds a copy as MD button to the top right of all doc pages.
It provides an easy way of quickly copying something into an LLM.
it doesn't properly support the whitespaces in code blocks but it doesn't matter to LLMs
There are libraries that do this, for example, turndown but I opted for a more simpler Js solution since it doesn't need to be that robust.
Breaking changes
Does this PR include any breaking changes we should be aware of?
Screenshots
Checklist
📌 Always:
👍 Most of the time:
Summary by CodeRabbit