Skip to content
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

Add CopyPrimitive components for copying text functionality #566

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
3 changes: 1 addition & 2 deletions src/components/ui/Avatar/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@ export type AvatarProps = {

const Avatar = ({ customRootClass = '', fallback, className, src, alt, ...props }: AvatarProps) => {
return (
<AvatarPrimitive.Root src={src} customRootClass={customRootClass}>
<AvatarPrimitive.Root src={src} customRootClass={customRootClass} {...props}>
<AvatarPrimitive.Image
src={src}
alt={alt}
className={className}
{...props}
/>
<AvatarPrimitive.Fallback>
{fallback}
Expand Down
20 changes: 20 additions & 0 deletions src/components/ui/Copy/Copy.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import CopyPrimitive from '~/core/primitives/Copy';

const COMPONENT_NAME = 'Copy';

const Copy = ({ customRootClass = '', children, isCopiedContent = 'Copied!', resetDelay = 2000, copyContent = '', ...props }: any) => {
return <CopyPrimitive.Root asChild customRootClass={customRootClass} {...props}>
<CopyPrimitive.Trigger resetDelay={resetDelay} copyContent={copyContent} >{children}</CopyPrimitive.Trigger>
<CopyPrimitive.Feedback >
{isCopiedContent}
</CopyPrimitive.Feedback>
</CopyPrimitive.Root>;
Comment on lines +7 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance component implementation with error handling and accessibility

The current implementation could benefit from several improvements:

  1. Error handling for clipboard operations
  2. Accessibility attributes
  3. Loading and error states

Here's a suggested implementation:

-    return <CopyPrimitive.Root asChild customRootClass={customRootClass} {...props}>
-        <CopyPrimitive.Trigger resetDelay={resetDelay} copyContent={copyContent} >{children}</CopyPrimitive.Trigger>
-        <CopyPrimitive.Feedback >
-            {isCopiedContent}
-        </CopyPrimitive.Feedback>
-    </CopyPrimitive.Root>;
+    return (
+        <CopyPrimitive.Root 
+            asChild 
+            customRootClass={customRootClass} 
+            {...props}
+        >
+            <CopyPrimitive.Trigger 
+                resetDelay={resetDelay} 
+                copyContent={copyContent}
+                aria-label="Copy to clipboard"
+                onError={(error) => console.error('Failed to copy:', error)}
+            >
+                {children}
+            </CopyPrimitive.Trigger>
+            <CopyPrimitive.Feedback 
+                role="status"
+                aria-live="polite"
+            >
+                {isCopiedContent}
+            </CopyPrimitive.Feedback>
+        </CopyPrimitive.Root>
+    );
📝 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
return <CopyPrimitive.Root asChild customRootClass={customRootClass} {...props}>
<CopyPrimitive.Trigger resetDelay={resetDelay} copyContent={copyContent} >{children}</CopyPrimitive.Trigger>
<CopyPrimitive.Feedback >
{isCopiedContent}
</CopyPrimitive.Feedback>
</CopyPrimitive.Root>;
return (
<CopyPrimitive.Root
asChild
customRootClass={customRootClass}
{...props}
>
<CopyPrimitive.Trigger
resetDelay={resetDelay}
copyContent={copyContent}
aria-label="Copy to clipboard"
onError={(error) => console.error('Failed to copy:', error)}
>
{children}
</CopyPrimitive.Trigger>
<CopyPrimitive.Feedback
role="status"
aria-live="polite"
>
{isCopiedContent}
</CopyPrimitive.Feedback>
</CopyPrimitive.Root>
);

};
Comment on lines +6 to +13
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

Define proper TypeScript interfaces for props

The component uses any type which defeats TypeScript's type safety. Consider defining a proper interface for the props.

Here's a suggested implementation:

+interface CopyProps {
+  /** Custom class name for the root element */
+  customRootClass?: string;
+  /** Content to be displayed in the trigger */
+  children: React.ReactNode;
+  /** Text shown when content is copied */
+  isCopiedContent?: string;
+  /** Delay in milliseconds before resetting the copied state */
+  resetDelay?: number;
+  /** Content to be copied to clipboard */
+  copyContent: string;
+}
+
-const Copy = ({ customRootClass = '', children, isCopiedContent = 'Copied!', resetDelay = 2000, copyContent = '', ...props }: any) => {
+const Copy = ({ 
+  customRootClass = '', 
+  children, 
+  isCopiedContent = 'Copied!', 
+  resetDelay = 2000, 
+  copyContent,
+  ...props 
+}: CopyProps) => {
📝 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 Copy = ({ customRootClass = '', children, isCopiedContent = 'Copied!', resetDelay = 2000, copyContent = '', ...props }: any) => {
return <CopyPrimitive.Root asChild customRootClass={customRootClass} {...props}>
<CopyPrimitive.Trigger resetDelay={resetDelay} copyContent={copyContent} >{children}</CopyPrimitive.Trigger>
<CopyPrimitive.Feedback >
{isCopiedContent}
</CopyPrimitive.Feedback>
</CopyPrimitive.Root>;
};
interface CopyProps {
/** Custom class name for the root element */
customRootClass?: string;
/** Content to be displayed in the trigger */
children: React.ReactNode;
/** Text shown when content is copied */
isCopiedContent?: string;
/** Delay in milliseconds before resetting the copied state */
resetDelay?: number;
/** Content to be copied to clipboard */
copyContent: string;
}
const Copy = ({
customRootClass = '',
children,
isCopiedContent = 'Copied!',
resetDelay = 2000,
copyContent,
...props
}: CopyProps) => {
return <CopyPrimitive.Root asChild customRootClass={customRootClass} {...props}>
<CopyPrimitive.Trigger resetDelay={resetDelay} copyContent={copyContent} >{children}</CopyPrimitive.Trigger>
<CopyPrimitive.Feedback >
{isCopiedContent}
</CopyPrimitive.Feedback>
</CopyPrimitive.Root>;
};


Copy.displayName = COMPONENT_NAME;
Copy.Root = CopyPrimitive.Root;
Copy.Trigger = CopyPrimitive.Trigger;
Copy.Feedback = CopyPrimitive.Feedback;

export default Copy;
39 changes: 39 additions & 0 deletions src/components/ui/Copy/stories/Copy.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React from 'react';
import Copy from '../Copy';
import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor';

// More on how to set up stories at: https://storybook.js.org/docs/react/writing-stories/introduction#default-export

const CopyIcon = () => <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M1 9.50006C1 10.3285 1.67157 11.0001 2.5 11.0001H4L4 10.0001H2.5C2.22386 10.0001 2 9.7762 2 9.50006L2 2.50006C2 2.22392 2.22386 2.00006 2.5 2.00006L9.5 2.00006C9.77614 2.00006 10 2.22392 10 2.50006V4.00002H5.5C4.67158 4.00002 4 4.67159 4 5.50002V12.5C4 13.3284 4.67158 14 5.5 14H12.5C13.3284 14 14 13.3284 14 12.5V5.50002C14 4.67159 13.3284 4.00002 12.5 4.00002H11V2.50006C11 1.67163 10.3284 1.00006 9.5 1.00006H2.5C1.67157 1.00006 1 1.67163 1 2.50006V9.50006ZM5 5.50002C5 5.22388 5.22386 5.00002 5.5 5.00002H12.5C12.7761 5.00002 13 5.22388 13 5.50002V12.5C13 12.7762 12.7761 13 12.5 13H5.5C5.22386 13 5 12.7762 5 12.5V5.50002Z" fill="currentColor" fill-rule="evenodd" clip-rule="evenodd"></path></svg>;

const TickIcon = () => <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M11.4669 3.72684C11.7558 3.91574 11.8369 4.30308 11.648 4.59198L7.39799 11.092C7.29783 11.2452 7.13556 11.3467 6.95402 11.3699C6.77247 11.3931 6.58989 11.3355 6.45446 11.2124L3.70446 8.71241C3.44905 8.48022 3.43023 8.08494 3.66242 7.82953C3.89461 7.57412 4.28989 7.55529 4.5453 7.78749L6.75292 9.79441L10.6018 3.90792C10.7907 3.61902 11.178 3.53795 11.4669 3.72684Z" fill="currentColor" fill-rule="evenodd" clip-rule="evenodd"></path></svg>;

export default {
title: 'Components/Copy',
component: Copy,
render: (args: any) => <SandboxEditor>
<div >
<Copy.Root {...args}>
<Copy.Trigger copyContent="Hello, world!" {...args}>
<div className="flex items-center gap-2">
<span>Copy</span>
<CopyIcon />
</div>
</Copy.Trigger>
<Copy.Feedback {...args}>
<div className="flex items-center gap-2">
<span>Copied!</span>
<TickIcon />
</div>
</Copy.Feedback>
</Copy.Root>
</div>
</SandboxEditor>
Comment on lines +14 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve type safety and component flexibility.

Several improvements could enhance this story:

  1. Replace any with proper type definitions
  2. Make copy content configurable via args
  3. Consider using design system tokens for spacing instead of utility classes
-    render: (args: any) => <SandboxEditor>
+    render: (args: CopyProps) => <SandboxEditor>
         <div >
             <Copy.Root {...args}>
-                <Copy.Trigger copyContent="Hello, world!" {...args}>
+                <Copy.Trigger copyContent={args.copyContent} {...args}>
                     <div className="flex items-center gap-2">

Also, consider extracting the hardcoded styles into your design system's spacing tokens:

-    <div className="flex items-center gap-2">
+    <div className={styles.container}>

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

};

// More on writing stories with args: https://storybook.js.org/docs/react/writing-stories/args
export const Button = {
args: {
children: 'Copy'
}
};
5 changes: 5 additions & 0 deletions src/core/primitives/Copy/contexts/CopyPrimitiveContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { createContext } from 'react';

const CopyPrimitiveContext = createContext({});

export default CopyPrimitiveContext;
15 changes: 15 additions & 0 deletions src/core/primitives/Copy/fragments/CopyPrimitiveFeedback.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React, { useContext } from 'react';
import Primitive from '~/core/primitives/Primitive';
import CopyPrimitiveContext from '../contexts/CopyPrimitiveContext';

const CopyPrimitiveFeedback = ({ children, className = '', ...props }: any) => {
const { isCopied, rootClass } = useContext(CopyPrimitiveContext);

if (!isCopied) {
return null;
}

return <Primitive.span className={`${rootClass}-feedback ${className}`} {...props}>{children}</Primitive.span>;
};

export default CopyPrimitiveFeedback;
25 changes: 25 additions & 0 deletions src/core/primitives/Copy/fragments/CopyPrimitiveRoot.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React, { useState } from 'react';
import Primitive from '~/core/primitives/Primitive';

import { customClassSwitcher } from '~/core/customClassSwitcher';

import CopyPrimitiveContext from '../contexts/CopyPrimitiveContext';

const CopyPrimitiveRoot = ({ customRootClass = '', className = '', children, ...props }: any) => {
const [isCopied, setIsCopied] = useState(false);
const rootClass = customClassSwitcher(customRootClass, 'Copy');

const values = {
isCopied,
setIsCopied,
rootClass
};

return (
<CopyPrimitiveContext.Provider value={values}>
<Primitive.span className={`${rootClass} ${className}`} {...props}>{children}</Primitive.span>
</CopyPrimitiveContext.Provider>
);
Comment on lines +18 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use semantic HTML and improve accessibility

The component uses a span element which might not be the most semantic choice for copy functionality. Consider using a more appropriate element and adding accessibility attributes.

Consider this improvement:

 return (
     <CopyPrimitiveContext.Provider value={values}>
-        <Primitive.span className={`${rootClass} ${className}`} {...props}>{children}</Primitive.span>
+        <Primitive.div
+            role="region"
+            aria-label="Copyable content"
+            className={`${rootClass}`.trim() + (className ? ` ${className}` : '')}
+            {...props}
+        >
+            {children}
+        </Primitive.div>
     </CopyPrimitiveContext.Provider>
 );
📝 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
return (
<CopyPrimitiveContext.Provider value={values}>
<Primitive.span className={`${rootClass} ${className}`} {...props}>{children}</Primitive.span>
</CopyPrimitiveContext.Provider>
);
return (
<CopyPrimitiveContext.Provider value={values}>
<Primitive.div
role="region"
aria-label="Copyable content"
className={`${rootClass}`.trim() + (className ? ` ${className}` : '')}
{...props}
>
{children}
</Primitive.div>
</CopyPrimitiveContext.Provider>
);

};

export default CopyPrimitiveRoot;
26 changes: 26 additions & 0 deletions src/core/primitives/Copy/fragments/CopyPrimitiveTrigger.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React, { useContext } from 'react';
import Primitive from '~/core/primitives/Primitive';
import CopyPrimitiveContext from '../contexts/CopyPrimitiveContext';

// The triggering action (button) is logically part of the copying mechanism
const CopyTrigger = ({ children, className = '', copyContent = '', resetDelay = 2000, ...props }: any) => {
const { setIsCopied, isCopied, rootClass } = useContext(CopyPrimitiveContext);

const handleClick = () => {
if (copyContent) {
setIsCopied(true);
navigator.clipboard.writeText(copyContent);
setTimeout(() => {
setIsCopied(false);
}, resetDelay);
}
};

if (!copyContent || isCopied) {
return null;
}
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconsider UX for button visibility.

Hiding the button after copying might confuse users or prevent them from copying the same content multiple times. Consider:

  1. Keeping the button visible and updating its text/icon to indicate the copied state
  2. Allowing users to copy content multiple times without waiting for the reset
-    if (!copyContent || isCopied) {
+    if (!copyContent) {
         return null;
     }
📝 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
if (!copyContent || isCopied) {
return null;
}
if (!copyContent) {
return null;
}


return <Primitive.button className={`${rootClass}-trigger ${className}`} {...props} onClick={handleClick}>{children}</Primitive.button>;
};

export default CopyTrigger;
11 changes: 11 additions & 0 deletions src/core/primitives/Copy/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import CopyPrimitiveRoot from './fragments/CopyPrimitiveRoot';
import CopyTrigger from './fragments/CopyPrimitiveTrigger';
import CopyFeedback from './fragments/CopyPrimitiveFeedback';
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Maintain consistent naming conventions across imports

The import names should match their usage in the CopyPrimitive object for better maintainability and clarity.

Apply this diff to maintain consistency:

-import CopyTrigger from './fragments/CopyPrimitiveTrigger';
-import CopyFeedback from './fragments/CopyPrimitiveFeedback';
+import CopyPrimitiveTrigger from './fragments/CopyPrimitiveTrigger';
+import CopyPrimitiveFeedback from './fragments/CopyPrimitiveFeedback';
📝 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
import CopyPrimitiveRoot from './fragments/CopyPrimitiveRoot';
import CopyTrigger from './fragments/CopyPrimitiveTrigger';
import CopyFeedback from './fragments/CopyPrimitiveFeedback';
import CopyPrimitiveRoot from './fragments/CopyPrimitiveRoot';
import CopyPrimitiveTrigger from './fragments/CopyPrimitiveTrigger';
import CopyPrimitiveFeedback from './fragments/CopyPrimitiveFeedback';


const CopyPrimitive = {
Root: CopyPrimitiveRoot,
Trigger: CopyTrigger,
Feedback: CopyFeedback
} as const;

export default CopyPrimitive;
24 changes: 24 additions & 0 deletions src/core/primitives/Copy/stories/CopyPrimitive.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';
import CopyPrimitive from '../index';
import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor';

// More on how to set up stories at: https://storybook.js.org/docs/react/writing-stories/introduction#default-export
export default {
title: 'Primitives/CopyPrimitive',
component: CopyPrimitive,
render: (args: any) => <SandboxEditor>
<div >
<CopyPrimitive.Root {...args}>
<CopyPrimitive.Trigger copyContent="Hello, world!" {...args}>Copy</CopyPrimitive.Trigger>
<CopyPrimitive.Feedback {...args}>Copied!</CopyPrimitive.Feedback>
</CopyPrimitive.Root>
</div>
</SandboxEditor>
};

// More on writing stories with args: https://storybook.js.org/docs/react/writing-stories/args
export const Button = {
args: {
children: 'Copy'
}
};
23 changes: 23 additions & 0 deletions styles/themes/components/copy.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
.rad-ui-copy{
border: 1px solid var(--rad-ui-color-gray-700);
display: inline-flex;
align-items: center;
gap: 8px;
background-color: var(--rad-ui-color-gray-100);
border-radius: 12px;
cursor: pointer;

.rad-ui-copy-trigger{
display: flex;
align-items: center;
gap: 8px;
padding: 12px;
}

.rad-ui-copy-feedback{
display: flex;
align-items: center;
gap: 8px;
padding: 12px;
}
}
1 change: 1 addition & 0 deletions styles/themes/default.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
@import "./components/button.scss";
@import "./components/callout.scss";
@import "./components/card.scss";
@import "./components/copy.scss";
@import "./components/table.scss";
@import "./components/code.scss";
@import "./components/heading.scss";
Expand Down
Loading