Skip to content

Util classname 2 #631

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Dec 11, 2024
Merged

Util classname 2 #631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
080dc86
Util classname 2
GoldGroove06 Dec 11, 2024
de2c89f
Update src/components/ui/RadioGroup/RadioGroup.tsx
GoldGroove06 Dec 11, 2024
8d90139
util classname 2
GoldGroove06 Dec 11, 2024
a20e15e
Merge branch 'util-function-classname-2' of https://github.com/GoldGr…
GoldGroove06 Dec 11, 2024
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
4 changes: 2 additions & 2 deletions src/components/ui/Collapsible/Collapsible.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import ButtonPrimitive from '~/core/primitives/Button';
export type CollapsibleProps = { open?: boolean, title?: string, trigger?: ReactNode} & PropsWithChildren;

const ExpandIcon = () => (
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" className="size-6">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" className='size-6'>
<path strokeLinecap="round" strokeLinejoin="round" d="M3.75 3.75v4.5m0-4.5h4.5m-4.5 0L9 9M3.75 20.25v-4.5m0 4.5h4.5m-4.5 0L9 15M20.25 3.75h-4.5m4.5 0v4.5m0-4.5L15 9m5.25 11.25h-4.5m4.5 0v-4.5m0 4.5L15 15" />
</svg>
);

const CollapseIcon = () => (
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" className="size-6">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" className='size-6'>
<path strokeLinecap="round" strokeLinejoin="round" d="M9 9V4.5M9 9H4.5M9 9 3.75 3.75M9 15v4.5M9 15H4.5M9 15l-5.25 5.25M15 9h4.5M15 9V4.5M15 9l5.25-5.25M15 15h4.5M15 15v4.5m0-4.5 5.25 5.25" />
</svg>
);
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/Em/Em.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import React from 'react';
import { customClassSwitcher } from '~/core';

import { clsx } from 'clsx';
const COMPONENT_NAME = 'Em';

export type EmProps = {
Expand All @@ -13,7 +13,7 @@ export type EmProps = {

const Em = ({ children, customRootClass, className, ...props }: EmProps) => {
const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME);
return <em className={`${rootClass} ${className}`} {...props}>
return <em className={clsx(rootClass, className)} {...props}>
{children}
</em>;
};
Expand Down
6 changes: 3 additions & 3 deletions src/components/ui/Heading/Heading.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client';
import React from 'react';

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

const RENDER_AS_ENUMS = [
Expand Down Expand Up @@ -35,9 +35,9 @@ const Heading = ({ children, as = undefined, customRootClass = '', className = '

if (as !== undefined && RENDER_AS_ENUMS.find((item) => item.tag === as)) {
const { tag: Tag } = RENDER_AS_ENUMS.find((item) => item.tag === as);
return <Tag className={`${rootClass} ${className}`} {...props}>{children}</Tag>;
return <Tag className={clsx(rootClass, className)} {...props}>{children}</Tag>;
}
return <h1 className={`${rootClass} ${className}`} {...props}>{children}</h1>;
return <h1 className={clsx(rootClass, className)} {...props}>{children}</h1>;
};
Heading.displayName = 'Heading';

Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/HoverCard/fragments/HoverCardArrow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import React, { useContext } from 'react';

import { clsx } from 'clsx';
import Floater from '~/core/primitives/Floater';
import HoverCardContext from '../contexts/HoverCardContext';

const HoverCardArrow = ({ ...props }) => {
const { floatingContext, arrowRef, rootClass } = useContext(HoverCardContext);

return <Floater.Arrow className={`${rootClass}-arrow`} {...props} context={floatingContext} ref={arrowRef} />;
return <Floater.Arrow className={clsx(`${rootClass}-arrow`)} {...props} context={floatingContext} ref={arrowRef} />;
};

export default HoverCardArrow;
4 changes: 2 additions & 2 deletions src/components/ui/HoverCard/fragments/HoverCardContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useContext, useEffect } from 'react';

import { clsx } from 'clsx';
import HoverCardContext from '../contexts/HoverCardContext';

type HoverCardContentProps = {
Expand Down Expand Up @@ -32,7 +32,7 @@ const HoverCardContent = ({ children, ...props }: HoverCardContentProps) => {
return <div
onPointerEnter={openWithDelay}
onPointerLeave={closeWithDelay}
className={`${rootClass}`} {...props}
className={clsx(rootClass)} {...props}
ref={floatingRefs.setFloating}
style={floatingStyles}
{...getFloatingProps()}>{children}</div>;
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/HoverCard/fragments/HoverCardRoot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { useState, useRef } from 'react';
import HoverCardContext from '../contexts/HoverCardContext';
import Floater from '~/core/primitives/Floater';
import { customClassSwitcher } from '~/core';

import { clsx } from 'clsx';
const COMPONENT_NAME = 'HoverCard';

type HoverCardRootProps = {
Expand Down Expand Up @@ -114,7 +114,7 @@ const HoverCardRoot = ({ children, open: controlledOpen = undefined, onOpenChang
};

return <HoverCardContext.Provider value={sendValues}>
<div className={rootClass} {...props}>{children}</div>
<div className={clsx(rootClass)} {...props}>{children}</div>
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 11, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unnecessary clsx usage

Similar to other files, wrapping a single static variable in clsx doesn't provide any benefits here.

-className={clsx(rootClass)}
+className={rootClass}

Consider a more strategic approach to className management

Based on the changes across multiple files, it seems the goal is to improve className management. However, clsx provides the most value when:

  1. Merging multiple classes conditionally
  2. Handling undefined/null class names
  3. Managing complex conditional styling logic

Consider using clsx only where these benefits are needed. For example:

className={clsx(
  rootClass,
  {
    'state-open': isOpen,
    'state-disabled': disabled
  },
  customClass
)}

</HoverCardContext.Provider>;
};

Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useContext } from 'react';

import { clsx } from 'clsx';
import HoverCardContext from '../contexts/HoverCardContext';

import Primitive from '~/core/primitives/Primitive';
Expand All @@ -14,7 +14,7 @@ const HoverCardTrigger = ({ children, className = '', ...props }: HoverCardTrigg

return <>
<Primitive.span
className={`${rootTriggerClass} ${className}`}
className={clsx(rootTriggerClass, className)}
onClick={() => {}}
onMouseEnter={openWithDelay} onMouseLeave={closeWithDelay}
ref={floatingRefs.setReference}
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/Kbd/Kbd.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import React from 'react';
import { customClassSwitcher } from '~/core';

import { clsx } from 'clsx';
const COMPONENT_NAME = 'Kbd';

export type KbdProps = {
Expand All @@ -13,7 +13,7 @@ export type KbdProps = {

const Kbd = ({ children, customRootClass, className, ...props }: KbdProps) => {
const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME);
return <kbd className={`${rootClass} ${className}`} {...props}>{children}</kbd>;
return <kbd className={clsx(rootClass, className)} {...props}>{children}</kbd>;
};

export default Kbd;
6 changes: 3 additions & 3 deletions src/components/ui/Link/Link.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client';
import React from 'react';

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

const COMPONENT_NAME = 'Link';
Expand All @@ -15,11 +15,11 @@ export type LinkProps = {
}

// TODO: in the previous return value
// return <a href={href} alt={alt} className={`${rootClass} ${className}`} {...props}>{children}</a>;
// return <a href={href} alt={alt} className={clsx(rootClass, className)} {...props}>{children}</a>;
// 'alt' prop does not exist on an anchor element
const Link = ({ children, href = '#', alt, customRootClass, className, ...props }: LinkProps) => {
const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME);
return <a href={href} className={`${rootClass} ${className}`} {...props}>{children}</a>;
return <a href={href} className={clsx(rootClass, className)} {...props}>{children}</a>;
};

Link.displayName = COMPONENT_NAME;
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/Progress/fragments/ProgressIndicator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useEffect } from 'react';
import { ProgressProps, COMPONENT_NAME } from '../Progress';
import { customClassSwitcher } from '~/core';

import { clsx } from 'clsx';
interface IndicatorProps
extends Pick<
ProgressProps,
Expand All @@ -28,7 +28,7 @@ export default function ProgressIndicator({
return (
<div
role="progressbar"
className={`${rootClass}-indicator`}
className={clsx(`${rootClass}-indicator`)}
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 11, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Simplify the className assignment

The current implementation unnecessarily wraps a template literal in clsx. Since there's no conditional logic or class merging needed here, the template literal alone is sufficient.

-className={clsx(`${rootClass}-indicator`)}
+className={`${rootClass}-indicator`}
📝 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
className={clsx(`${rootClass}-indicator`)}
className={`${rootClass}-indicator`}

style={{ transform: `translateX(-${maxValue - value}%)` }}
aria-valuenow={value}
aria-valuemax={maxValue}
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/Progress/fragments/ProgressRoot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';

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

import { ProgressProps, COMPONENT_NAME } from '../Progress';
Expand All @@ -9,5 +9,5 @@ interface ProgressRootProps extends Partial<ProgressProps> {}
export default function ProgressRoot({ children, customRootClass }: ProgressRootProps) {
const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME);

return (<div className={rootClass}>{children}</div>);
return (<div className={clsx(rootClass)}>{children}</div>);
}
4 changes: 2 additions & 2 deletions src/components/ui/Quote/Quote.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import React from 'react';
import { customClassSwitcher } from '~/core';

import { clsx } from 'clsx';
const COMPONENT_NAME = 'Quote';

export type QuoteProps = {
Expand All @@ -13,7 +13,7 @@ export type QuoteProps = {

const Quote = ({ children, customRootClass, className, ...props }: QuoteProps) => {
const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME);
return <q className={`${rootClass} ${className}`} {...props}>{children}</q>;
return <q className={clsx(rootClass, className)} {...props}>{children}</q>;
};

Quote.displayName = COMPONENT_NAME;
Expand Down
7 changes: 6 additions & 1 deletion src/components/ui/RadioGroup/RadioGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import React, { DetailedHTMLProps, InputHTMLAttributes, PropsWithChildren } from
import { customClassSwitcher } from '~/core';
import RadioPrimitive from '~/core/primitives/Radio';
const COMPONENT_NAME = 'RadioGroup';
import React, { DetailedHTMLProps, InputHTMLAttributes, PropsWithChildren } from 'react';
Copy link
Collaborator

@kotAPI kotAPI Dec 11, 2024

Choose a reason for hiding this comment

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

Bug: Duplicate declarations found

import { clsx } from 'clsx';
import { customClassSwitcher } from '~/core';
import RadioPrimitive from '~/core/primitives/Radio';
const COMPONENT_NAME = 'RadioGroup';

export type RadioGroupProps = {

Expand All @@ -14,7 +19,7 @@ const RadioGroup = ({ children, type = 'radio', className = '', customRootClass
const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME);

return (
<div className={`${rootClass} ${className}`} role='radiogroup'>
<div className={clsx(rootClass, className)} role='radiogroup'>
<RadioPrimitive
type={type}
{...props}>
Expand Down
Loading