Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
17 changes: 7 additions & 10 deletions src/components/Button.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from "react";

import clsx from "clsx";
import { twMerge } from "tailwind-merge";
import { cn } from "@/utils/cn";

type ButtonVariant = "primary" | "white";
type ButtonTextSize = "lg" | "md" | "sm";
Expand Down Expand Up @@ -38,14 +37,12 @@ export default function Button({
}: ButtonProps) {
const baseClasses = "rounded-md";

const mergedClasses = twMerge(
clsx(
baseClasses,
textSizeClassMap[textSize],
disabled ? disabledClass : variantClassMap[variant],
fullWidth && "w-full",
className,
),
const mergedClasses = cn(
baseClasses,
textSizeClassMap[textSize],
disabled ? disabledClass : variantClassMap[variant],
fullWidth && "w-full",
className,
);

return (
Expand Down
192 changes: 192 additions & 0 deletions src/components/Select.tsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Select 컴포넌트의 리스트가 나타나고 난 뒤, 같은 가로 열 어딘가를 클릭했을 때 닫히지 않는 버그가 있습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

그리고 버튼이나 리스트에 호버했을 때, 마우스 커서가 pointer로 변경되면 좋을 것 같아요! 🤔

Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
import {
useState,
useEffect,
useRef,
ReactNode,
SelectHTMLAttributes,
} from "react";

import { DropdownDown, DropdownUp } from "@/assets/icon";
import { cn } from "@/utils/cn";

const sizeMap = {
lg: "py-4 px-5 text-[1rem]",
sm: "p-2.5 text-sm",
} as const;

interface Option {
label: string;
value: string;
}

interface SelectProps
extends Omit<
SelectHTMLAttributes<HTMLButtonElement>,
"size" | "onChange" | "disabled"
> {
id?: string;
label?: string;
options: Option[];
value?: string;
onValueChange?: (value: string) => void;
placeholder?: string;
size?: keyof typeof sizeMap;
fullWidth?: boolean;
width?: number | string;
className?: string;
wrapperClassName?: string;
}

// 라벨과 입력 영역을 감싸는 공통 컴포넌트
function Field({
id,
label,
children,
}: {
id?: string;
label?: string;
children: ReactNode;
}) {
return (
<div>
{label && (
<label htmlFor={id} className="inline-block mb-2 leading-[1.625rem]">
{label}
</label>
)}
{children}
</div>
);
}

function Select({
id,
label,
options,
value,
onValueChange,
placeholder = "선택",
size = "lg",
fullWidth,
width,
Copy link
Collaborator

Choose a reason for hiding this comment

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

width, fullWidth 속성이 없어도 className으로 제어 할 수 있을 것 같아요!
리스트의 가로 너비도 useEffect로 동기화 시켜주고 있으니까요 하하 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fullWidth 같은 경우 TextField 컴포넌트와 통일성을 맞추기 위해 유지하는 게 더 좋을 것 같습니당. width는 말씀하신 것처럼 className으로 제어하는 게 더 좋아보이네요! 리뷰 내용 반영해 PR 올리겠습니당

className,
wrapperClassName,
...rest
}: SelectProps) {
const [open, setOpen] = useState(false);
const [buttonWidth, setButtonWidth] = useState<number>(0);

const wrapperRef = useRef<HTMLDivElement>(null);
const buttonRef = useRef<HTMLButtonElement>(null);

const selectedOption = options.find((option) => option.value === value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

어느 정도 크기의 options prop이 전달될 지 모르니
useMemo로 최적화 해봐도 좋을 것 같아요! 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현재 사용하는 옵션 수는 기껏해야 20개 정도라서 성능 이슈가 없다고 판단했습니다!

불필요한 deps 배열 관리로 인한 가독성 저하를 막는 것이 현재로서는 최적화보다 우선이라는 판단 하에 useMemo를 사용하지 않고 작성하였는데 .. 혹시 다른 의견이 있으시다면 더 제안해주시면 감사하겠습니다!

Copy link
Collaborator

@cozy-ito cozy-ito Apr 29, 2025

Choose a reason for hiding this comment

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

가독성 부분은 selectedOption이라는 변수명에서 어떤 역할을 하는지에 대해서 잘 나타내고 있다고 저는 생각을 했습니다 하핳.
때문에 현재는 서울권 지역만 해당되어 20가지이지만,
본래는 전역권의 지역일테니 미리 최적화 코드를 적용하는 것이 어떨까 했습니다 😅
적용 여부는 조금 더 고민해 봐 주시고 자유롭게 결정해주셔도 좋을 것 같아요! 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

흠... 맞는 말입니다ㅎㅎ 피그마에 구현된 수준에서만 고민했던 것 같네요 그럼 적용해보겠습니다!~


const wrapperClassNames = cn(
"relative",
{
"w-full": fullWidth,
},
wrapperClassName,
);

const buttonClassNames = cn(
"flex items-center justify-between rounded-[0.375rem]",
{
"w-full": fullWidth,
"bg-white placeholder:text-gray-40 border border-gray-30": size === "lg",
"bg-gray-10 font-bold": size === "sm",
},
sizeMap[size],
className,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

span 태그를 사용하지 않고, button 태그의 className속성으로만 placeholder의 스타일을 제어하는 건 어떨까요? 🤔
그리고 value가 존재한다면, text-black이 적용 되어야 합니다. 😅

Suggested change
const buttonClassNames = cn(
"flex items-center justify-between rounded-[0.375rem]",
{
"w-full": fullWidth,
"bg-white placeholder:text-gray-40 border border-gray-30": size === "lg",
"bg-gray-10 font-bold": size === "sm",
},
sizeMap[size],
className,
);
const buttonClassNames = cn(
"flex items-center justify-between rounded-[0.375rem]",
{
"w-full": fullWidth,
"bg-white placeholder:text-gray-40 border border-gray-30": size === "lg",
"bg-gray-10 font-bold": size === "sm",
},
value ? "text-black" : "text-gray-40", // 추가
sizeMap[size],
className,
);
<button
  id={id}
  type="button"
  ref={buttonRef}
  onClick={() => setOpen((prev) => !prev)}
  className={buttonClassNames}
  style={width ? { width } : undefined}
  {...rest}
>
  {/* <span className={cn(value ? "" : "text-gray-40")}> */}
    {selectedOption?.label || placeholder}
  {/* </span> */}
  {open ? <DropdownUp className="ml-2" /> : <DropdownDown className="ml-2" />}
</button>


const listClassNames = cn(
"absolute top-full left-0 mt-1 border rounded-[0.375rem] bg-white border-gray-30 shadow-lg z-10 max-h-48 overflow-y-auto",
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const listClassNames = cn(
"absolute top-full left-0 mt-1 border rounded-[0.375rem] bg-white border-gray-30 shadow-lg z-10 max-h-48 overflow-y-auto",
);
const listClassNames = cn(
"absolute top-full left-0 mt-1 border rounded-[0.375rem] bg-white border-gray-30 text-black shadow-lg z-10 max-h-48 overflow-y-auto",
);

리스트 아이템에 text-black 스타일이 적용되어 있지 않아요.
현재는 #000000이지만, 수민님께서 등록해주신 테마 색상인 text-black은 엄연히 #111322로 다르니까요! 👍

텍스트와 관련된 속성은 대부분 하위 자식요소까지 상속되기 때문에 리스트에 클래스명을 주면 좋을 것 같습니다! 😄


const handleSelect = (selectedValue: string) => {
onValueChange?.(selectedValue);
setOpen(false);
};

// 드롭다운 외부 클릭 시 닫기
useEffect(() => {
const handleClickOutside = (event: MouseEvent) => {
if (
wrapperRef.current &&
!wrapperRef.current.contains(event.target as Node)
) {
setOpen(false);
}
};

document.addEventListener("mousedown", handleClickOutside);
return () => {
document.removeEventListener("mousedown", handleClickOutside);
};
}, []);

// 버튼 너비 측정 (드롭다운 너비 일치시키기 위함)
useEffect(() => {
if (buttonRef.current) {
const rect = buttonRef.current.getBoundingClientRect();
setButtonWidth(rect.width);
}
}, [open, fullWidth, size, value]); // 버튼 사이즈가 변할 수 있는 경우

return (
<Field id={id} label={label}>
<div className={wrapperClassNames} ref={wrapperRef}>
<button
id={id}
type="button"
ref={buttonRef}
onClick={() => setOpen((prev) => !prev)}
className={buttonClassNames}
style={width ? { width } : undefined}
{...rest}
>
<span className={cn(value ? "" : "text-gray-40")}>
{selectedOption?.label || placeholder}
</span>
{open ? (
<DropdownUp className="ml-2" />
) : (
<DropdownDown className="ml-2" />
)}
</button>

{open && (
<ul
className={listClassNames}
style={{
width: width || buttonWidth,
}}
>
{options.map((option) => (
<li
key={option.value}
className="border-b border-gray-20 last:border-0"
>
<button
type="button"
className={cn(
"w-full text-center hover:bg-gray-10",
size === "sm"
? "px-3 py-2 text-sm"
: "px-5 py-3 text-[1rem]",
)}
onClick={() => handleSelect(option.value)}
>
{option.label}
</button>
</li>
))}
</ul>
)}
</div>
</Field>
);
}

export default Select;
24 changes: 24 additions & 0 deletions src/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,27 @@
@import "tailwindcss";
@import "./styles/base.css";
@import "./styles/utilities.css";

::-webkit-scrollbar {
width: 12px;
}

::-webkit-scrollbar-thumb {
background-color: #7d7986;
border: 4px solid transparent;
background-clip: padding-box;
border-radius: 9999px;
min-height: 60px;
}

::-webkit-scrollbar-thumb:hover {
background-color: #636169;
}

::-webkit-scrollbar-track {
background: transparent;
}

::-webkit-scrollbar-button {
display: none;
}