-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Util classname 1 #616
Util classname 1 #616
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import React, { useContext } from 'react'; | ||
import { AccordionContext } from '../contexts/AccordionContext'; | ||
import { AccordionItemContext } from '../contexts/AccordionItemContext'; | ||
import { clsx } from 'clsx'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize clsx usage by removing template literal The current implementation uses a template literal within - className={clsx(`${rootClass}-trigger`, className)}
+ className={clsx(rootClass + '-trigger', className)} Or even better, if you want to maintain consistency across the codebase: - className={clsx(`${rootClass}-trigger`, className)}
+ className={clsx([rootClass + '-trigger', className])} Also applies to: 35-35 💡 Codebase verification Template literals in clsx calls need to be replaced with string concatenation The verification revealed inconsistent className handling patterns across the codebase:
These should be simplified to 🔗 Analysis chainVerify consistent className handling patterns Let's check for consistency in className handling across the codebase. Also applies to: 35-35 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for different className handling patterns
# Check for template literals within clsx calls
echo "Checking for template literals within clsx..."
rg "clsx\(`\${.*?}`" --type typescript
# Check for direct string concatenation in className
echo "Checking for string concatenation in className..."
rg "className=.*?\+.*?" --type typescript
# Check for potential mixing of different className utilities
echo "Checking for mixed usage of className utilities..."
rg "(clsx|classnames|cn)\(" --type typescript
Length of output: 645 Script: #!/bin/bash
# Description: Check for different className handling patterns with correct file types
# Check for template literals within clsx calls
echo "Checking for template literals within clsx..."
rg "clsx\(`.*?\$" -t tsx -t jsx -t ts -t js
# Check for direct string concatenation in className
echo "Checking for string concatenation in className..."
rg "className=.*?\+" -t tsx -t jsx -t ts -t js
# Check for potential mixing of different className utilities
echo "Checking for mixed usage of className utilities..."
rg "(clsx|classnames|cn)\(" -t tsx -t jsx -t ts -t js
# Check for files using clsx
echo "Files using clsx..."
fd -e tsx -e jsx -e ts -e js -x rg -l "from ['\"]clsx['\"]"
Length of output: 217 Script: #!/bin/bash
# Description: Check for different className handling patterns with correct file types
# Check for template literals within clsx calls
echo "Checking for template literals within clsx..."
rg "clsx\(\`" -t tsx -t jsx -t ts -t js
# Check for direct string concatenation in className
echo "Checking for string concatenation in className..."
rg "className=\".*?\+" -t tsx -t jsx -t ts -t js
# Check for potential mixing of different className utilities
echo "Checking for mixed usage of className utilities..."
rg "from ['\"](?:clsx|classnames)['\"]" -t tsx -t jsx -t ts -t js
# List all className assignments
echo "Checking className patterns..."
rg "className=" -t tsx -t jsx -t ts -t js -C 1
Length of output: 713 Script: #!/bin/bash
# Description: Check for different className handling patterns
# Check for template literals within clsx calls
echo "Checking for template literals within clsx..."
rg "clsx\(\`"
# Check for direct string concatenation in className
echo "Checking for string concatenation in className..."
rg "className=\".*?\+"
# Check for potential mixing of different className utilities
echo "Checking for mixed usage of className utilities..."
rg "from ['\"](?:clsx|classnames)['\"]"
# List files with className assignments
echo "Files with className patterns..."
rg -l "className="
Length of output: 10305 |
||
|
||
type AccordionTriggerProps = { | ||
children: React.ReactNode; | ||
|
@@ -31,7 +32,7 @@ const AccordionTrigger: React.FC<AccordionTriggerProps> = ({ children, index, cl | |
|
||
<button | ||
type="button" | ||
className={`${rootClass}-trigger ${className}`} | ||
className={clsx(`${rootClass}-trigger`, className)} | ||
onBlur={handleBlurEvent} | ||
onFocus={onFocusHandler} | ||
onKeyDown={(e) => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import React, { useState } from 'react'; | ||
import { customClassSwitcher } from '~/core'; | ||
import { AlertDialogContext } from '../contexts/AlertDialogContext'; | ||
|
||
import { clsx } from 'clsx'; | ||
import Floater from '~/core/primitives/Floater'; | ||
|
||
export type AlertDialogRootProps = { | ||
|
@@ -31,7 +31,7 @@ const AlertDialogRoot = ({ children, customRootClass = '', open, onOpenChange, o | |
const props = { isOpen, handleOpenChange, floaterContext, rootClass, handleOverlayClick }; | ||
return ( | ||
<AlertDialogContext.Provider value={props}> | ||
<div className={rootClass} > | ||
<div className={clsx(rootClass)} > | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dont need |
||
{children} | ||
</div> | ||
</AlertDialogContext.Provider> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import React from 'react'; | ||
|
||
import { clsx } from 'clsx'; | ||
import AvatarPrimitive from '~/core/primitives/Avatar'; | ||
|
||
const COMPONENT_NAME = 'Avatar'; | ||
|
@@ -20,7 +20,7 @@ const Avatar = ({ customRootClass = '', fallback, className, src, alt, ...props | |
<AvatarPrimitive.Image | ||
src={src} | ||
alt={alt} | ||
className={className} | ||
className={clsx(className)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dont need |
||
{...props} | ||
/> | ||
<AvatarPrimitive.Fallback> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import React from 'react'; | ||
|
||
import { clsx } from 'clsx'; | ||
import AvatarGroupRoot from './fragments/AvatarGroupRoot'; | ||
import AvatarPrimitiveRoot from '~/core/primitives/Avatar/fragments/AvatarPrimitiveRoot'; | ||
import AvatarPrimitiveFallback from '~/core/primitives/Avatar/fragments/AvatarPrimitiveFallback'; | ||
|
@@ -18,7 +18,7 @@ type AvatarGroupProps = { | |
} | ||
|
||
const AvatarGroup = ({ avatars = [], size, customRootClass = '', className, ...props }: AvatarGroupProps) => { | ||
return <AvatarGroupRoot customRootClass={customRootClass} className={className} {...props} > | ||
return <AvatarGroupRoot customRootClass={customRootClass} className={clsx(className)} {...props} > | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dont need it here |
||
{avatars.map((avatar, index) => ( | ||
<AvatarPrimitiveRoot key={index} src={avatar.src}> | ||
<AvatarPrimitiveImage src={avatar.src} alt={avatar.alt} /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
import React from 'react'; | ||
import BadgeRoot from './fragments/BadgeRoot'; | ||
import BadgeContent from './fragments/BadgeContent'; | ||
import { clsx } from 'clsx'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Simplify unnecessary clsx usage The current usage of - return <BadgeRoot customRootClass={customRootClass} className={clsx(className)} color={color ?? undefined} {...props}>
+ return <BadgeRoot customRootClass={customRootClass} className={className} color={color ?? undefined} {...props}> Also applies to: 15-15 |
||
export type BadgeProps = { | ||
children?: React.ReactNode, | ||
customRootClass?: string, | ||
|
@@ -11,7 +12,7 @@ export type BadgeProps = { | |
} | ||
|
||
const Badge = ({ children, customRootClass, className, color, ...props }: BadgeProps) => { | ||
return <BadgeRoot customRootClass={customRootClass} className={`${className}`} color={color ?? undefined} {...props}> | ||
return <BadgeRoot customRootClass={customRootClass} className={clsx(className)} color={color ?? undefined} {...props}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dont need here |
||
|
||
<BadgeContent> | ||
{children} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import React from 'react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Props typing issues confirmed in multiple components The following components have incorrect props typing where
These components use rest parameters ( 🔗 Analysis chainVerify props typing across all components. Similar props typing issues were found in multiple components. It would be beneficial to verify and fix this pattern across the entire codebase. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for potentially incorrect props type definitions
rg "props:.*\[\]" "src/components/"
Length of output: 303 Script: #!/bin/bash
# Let's check the actual component props definitions in these files
echo "=== Link.tsx ==="
ast-grep --pattern 'interface $_ {
$$$
}' src/components/ui/Link/Link.tsx
echo -e "\n=== Em.tsx ==="
ast-grep --pattern 'interface $_ {
$$$
}' src/components/ui/Em/Em.tsx
echo -e "\n=== Kbd.tsx ==="
ast-grep --pattern 'interface $_ {
$$$
}' src/components/ui/Kbd/Kbd.tsx
echo -e "\n=== BlockQuote.tsx ==="
ast-grep --pattern 'interface $_ {
$$$
}' src/components/ui/BlockQuote/BlockQuote.tsx
Length of output: 513 Script: #!/bin/bash
# Let's check the component definitions directly
echo "=== Link.tsx ==="
rg -A 10 "type.*Props.*=|interface.*Props" src/components/ui/Link/Link.tsx
echo -e "\n=== Em.tsx ==="
rg -A 10 "type.*Props.*=|interface.*Props" src/components/ui/Em/Em.tsx
echo -e "\n=== Kbd.tsx ==="
rg -A 10 "type.*Props.*=|interface.*Props" src/components/ui/Kbd/Kbd.tsx
echo -e "\n=== BlockQuote.tsx ==="
rg -A 10 "type.*Props.*=|interface.*Props" src/components/ui/BlockQuote/BlockQuote.tsx
Length of output: 2031 |
||
|
||
import { clsx } from 'clsx'; | ||
/** | ||
* Shards | ||
*/ | ||
|
@@ -15,7 +15,7 @@ export type CalloutProps = { | |
|
||
const COMPONENT_NAME = 'Callout'; | ||
const Callout = ({ children, className = '', color, customRootClass, ...props }: CalloutProps) => { | ||
return (<CalloutRoot customRootClass={customRootClass} className={`${className}`} color={color ?? undefined} {...props}> | ||
return (<CalloutRoot customRootClass={customRootClass} className={clsx(className)} color={color ?? undefined} {...props}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dont need here since its direct usage |
||
{children} | ||
</CalloutRoot>); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,14 +1,14 @@ | ||||||||||||||||||
import React, { PropsWithChildren } from 'react'; | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Based on the shell script results, I can now generate the final response: Inconsistent className handling patterns detected The codebase shows mixed patterns for handling classNames:
🔗 Analysis chainVerify consistent className handling across components Let's verify that all UI components follow the same pattern for className handling. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for consistent className handling patterns across UI components
# Find all UI component files
echo "Checking className handling patterns..."
rg -l "className=" src/components/ui/ | while read -r file; do
echo "\nFile: $file"
# Show className related code with context
rg -A 2 -B 2 "className=" "$file"
done
# Verify base class naming convention
echo "\nChecking base class naming convention..."
rg "className=.*['\"]rui-" src/components/ui/
Length of output: 46635 |
||||||||||||||||||
import CardRoot from './fragments/CardRoot'; | ||||||||||||||||||
|
||||||||||||||||||
import { clsx } from 'clsx'; | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be removed since its direct usage |
||||||||||||||||||
export type CardProps = { | ||||||||||||||||||
customRootClass?: string; | ||||||||||||||||||
className?: string; | ||||||||||||||||||
props?: any; | ||||||||||||||||||
Comment on lines
4
to
7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid using 'any' type and consider consolidating className props
export type CardProps = {
- customRootClass?: string;
className?: string;
- props?: any;
+ props?: React.ComponentProps<'div'>;
} & React.ComponentProps<'div'>; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
} & React.ComponentProps<'div'>; | ||||||||||||||||||
|
||||||||||||||||||
const Card = ({ children, className = '', customRootClass, ...props }: PropsWithChildren<CardProps>) => ( | ||||||||||||||||||
<CardRoot className={className} customRootClass={customRootClass} {...props}> | ||||||||||||||||||
<CardRoot className={clsx(className)} customRootClass={customRootClass} {...props}> | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we dont need this here |
||||||||||||||||||
{children} | ||||||||||||||||||
Comment on lines
10
to
12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Simplify props handling by removing customRootClass After consolidating the className props, update the component implementation: -const Card = ({ children, className = '', customRootClass, ...props }: PropsWithChildren<CardProps>) => (
- <CardRoot className={clsx(className)} customRootClass={customRootClass} {...props}>
+const Card = ({ children, className = '', ...props }: PropsWithChildren<CardProps>) => (
+ <CardRoot className={clsx('rui-card', className)} {...props}> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
</CardRoot> | ||||||||||||||||||
); | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
'use client'; | ||
import React from 'react'; | ||
|
||
import { clsx } from 'clsx'; | ||
export type CodeProps= { | ||
children: React.ReactNode; | ||
} | ||
|
||
const Code = ({ children }: CodeProps) => { | ||
return <code className='rui-code-root'> | ||
return <code className={clsx('rui-code-root')}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing: customRootClass on Code component There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent use of class prefix - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ignore above comments, referenced new issues for them |
||
{children} | ||
</code>; | ||
}; | ||
|
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.
dont need