feat: introduce @unkey/ui internal package with a new hook #2661
Conversation
unrenamed
requested review from
perkinsjr,
chronark,
mcstepp and
MichaelUnkey
as code owners
|
|
@unrenamed is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new custom hook, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (8)
apps/dashboard/components/dashboard/copy-button.tsx (1)
15-15: Consider making the timeout configurable.The timeout value (2000ms) could be made configurable through props to allow different components to customize the duration of the "copied" state.
interface CopyButtonProps extends React.HTMLAttributes<HTMLButtonElement> { value: string; src?: string; + copyTimeout?: number; } -export function CopyButton({ value, className, src, ...props }: CopyButtonProps) { - const [copied, copyToClipboard] = useCopyToClipboard(2000); +export function CopyButton({ value, className, src, copyTimeout = 2000, ...props }: CopyButtonProps) { + const [copied, copyToClipboard] = useCopyToClipboard(copyTimeout);apps/www/components/copy-button.tsx (1)
12-12: Consider making the timeout configurable via props.While 2000ms is a reasonable default, different use cases might benefit from custom timeout durations.
Consider this enhancement:
interface CopyButtonProps extends React.HTMLAttributes<HTMLButtonElement> { value: string; src?: string; + timeout?: number; } -export function CopyButton({ value, className, src, children, ...props }: CopyButtonProps) { - const [copied, copyToClipboard] = useCopyToClipboard(2000); +export function CopyButton({ value, className, src, children, timeout = 2000, ...props }: CopyButtonProps) { + const [copied, copyToClipboard] = useCopyToClipboard(timeout);knip.ts (1)
40-42: Consider adding test files to entry points.For consistency with other internal packages like "internal/billing" and "internal/keys", consider including test files in the entry points.
"internal/ui": { - entry: "src/index.ts", + entry: ["src/index.ts", "**/*.test.ts"], },internal/ui/src/hooks/use-copy-to-clipboard.tsx (2)
16-36: Enhance error handling and state managementThe current implementation could be improved in several ways:
- Expose error state to consumers
- Add proper type guards for ClipboardItem
- Handle specific error types differently
Consider this enhanced implementation:
export const useCopyToClipboard = ( timeout = 3000, -): [boolean, (value: string | ClipboardItem) => Promise<void>] => { +): [boolean, (value: string | ClipboardItem) => Promise<void>, Error | null] => { // ... existing code ... const [copied, setCopied] = useState(false); + const [error, setError] = useState<Error | null>(null); const copyToClipboard = useCallback( async (value: string | ClipboardItem) => { clearTimer(); + setError(null); try { if (typeof value === "string") { await navigator.clipboard.writeText(value); - } else if (value instanceof ClipboardItem) { + } else if ( + typeof ClipboardItem !== 'undefined' && + value instanceof ClipboardItem + ) { await navigator.clipboard.write([value]); + } else { + throw new TypeError('Invalid value type provided to copyToClipboard'); } setCopied(true); if (Number.isFinite(timeout) && timeout >= 0) { timer.current = setTimeout(() => setCopied(false), timeout); } } catch (error) { - console.error("Failed to copy: ", error); + const errorMessage = error instanceof Error ? + error.message : 'Failed to copy to clipboard'; + setError(new Error(errorMessage)); + setCopied(false); } }, [timeout], ); // ... existing code ... - return [copied, copyToClipboard]; + return [copied, copyToClipboard, error];
43-44: Add JSDoc documentation for better developer experienceConsider adding comprehensive JSDoc documentation to improve the hook's usability.
Add this documentation above the hook:
+/** + * React hook for copying text or ClipboardItem to clipboard with timeout functionality + * @param timeout - Duration in milliseconds to show the copied state (default: 3000ms) + * @returns A tuple containing: + * - copied: Boolean indicating if content was recently copied + * - copyToClipboard: Function to copy content to clipboard + * - error: Error object if the copy operation fails, null otherwise + * @example + * ```tsx + * const [copied, copy, error] = useCopyToClipboard(2000); + * return ( + * <button onClick={() => copy("Hello World")}> + * {copied ? "Copied!" : "Copy"} + * </button> + * ); + * ``` + */ export const useCopyToClipboard = (apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/menu.tsx (1)
12-12: Consider utilizing the copy state for better UXThe
useCopyToClipboardhook likely provides a state value (first destructured value) that could be used to enhance the UI feedback. Consider using it to show loading/success states directly in the menu item.- const [, copyToClipboard] = useCopyToClipboard(); + const [copyState, copyToClipboard] = useCopyToClipboard();Then update the menu item to reflect the state:
<DropdownMenuItem + disabled={copyState.copying} onClick={() => { copyToClipboard(identifier).then(() =>Also applies to: 22-22
apps/www/components/ui/copy-code-button.tsx (2)
16-16: Consider adding error handling for clipboard operations.While the implementation is clean, clipboard operations can fail due to permissions or browser security policies. Consider handling potential errors to provide feedback to users when copying fails.
- onClick={() => copyToClipboard(props.textToCopy)} + onClick={async () => { + try { + await copyToClipboard(props.textToCopy); + } catch (error) { + // Consider showing a user-friendly error message + console.error('Failed to copy:', error); + } + }}
3-7: Add JSDoc documentation for the component.Consider adding JSDoc documentation to describe the component's purpose, props, and usage examples. This would help other developers understand how to use this component effectively.
+/** + * A button component that copies text to clipboard with visual feedback. + * @param {Object} props + * @param {string} props.textToCopy - The text to be copied to clipboard + * @param {string} [props.className] - Optional CSS class for styling + * @example + * ```tsx + * <CopyCodeSnippetButton textToCopy="npm install @unkey/ui" /> + * ``` + */ type Props = { textToCopy: string; className?: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/menu.tsx(3 hunks)apps/dashboard/components/dashboard/copy-button.tsx(1 hunks)apps/dashboard/package.json(1 hunks)apps/www/components/copy-button.tsx(2 hunks)apps/www/components/ui/copy-code-button.tsx(1 hunks)apps/www/package.json(1 hunks)internal/tsconfig/react-library.json(1 hunks)internal/ui/README.md(1 hunks)internal/ui/package.json(1 hunks)internal/ui/postcss.config.js(1 hunks)internal/ui/src/hooks/index.ts(1 hunks)internal/ui/src/hooks/use-copy-to-clipboard.tsx(1 hunks)internal/ui/src/index.ts(1 hunks)internal/ui/tsconfig.json(1 hunks)knip.ts(1 hunks)packages/nextjs/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- internal/ui/README.md
- internal/ui/package.json
- internal/ui/postcss.config.js
- internal/ui/src/hooks/index.ts
- internal/ui/src/index.ts
- internal/ui/tsconfig.json
- packages/nextjs/package.json
🧰 Additional context used
📓 Learnings (2)
apps/dashboard/components/dashboard/copy-button.tsx (1)
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/dashboard/components/dashboard/copy-button.tsx:38-38
Timestamp: 2024-11-10T16:45:16.994Z
Learning: The `copyToClipboardWithMeta` function currently has an unused `_meta` parameter. Consider removing it or utilizing it appropriately, especially when introducing the `useCopyToClipboard` hook.
apps/www/components/copy-button.tsx (1)
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/dashboard/components/dashboard/copy-button.tsx:38-38
Timestamp: 2024-11-10T16:45:16.994Z
Learning: The `copyToClipboardWithMeta` function currently has an unused `_meta` parameter. Consider removing it or utilizing it appropriately, especially when introducing the `useCopyToClipboard` hook.
🔇 Additional comments (16)
internal/tsconfig/react-library.json (3)
6-6: LGTM! DOM lib and JSX configuration are appropriate.
The addition of "DOM" to lib array is essential for clipboard functionality, and "react-jsx" transform is the recommended setting for modern React development.
Also applies to: 9-9
10-10: LGTM! noEmit setting is appropriate for library packages.
The noEmit: true setting is correct for a UI library package as it prevents TypeScript from generating output files, leaving the bundling process to handle the emissions.
8-8: Verify browser compatibility requirements for ES2022.
The target has been updated from "es6" to "ES2022", which is a significant change. While this enables modern JavaScript features, it's important to verify that this aligns with the project's browser support requirements.
✅ Verification successful
Let me try another approach to gather information about browser compatibility requirements.
Let me try one more search to find any browser compatibility related configurations or documentation.
Based on the findings from our searches, I can now provide a final response:
ES2022 target appears safe for this project's context
The change from ES6 to ES2022 appears appropriate for this React library configuration because:
- The codebase shows consistent use of modern ECMAScript targets across multiple packages
- This is a React library configuration, which typically gets bundled/transpiled by the consuming application
- No explicit browser compatibility constraints were found in browserslist configs or documentation
- Other TypeScript configurations in the project already use modern targets like "ESNext"
The target will be processed by the consumer's build pipeline, allowing them to apply their own browser compatibility requirements during final bundling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check browser support requirements and current configurations
# Check for browserslist configuration
echo "Checking browserslist configuration..."
fd -t f -e json package.json --exec jq -r '.browserslist // empty'
fd -t f -e json .browserslistrc --exec cat
# Check for any existing polyfills or transpilation settings
echo "Checking for babel configuration..."
fd -t f -e json -e js babel.config --exec cat
fd -t f -e json .babelrc --exec cat
# Look for any documented browser support requirements
echo "Checking documentation for browser support..."
fd -t f README.md --exec grep -i -A 5 "browser.*support" || true
Length of output: 534
Script:
#!/bin/bash
# Check for any TypeScript configuration files that might define compilation targets
echo "Checking all TypeScript configurations..."
fd -t f -e json tsconfig --exec cat {}
# Look for any dependencies that might indicate browser support requirements
echo "Checking package.json for relevant dependencies..."
fd -t f package.json --exec jq -r '.dependencies, .devDependencies'
# Search for any comments or documentation about browser support in source files
echo "Searching for browser support comments in source..."
rg -i "browser.*(support|compat)" --type ts --type tsx
Length of output: 196851
Script:
#!/bin/bash
# Check for browser targets in package.json and any .browserslistrc files
echo "Checking for browserslist configuration..."
fd -t f -e json package.json --exec jq -r '.browserslist, .engines'
# Look for any documentation about browser support in README or docs
echo "Checking documentation..."
rg -i "browser|es202[0-9]|ecmascript" -t md
# Check for any build tool configurations that might specify browser targets
echo "Checking build configurations..."
fd -t f -e js -e json -e yaml -e yml -g "*config*" --exec grep -l -i "browser\|target\|es202[0-9]\|ecmascript"
Length of output: 2214
apps/dashboard/components/dashboard/copy-button.tsx (3)
3-3: LGTM! Clean import changes.
Good optimization by using type-only import for React and introducing the new hook from @unkey/ui.
Also applies to: 6-6
21-21: LGTM! Clean event handler implementation.
The onClick handler is concise and properly utilizes the hook's functionality.
Line range hint 13-27: Verify CopyButton usage across the codebase.
Since this is a shared component with modified behavior, let's verify its usage to ensure all consumers are compatible with the new implementation.
✅ Verification successful
CopyButton implementation is consistent across usages
The verification shows that:
- Both implementations (dashboard and www) have identical interfaces and behavior
- All usages only pass the required
valueprop and optional className/src - No consumers rely on custom clipboard behavior or additional props
- The component maintains consistent functionality using
useCopyToClipboardhook with a 2000ms timeout
The modified behavior is compatible with all current usage patterns in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of CopyButton component to verify compatibility
# Search for CopyButton imports and usages
echo "Searching for CopyButton imports and usages..."
rg -l "CopyButton" --type tsx
# Search for specific prop patterns that might need updates
echo "Checking for any custom clipboard-related props or handlers..."
rg "CopyButton.*onCopy" --type tsx
rg "CopyButton.*clipboard" --type tsx
Length of output: 428
Script:
#!/bin/bash
# Let's try without the file type restriction since tsx isn't recognized
echo "Searching for CopyButton imports and usages..."
rg -l "CopyButton"
echo "\nChecking for specific prop patterns..."
rg "CopyButton.*onCopy"
rg "CopyButton.*clipboard"
# Also search for the component definition to understand its interface
echo "\nSearching for CopyButton component definition..."
rg "interface CopyButtonProps|type CopyButtonProps"
Length of output: 2017
Script:
#!/bin/bash
# Let's examine both CopyButton implementations and their interfaces
echo "Examining CopyButton in dashboard:"
rg -A 10 "interface CopyButtonProps" apps/dashboard/components/dashboard/copy-button.tsx
echo "\nExamining CopyButton in www:"
rg -A 10 "interface CopyButtonProps" apps/www/components/copy-button.tsx
# Let's also check actual usage patterns
echo "\nChecking usage patterns in a few files:"
rg "CopyButton" apps/dashboard/app/new/keys.tsx -A 2
rg "CopyButton" apps/www/components/template/codeblock.tsx -A 2
Length of output: 2036
apps/www/components/copy-button.tsx (2)
3-3: LGTM! Import aligns with the PR objectives.
The addition of useCopyToClipboard from @unkey/ui supports the goal of centralizing UI utilities.
22-22: Verify removal of metadata tracking.
The simplified clipboard operation removes the previous metadata parameter. While this aligns with the previous learning about the unused _meta parameter, let's verify that no analytics or tracking was dependent on this metadata.
knip.ts (1)
40-42: LGTM! Configuration follows established patterns.
The workspace configuration for "internal/ui" follows the same structure as other internal packages, using "src/index.ts" as the entry point.
internal/ui/src/hooks/use-copy-to-clipboard.tsx (2)
9-14: LGTM!
The timer cleanup implementation is correct and follows best practices.
38-41: LGTM!
The cleanup effect is properly implemented and follows React best practices.
apps/www/package.json (2)
30-30: LGTM! Dependency addition looks correct.
The @unkey/ui package is correctly added with workspace:^ version specification, which is appropriate for monorepo internal packages.
30-30: Verify package configuration.
Let's ensure the new UI package is properly configured:
✅ Verification successful
Package configuration is correctly set up
The @unkey/ui package is properly configured in internal/ui with:
- Correct package name and workspace configuration
- Required peer dependencies (next, react, react-dom)
- TypeScript support with proper entry points (src/index.ts)
- Appropriate dev dependencies and build tools
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify @unkey/ui package configuration
# Check if package.json exists for @unkey/ui
if [ -f "packages/ui/package.json" ]; then
echo "Checking @unkey/ui package.json..."
# Extract and display peer dependencies
jq '.peerDependencies // empty' packages/ui/package.json
# Check for TypeScript configuration
if [ -f "packages/ui/tsconfig.json" ]; then
echo "TypeScript configuration exists"
cat packages/ui/tsconfig.json
fi
# Check for type definitions
find packages/ui -name "*.d.ts" -o -name "index.ts" -o -name "*.tsx"
else
echo "Error: @unkey/ui package.json not found"
exit 1
fi
Length of output: 135
Script:
#!/bin/bash
# Let's first check the actual location of the UI package
fd -t d "ui$" .
# Then check package.json files in any UI-related directories
fd -t f "package.json" . | grep -i "ui"
# List all workspace packages to understand the structure
cat pnpm-workspace.yaml 2>/dev/null || cat package.json
Length of output: 280
Script:
#!/bin/bash
# Let's check the package.json in internal/ui since that seems to be the correct location
cat internal/ui/package.json
# Also check if there are any TypeScript configurations
find internal/ui -name "*.d.ts" -o -name "index.ts" -o -name "*.tsx"
# Check the package name to ensure it matches @unkey/ui
jq '.name' internal/ui/package.json
Length of output: 1236
apps/dashboard/package.json (2)
58-58: LGTM! Dependency addition follows workspace conventions.
The @unkey/ui package is correctly added with the workspace version specifier, consistent with other internal packages.
58-58: Verify package configuration and build impact.
Since this is a new UI package, let's verify its configuration and impact on the build.
✅ Verification successful
The previous attempt failed because we assumed the wrong directory structure. Let's first locate the UI package directory and then analyze its configuration.
UI package configuration is properly set up and used across the workspace
The UI package is well-configured with:
- Proper peer dependencies for Next.js and React
- TypeScript and build tooling setup
- Clear exports through
src/index.ts - Currently used by dashboard and www apps
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package configuration and build impact
# Check if package.json exists for the new UI package
if [ -f "packages/ui/package.json" ]; then
echo "Checking UI package configuration..."
# Check for React peer dependencies
cat packages/ui/package.json | jq '.peerDependencies'
# Check package size
cat packages/ui/package.json | jq '.files'
# Check exports configuration
cat packages/ui/package.json | jq '.exports'
fi
# List all files in the UI package to understand its scope
fd . "packages/ui" -t f
Length of output: 160
Script:
#!/bin/bash
# Let's check if the package is under a different directory structure
fd -t d -d 1 ui
# Once we find the correct location, let's examine its package.json
fd package.json -t f | grep -i ui
Length of output: 78
Script:
#!/bin/bash
# Now that we found the correct path, let's analyze the package configuration
echo "=== UI Package Configuration ==="
cat internal/ui/package.json | jq '.'
echo -e "\n=== UI Package Structure ==="
fd . internal/ui -t f
echo -e "\n=== Workspace Dependencies ==="
# Check other packages that depend on @unkey/ui
fd package.json -t f -x grep -l "@unkey/ui"
Length of output: 1615
apps/www/components/ui/copy-code-button.tsx (1)
1-1: LGTM! Clean implementation using the new UI package.
The migration to useCopyToClipboard hook from @unkey/ui effectively simplifies the component by removing the need for local state management and timeout effects.
Also applies to: 9-9
| export const useCopyToClipboard = ( | ||
| timeout = 3000, | ||
| ): [boolean, (value: string | ClipboardItem) => Promise<void>] => { | ||
| const timer = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
| const [copied, setCopied] = useState(false); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add browser compatibility check for Clipboard API
The hook should verify browser support for the Clipboard API before attempting to use it. This would prevent runtime errors in unsupported browsers.
Consider adding this check at the beginning of the hook:
export const useCopyToClipboard = (
timeout = 3000,
): [boolean, (value: string | ClipboardItem) => Promise<void>] => {
+ const isClipboardAvailable = typeof navigator !== 'undefined' &&
+ navigator.clipboard !== undefined;
const timer = useRef<ReturnType<typeof setTimeout> | null>(null);
- const [copied, setCopied] = useState(false);
+ const [copied, setCopied] = useState<boolean>(false);
+ const [error, setError] = useState<Error | null>(null);
+
+ if (!isClipboardAvailable) {
+ console.warn('Clipboard API is not supported in this browser');
+ }📝 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.
| export const useCopyToClipboard = ( | |
| timeout = 3000, | |
| ): [boolean, (value: string | ClipboardItem) => Promise<void>] => { | |
| const timer = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| const [copied, setCopied] = useState(false); | |
| export const useCopyToClipboard = ( | |
| timeout = 3000, | |
| ): [boolean, (value: string | ClipboardItem) => Promise<void>] => { | |
| const isClipboardAvailable = typeof navigator !== 'undefined' && | |
| navigator.clipboard !== undefined; | |
| const timer = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| const [copied, setCopied] = useState<boolean>(false); | |
| const [error, setError] = useState<Error | null>(null); | |
| if (!isClipboardAvailable) { | |
| console.warn('Clipboard API is not supported in this browser'); | |
| } |
| copyToClipboard(identifier).then(() => | ||
| toast.success("Copied to clipboard", { | ||
| description: identifier, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Add error handling and improve user feedback
The current implementation lacks error handling for clipboard operations which could fail due to permissions or other issues. Also, the success message could be more specific.
Apply this improvement:
-copyToClipboard(identifier).then(() =>
- toast.success("Copied to clipboard", {
- description: identifier,
- }),
-);
+copyToClipboard(identifier)
+ .then(() =>
+ toast.success("Identifier copied to clipboard", {
+ description: identifier,
+ })
+ )
+ .catch((error) =>
+ toast.error("Failed to copy to clipboard", {
+ description: error.message,
+ })
+ );📝 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.
| copyToClipboard(identifier).then(() => | |
| toast.success("Copied to clipboard", { | |
| description: identifier, | |
| }), | |
| ); | |
| copyToClipboard(identifier) | |
| .then(() => | |
| toast.success("Identifier copied to clipboard", { | |
| description: identifier, | |
| }) | |
| ) | |
| .catch((error) => | |
| toast.error("Failed to copy to clipboard", { | |
| description: error.message, | |
| }) | |
| ); |
|
rn, the This update could be implemented in a few ways:
|
- add browser compatibility check for Clipboard API - propagate copy to clipboard error for higher-level handling - add error handling and improve user feedback to ratelimits logs menu
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
internal/ui/src/hooks/use-copy-to-clipboard.tsx (3)
11-16: Consider a more concise implementation of clearTimer.The function can be simplified while maintaining the same functionality.
- const clearTimer = () => { - if (timer.current) { - clearTimeout(timer.current); - timer.current = null; - } - }; + const clearTimer = () => { + timer.current && clearTimeout(timer.current); + timer.current = null; + };
33-40: Optimize timeout handling logic.The current implementation can be more efficient by avoiding duplicate setTimeout calls.
const handleTimeout = () => { + const timeoutValue = Number.isFinite(timeout) && timeout >= 0 + ? timeout + : DEFAULT_TIMEOUT; + + if (timeoutValue !== timeout) { + console.warn(`Invalid timeout value; defaulting to ${DEFAULT_TIMEOUT}ms`); + } + + timer.current = setTimeout(() => setCopied(false), timeoutValue); - if (Number.isFinite(timeout) && timeout >= 0) { - timer.current = setTimeout(() => setCopied(false), timeout); - } else { - console.warn(`Invalid timeout value; defaulting to ${DEFAULT_TIMEOUT}ms`); - timer.current = setTimeout(() => setCopied(false), DEFAULT_TIMEOUT); - } };
42-55: Enhance error handling with more specific error messages.The current error logging could be more informative for debugging purposes.
} catch (error) { - console.warn("Failed to copy to clipboard. ", error); + const errorMessage = error instanceof Error + ? error.message + : 'Unknown error occurred'; + console.warn(`Failed to copy to clipboard: ${errorMessage}`); throw error; // Propagate error for higher-level handling }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/menu.tsx(3 hunks)internal/ui/src/hooks/use-copy-to-clipboard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/menu.tsx
🔇 Additional comments (4)
internal/ui/src/hooks/use-copy-to-clipboard.tsx (4)
1-9: LGTM! Well-structured hook signature and state management.
The hook's interface is clear and type-safe, with appropriate default values and state management setup.
18-31: LGTM! Robust clipboard operation implementation.
The implementation includes proper browser compatibility checks and type handling as suggested in the previous review.
57-60: LGTM! Proper cleanup implementation.
The cleanup effect is well-implemented to prevent memory leaks.
62-63: LGTM! Clear and type-safe return value.
The return statement correctly matches the hook's type signature.
| "typescript": "^5.5.3" | ||
| }, | ||
| "dependencies": {}, | ||
| "author": "Nazar Poshtarenko <[email protected]>", |
There was a problem hiding this comment.
btw, not sure about this line. let me know if that’s okay or if you prefer it limited to core contributors.
There was a problem hiding this comment.
and since it's an internal package, one may find this info excessive. I'm ok with refining this file to keep it clean and consistent w/ other packages
What does this PR do?
Introduces an internal package,
@unkey/ui, to the project. The primary purpose of this package is to encapsulate UI components and utilities related to Unkey's functionality, starting with the addition of a new custom React hook.Key Details:
navigator.clipboardusages with the new hook throughout all the web apps.Type of change
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Release Notes
New Features
useCopyToClipboard, for improved clipboard functionality across various components.Bug Fixes
Documentation
@unkey/uilibrary, detailing installation and usage instructions.Chores
@unkey/uiacross relevant applications.