[FEAT] Dropdown 컴포넌트, 스토리북 #49
Conversation
Walkthrough이 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (12)
src/components/common/Dropdown/Dropdown.variants.ts (2)
3-13: 접근성 및 반응형 디자인 개선이 필요합니다현재 구현에 대한 몇 가지 개선 제안사항이 있습니다:
접근성 개선:
- 키보드 포커스 스타일 추가
- 고대비 모드 지원
반응형 디자인:
- 모바일 환경을 위한 터치 영역 최적화
- 다양한 화면 크기 대응
다음과 같이 변경하는 것을 제안드립니다:
export const DropdownVariants = cva( - `flex items-center rounded-lg bg-white text-black font-pretendard p-3`, + `flex items-center rounded-lg bg-white text-black font-pretendard p-3 + focus-visible:ring-2 focus-visible:ring-primary focus-visible:outline-none + min-h-[44px] md:min-h-[40px] + @media (prefers-contrast: high) { + border: 2px solid currentColor + }`, { variants: { state: { - default: 'hover:bg-secondary hover:text-primary font-bold transition-colors', - expanded: 'w-full hover:bg-secondary hover:text-primary font-medium transition-colors', + default: 'hover:bg-secondary hover:text-primary font-bold transition-colors active:bg-secondary-dark', + expanded: 'w-full hover:bg-secondary hover:text-primary font-medium transition-colors active:bg-secondary-dark', }, }, } );
8-9: 상태 전환 애니메이션 개선이 필요합니다현재 transition-colors만 적용되어 있는데, 사용자 경험 향상을 위해 다음과 같은 개선을 제안드립니다:
- 상태 변경 시 부드러운 전환
- hover/active 상태의 명확한 구분
다음과 같이 변경하는 것을 제안드립니다:
default: 'hover:bg-secondary hover:text-primary font-bold transition-colors', - expanded: 'w-full hover:bg-secondary hover:text-primary font-medium transition-colors', + expanded: 'w-full hover:bg-secondary hover:text-primary font-medium transition-all duration-200 ease-in-out',src/components/common/Dropdown/Dropdown.stories.tsx (3)
1-1: ESLint 규칙 비활성화에 대한 설명이 필요합니다.ESLint 규칙을 비활성화하는 이유를 주석으로 설명해주시기 바랍니다.
-/* eslint-disable no-restricted-exports */ +/* eslint-disable no-restricted-exports */ +// default export가 Storybook 설정에 필요하여 규칙을 비활성화합니다.
5-5: 상대 경로 임포트를 더 명확하게 지정해주세요.현재 인덱스 파일에서 임포트하는 방식은 파일의 실제 위치를 파악하기 어렵게 만듭니다.
-import { Dropdown } from '.'; +import { Dropdown } from './Dropdown';
7-11: 메타데이터에 추가 설정이 필요합니다.컴포넌트의 문서화와 테스트를 위한 추가 설정이 필요합니다.
const meta: Meta<typeof Dropdown> = { title: 'components/common/Dropdown', component: Dropdown, tags: ['autodocs'], + parameters: { + layout: 'centered', + docs: { + description: { + component: '카테고리 선택을 위한 드롭다운 컴포넌트입니다.', + }, + }, + }, + argTypes: { + selectedCategory: { + description: '현재 선택된 카테고리', + control: 'select', + options: DROPDOWN_CATEGORIES, + }, + onSelectCategory: { + description: '카테고리 선택 시 호출되는 콜백 함수', + }, + }, };src/constants/categories.ts (2)
13-23: 문서화 추가 제안상수의 구조와 용도를 설명하는 JSDoc 주석을 추가하면 좋을 것 같습니다.
다음과 같이 문서를 추가해보세요:
+/** + * 드롭다운 메뉴에서 사용되는 카테고리 목록입니다. + * @property name - 사용자에게 표시되는 카테고리 이름 + * @property markerCategory - 내부적으로 사용되는 마커 카테고리 값 (전체 선택의 경우 null) + */ export const DROPDOWN_CATEGORIES: { name: string; markerCategory: MarkerCategory | null }[] = [
13-13: 타입 정의 개선 제안반복되는 객체 타입을 별도로 정의하면 코드의 재사용성과 가독성이 향상될 것 같습니다.
다음과 같이 타입을 분리해보세요:
+type DropdownCategory = { + name: string; + markerCategory: MarkerCategory | null; +}; + -export const DROPDOWN_CATEGORIES: { name: string; markerCategory: MarkerCategory | null }[] = [ +export const DROPDOWN_CATEGORIES: DropdownCategory[] = [src/hooks/common/useDropdown.tsx (3)
1-4: 불필요한 빈 줄이 있습니다import 문 사이의 불필요한 빈 줄을 제거하여 코드의 일관성을 유지하는 것이 좋습니다.
import { useState, useCallback } from 'react'; - import { Category } from '@/components/common/Dropdown/Dropdown.types'; import { DROPDOWN_CATEGORIES } from '@/constants/categories';
6-8: 타입 안전성 개선이 필요합니다
onSelectCategory콜백 함수의 매개변수 타입을 더 구체적으로 정의하면 좋겠습니다. 현재는 일반적인 string 타입을 사용하고 있지만, 카테고리 이름에 대한 구체적인 타입을 정의하면 타입 안전성이 향상될 것입니다.-export const useDropdown = (onSelectCategory: (category: string) => void) => { +type CategoryName = Category['name']; +export const useDropdown = (onSelectCategory: (category: CategoryName) => void) => {
21-27: 반환 객체의 타입 정의가 필요합니다반환되는 객체의 타입을 명시적으로 정의하면 이 훅을 사용하는 컴포넌트에서 타입 추론이 더 정확해질 것입니다.
+interface DropdownHookReturn { + isOpen: boolean; + handleToggle: () => void; + handleSelect: (category: Category) => void; + categories: typeof DROPDOWN_CATEGORIES; +} + return { isOpen, handleToggle, handleSelect, categories: DROPDOWN_CATEGORIES, -}; +} as DropdownHookReturn;src/components/common/Dropdown/Dropdown.types.ts (1)
5-18: selectedCategory 타입을 더 구체적으로 정의하면 좋을 것 같습니다.현재
selectedCategory가 단순 string 타입으로 정의되어 있는데, 가능한 카테고리 값들을 제한하는 것이 타입 안전성을 높일 수 있습니다.다음과 같이 변경하는 것을 제안드립니다:
- selectedCategory: string; + selectedCategory: Category['name'];이렇게 하면
Category타입의name필드와 타입이 연동되어 더 안전한 타입 체크가 가능해집니다.src/components/common/Dropdown/index.tsx (1)
1-43: 컴포넌트 아키텍처 개선 제안
스타일링 추상화:
- Tailwind 클래스를 variants 파일로 더 많이 이동하여 재사용성 향상
- 테마 시스템 고려
에러 처리:
- 카테고리 데이터 로딩 실패시 폴백 UI 필요
- ErrorBoundary 구현 검토
성능 최적화:
- 큰 카테고리 목록의 경우 가상화 도입 고려
- 지연 로딩 전략 검토
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
src/components/common/Dropdown/Dropdown.stories.tsx(1 hunks)src/components/common/Dropdown/Dropdown.types.ts(1 hunks)src/components/common/Dropdown/Dropdown.variants.ts(1 hunks)src/components/common/Dropdown/index.tsx(1 hunks)src/constants/categories.ts(1 hunks)src/hooks/common/useDropdown.tsx(1 hunks)
🔇 Additional comments (5)
src/components/common/Dropdown/Dropdown.variants.ts (1)
4-4: font-pretendard 폰트의 가용성 확인이 필요합니다
font-pretendard 폰트가 프로젝트에 올바르게 설정되어 있는지 확인해 주세요. 폰트가 로드되지 않을 경우를 대비하여 fallback 폰트를 지정하는 것이 좋습니다.
✅ Verification successful
Pretendard 폰트가 올바르게 구성되어 있습니다
폰트 설정이 완벽하게 구현되어 있습니다:
src/index.css에@font-face선언이 올바르게 되어 있음tailwind.config.ts에fontFamily설정이 정확히 구성됨- 폰트 파일
public/fonts/PretendardVariable.woff2이 존재함
따라서 font-pretendard 클래스 사용에 문제가 없습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: font-pretendard 폰트 설정 확인
# 폰트 import 확인
rg -l "font-pretendard|Pretendard" --type css --type html
# tailwind 설정에서 폰트 패밀리 확인
cat tailwind.config.js
Length of output: 149
Script:
#!/bin/bash
# Check the content of index.css for Pretendard font configuration
cat src/index.css
# Look for any tailwind configuration files with different extensions
fd -e js -e cjs -e mjs -e ts "tailwind.config"
# Check for font configurations in package.json
cat package.json | jq '.dependencies, .devDependencies'
Length of output: 2458
Script:
#!/bin/bash
# Check tailwind configuration for font settings
cat tailwind.config.ts
# Verify if the font file exists in the public directory
fd -g "PretendardVariable.woff2" -t f
Length of output: 761
src/hooks/common/useDropdown.tsx (1)
9-11: handleToggle 함수의 구현이 적절합니다
useCallback을 사용하여 메모이제이션을 구현한 것이 좋습니다. 의존성 배열이 비어있는 것도 적절합니다.
src/components/common/Dropdown/Dropdown.types.ts (2)
1-4: 깔끔한 import 구조입니다!
외부 라이브러리와 내부 상수를 명확하게 구분하여 import하였습니다.
20-29: markerCategory가 null이 필요한지 확인해주세요.
markerCategory가 null을 허용하는 것이 의도된 설계인지 확인이 필요합니다. 만약 모든 카테고리가 항상 마커 카테고리와 연결되어야 한다면, null을 제거하는 것이 더 안전할 수 있습니다.
✅ Verification successful
markerCategory의 null 타입은 의도된 설계입니다
DROPDOWN_CATEGORIES 상수에서 '전체' 카테고리에 대해 markerCategory: null을 명시적으로 사용하고 있습니다. 이는 모든 마커를 표시하는 '전체' 필터 옵션을 위한 의도된 설계로 보입니다. 따라서 현재 타입 정의는 적절합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# MarkerCategory 사용 패턴을 확인
rg -A 2 "markerCategory.*null"
Length of output: 516
Script:
#!/bin/bash
# Check how markerCategory is used in components
ast-grep --pattern 'markerCategory: $_'
# Check if there's any specific handling for null markerCategory
rg "markerCategory.*=.*null" -B 2 -A 2
Length of output: 82
Script:
#!/bin/bash
# Check the implementation of dropdown component and its usage
rg -A 5 "DROPDOWN_CATEGORIES"
# Check if there's any filtering or conditional logic related to markerCategory
rg -A 3 "markerCategory" --type ts
Length of output: 2074
src/components/common/Dropdown/index.tsx (1)
1-8: 깔끔한 import 구조입니다!
필요한 의존성들이 잘 구성되어 있고, 타입과 변형(variants)을 별도 파일로 분리한 구조가 좋습니다.
| export const Basic: Story = { | ||
| render: () => { | ||
| const [selectedCategory, setSelectedCategory] = useState('전체'); | ||
|
|
||
| return ( | ||
| <Dropdown | ||
| selectedCategory={selectedCategory} | ||
| onSelectCategory={(category) => setSelectedCategory(category)} | ||
| /> | ||
| ); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스토리 구현을 개선해주세요.
다음과 같은 개선사항들이 필요합니다:
- 선택 가능한 카테고리 타입 안전성 확보
- 다양한 상태 (로딩, 에러, 비활성화 등) 테스트
- 접근성 테스트 추가
- Props 문서화
다음과 같이 개선할 것을 제안드립니다:
import { DROPDOWN_CATEGORIES } from '@/constants/categories';
import type { Category } from '@/types';
export const Basic: Story = {
args: {
selectedCategory: DROPDOWN_CATEGORIES[0],
onSelectCategory: (category: Category) => console.log(category),
},
parameters: {
docs: {
description: {
story: '기본적인 드롭다운 컴포넌트입니다.',
},
},
},
};
export const Loading: Story = {
args: {
...Basic.args,
isLoading: true,
},
};
export const Disabled: Story = {
args: {
...Basic.args,
disabled: true,
},
};
export const Error: Story = {
args: {
...Basic.args,
error: new Error('카테고리를 불러오는데 실패했습니다.'),
},
};| const handleSelect = useCallback( | ||
| (category: Category) => { | ||
| onSelectCategory(category.name); | ||
| setIsOpen(false); | ||
| }, | ||
| [onSelectCategory] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
handleSelect 함수의 에러 처리가 필요합니다
카테고리 선택 시 발생할 수 있는 예외 상황에 대한 처리가 없습니다. onSelectCategory 호출 시 에러가 발생할 수 있으므로, try-catch 블록으로 감싸는 것이 좋겠습니다.
const handleSelect = useCallback(
(category: Category) => {
- onSelectCategory(category.name);
- setIsOpen(false);
+ try {
+ onSelectCategory(category.name);
+ setIsOpen(false);
+ } catch (error) {
+ console.error('카테고리 선택 중 오류가 발생했습니다:', error);
+ // 에러 처리 로직 추가 필요
+ }
},
[onSelectCategory]
);Committable suggestion skipped: line range outside the PR's diff.
| return ( | ||
| <div className={`relative inline-block ${className}`}> | ||
| <button | ||
| type="button" | ||
| className={DropdownVariants({ state: 'default' })} | ||
| onClick={handleToggle} | ||
| > | ||
| <div className="flex items-center"> | ||
| {selectedCategory} | ||
| <span className="ml-2"> | ||
| {isOpen ? <ArrowUpIcon className="inline" /> : <ArrowDownIcon className="inline" />} | ||
| </span> | ||
| </div> | ||
| </button> | ||
| {isOpen && ( | ||
| <div className="absolute left-0 bg-white shadow-lg rounded-lg mt-0 w-full"> | ||
| {categories.map((category) => ( | ||
| <button | ||
| key={category.name} | ||
| type="button" | ||
| className={DropdownVariants({ state: 'expanded' })} | ||
| onClick={() => handleSelect(category)} | ||
| > | ||
| {category.name} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
현재 구현에 다음과 같은 접근성 관련 개선사항들이 있습니다:
- ARIA 속성 추가 필요
- 키보드 네비게이션 지원 필요
- 드롭다운 목록이 열렸을 때 포커스 관리 필요
다음과 같은 개선사항을 제안드립니다:
<div className={`relative inline-block ${className}`}>
<button
type="button"
className={DropdownVariants({ state: 'default' })}
onClick={handleToggle}
+ aria-haspopup="listbox"
+ aria-expanded={isOpen}
+ aria-controls="dropdown-list"
>
<div className="flex items-center">
{selectedCategory}
<span className="ml-2">
{isOpen ? <ArrowUpIcon className="inline" /> : <ArrowDownIcon className="inline" />}
</span>
</div>
</button>
{isOpen && (
- <div className="absolute left-0 bg-white shadow-lg rounded-lg mt-0 w-full">
+ <div
+ id="dropdown-list"
+ role="listbox"
+ className="absolute left-0 bg-white shadow-lg rounded-lg mt-0 w-full"
+ >
{categories.map((category) => (
<button
key={category.name}
type="button"
className={DropdownVariants({ state: 'expanded' })}
onClick={() => handleSelect(category)}
+ role="option"
+ aria-selected={category.name === selectedCategory}
>
{category.name}
</button>
))}
</div>
)}
</div>추가로 키보드 네비게이션을 위한 훅을 구현하시는 것을 추천드립니다.
📝 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.
| return ( | |
| <div className={`relative inline-block ${className}`}> | |
| <button | |
| type="button" | |
| className={DropdownVariants({ state: 'default' })} | |
| onClick={handleToggle} | |
| > | |
| <div className="flex items-center"> | |
| {selectedCategory} | |
| <span className="ml-2"> | |
| {isOpen ? <ArrowUpIcon className="inline" /> : <ArrowDownIcon className="inline" />} | |
| </span> | |
| </div> | |
| </button> | |
| {isOpen && ( | |
| <div className="absolute left-0 bg-white shadow-lg rounded-lg mt-0 w-full"> | |
| {categories.map((category) => ( | |
| <button | |
| key={category.name} | |
| type="button" | |
| className={DropdownVariants({ state: 'expanded' })} | |
| onClick={() => handleSelect(category)} | |
| > | |
| {category.name} | |
| </button> | |
| ))} | |
| </div> | |
| )} | |
| </div> | |
| ); | |
| return ( | |
| <div className={`relative inline-block ${className}`}> | |
| <button | |
| type="button" | |
| className={DropdownVariants({ state: 'default' })} | |
| onClick={handleToggle} | |
| aria-haspopup="listbox" | |
| aria-expanded={isOpen} | |
| aria-controls="dropdown-list" | |
| > | |
| <div className="flex items-center"> | |
| {selectedCategory} | |
| <span className="ml-2"> | |
| {isOpen ? <ArrowUpIcon className="inline" /> : <ArrowDownIcon className="inline" />} | |
| </span> | |
| </div> | |
| </button> | |
| {isOpen && ( | |
| <div | |
| id="dropdown-list" | |
| role="listbox" | |
| className="absolute left-0 bg-white shadow-lg rounded-lg mt-0 w-full" | |
| > | |
| {categories.map((category) => ( | |
| <button | |
| key={category.name} | |
| type="button" | |
| className={DropdownVariants({ state: 'expanded' })} | |
| onClick={() => handleSelect(category)} | |
| role="option" | |
| aria-selected={category.name === selectedCategory} | |
| > | |
| {category.name} | |
| </button> | |
| ))} | |
| </div> | |
| )} | |
| </div> | |
| ); |
| export const Dropdown = memo(({ selectedCategory, onSelectCategory, className }: Props) => { | ||
| const { isOpen, handleToggle, handleSelect, categories } = useDropdown(onSelectCategory); |
There was a problem hiding this comment.
💡 Codebase verification
memo 최적화가 불필요해 보입니다
현재 Dropdown 컴포넌트는 다음과 같은 특징이 있습니다:
- 단순한 Props 구조 (selectedCategory, onSelectCategory, className)
- Storybook에서만 사용되고 있음
- 복잡한 계산이나 중첩된 데이터 구조가 없음
이러한 상황에서 memo를 사용하는 것은 불필요한 최적화로 보입니다. memo 제거를 권장드립니다.
🔗 Analysis chain
memo 사용의 필요성 검토가 필요합니다
React.memo는 불필요한 리렌더링을 방지하는데 도움이 되지만, 모든 컴포넌트에 필요한 것은 아닙니다. 실제로 성능 이슈가 있는지 확인해보시는 것을 추천드립니다.
또한 Props 인터페이스의 정의를 확인하고 싶습니다.
Let me gather more information about the Props interface and component usage.
Let me check the Props interface definition and any potential parent components that might cause frequent updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Props 인터페이스 정의 확인
ast-grep --pattern 'interface Props {
$$$
}'
# 컴포넌트의 사용처 확인
rg -l "Dropdown.*selectedCategory.*onSelectCategory"
Length of output: 144
Script:
#!/bin/bash
# Check the content of the Dropdown component file
cat src/components/common/Dropdown/index.tsx
# Find parent components that might cause frequent re-renders
rg -A 5 "import.*Dropdown"
Length of output: 3146
Script:
#!/bin/bash
# Check the Props interface definition
cat src/components/common/Dropdown/Dropdown.types.ts
# Find where this component is being used
rg -A 5 "<Dropdown"
Length of output: 1210
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/assets/icons/index.tsx(2 hunks)src/components/common/Dropdown/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/Dropdown/index.tsx
🔇 Additional comments (1)
src/assets/icons/index.tsx (1)
1-2: 새로운 화살표 아이콘 임포트가 적절히 추가되었습니다.
드롭다운 컴포넌트의 상태를 시각적으로 표현하기 위한 아이콘들이 잘 추가되었습니다.
| arrowDown: ArrowDown, | ||
| arrowUp: ArrowUp, |
There was a problem hiding this comment.
💡 Codebase verification
아이콘 파일이 누락되어 있습니다
src/assets/icons/index.tsx에서 arrow-down.svg와 arrow-up.svg 파일을 임포트하고 있지만, 디렉토리 검색 결과 해당 SVG 파일들이 존재하지 않습니다. 다음 사항들을 확인해주세요:
arrow-down.svg파일 추가 필요arrow-up.svg파일 추가 필요
🔗 Analysis chain
Icons 객체에 새로운 아이콘들이 일관성 있게 추가되었습니다.
기존 네이밍 컨벤션을 따르면서 camelCase로 적절하게 명명되었습니다.
아이콘 파일들의 존재 여부를 확인하기 위해 다음 스크립트를 실행합니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 새로 추가된 아이콘 SVG 파일들의 존재 여부 확인
fd -e svg "arrow-(up|down)" src/assets/icons
Length of output: 46
Script:
#!/bin/bash
# 1. 아이콘 컴포넌트 파일들의 존재 여부 확인
fd ArrowDown.tsx ArrowUp.tsx src/assets/icons
# 2. 아이콘 컴포넌트 구현 확인
ast-grep --pattern 'const $_ = () => {
$$$
}'
Length of output: 8014
Script:
#!/bin/bash
# 1. src/assets/icons 디렉토리 내의 모든 파일 확인
fd . src/assets/icons
# 2. ArrowDown과 ArrowUp 관련 모든 파일 검색
rg -l "ArrowDown|ArrowUp" src/
Length of output: 1140
Script:
#!/bin/bash
# Dropdown 컴포넌트에서 ArrowDown/ArrowUp 사용 방식 확인
rg -A 5 "ArrowDown|ArrowUp" src/components/common/Dropdown/index.tsx
# icons/index.tsx 파일에서 ArrowDown/ArrowUp 관련 코드 확인
rg -A 5 "ArrowDown|ArrowUp" src/assets/icons/index.tsx
Length of output: 1023
|
Update: 2024년 11월 25일 22시 47분 43초 |
| export const Dropdown = memo(({ selectedCategory, onSelectCategory, className }: Props) => { | ||
| const { isOpen, handleToggle, handleSelect, categories } = useDropdown(onSelectCategory); | ||
|
|
||
| const ArrowDownIcon = Icons.arrowDown; |
| const ArrowUpIcon = Icons.arrowUp; | ||
|
|
||
| return ( | ||
| <div className={`relative inline-block ${className}`}> |
There was a problem hiding this comment.
요렇게 사용하면 좋을 것 같아요.
| <div className={`relative inline-block ${className}`}> | |
| <div className={cn('relative inline-block'), className}> |
| <div className="flex items-center"> | ||
| {selectedCategory} | ||
| <span className="ml-2"> | ||
| {isOpen ? <ArrowUpIcon className="inline" /> : <ArrowDownIcon className="inline" />} |
There was a problem hiding this comment.
IconButton 컴포넌트가 존재해서 변경해도 좋을 것 같아요~
| {isOpen ? <ArrowUpIcon className="inline" /> : <ArrowDownIcon className="inline" />} | |
| <IconButton name={isOpen ? 'arrowUp' : 'arrowDown'} /> |
| import { Props } from './Dropdown.types'; | ||
| import { DropdownVariants } from './Dropdown.variants'; | ||
|
|
||
| export const Dropdown = memo(({ selectedCategory, onSelectCategory, className }: Props) => { |
There was a problem hiding this comment.
필요없어져서 지우고 올렸어야 했는데 깜빡했습니다,, 수정해서 올리겠습니다!
| import { cva } from 'class-variance-authority'; | ||
|
|
||
| export const DropdownVariants = cva( | ||
| `flex items-center rounded-lg bg-white text-black font-pretendard p-3`, |
There was a problem hiding this comment.
이미 font는 pretendard로 설정 되어 있어요! 지워도 될 것 같아요.
| /** | ||
| * Corresponding marker category for the dropdown item. | ||
| */ | ||
| markerCategory: MarkerCategory | null; |
There was a problem hiding this comment.
요 Category 타입의 makerCategory는 어디서 사용되고 있는건가요???
There was a problem hiding this comment.
앗 요것도 지워야했는데 놓쳐서 다시 올리겠습니다!
| variants: { | ||
| state: { | ||
| default: 'hover:bg-secondary hover:text-primary font-bold transition-colors', | ||
| expanded: 'w-full hover:bg-secondary hover:text-primary font-medium transition-colors', |
There was a problem hiding this comment.
선택됐을 때 배경이 붉다면, 사용자관점에서 좋지 않을 것 같아요. bg-gray-50 로만 변경하는거 어떠신가요??
| { | ||
| variants: { | ||
| state: { | ||
| default: 'hover:bg-secondary hover:text-primary font-bold transition-colors', |
There was a problem hiding this comment.
default 가 선택된 드롭다운을 나타내는 것 같은데, 이때는 bg,hover가 없어도 될 것 같은데 어떻게 생각하시나요??
There was a problem hiding this comment.
호버에서 클릭으로 수정하면서 고쳐서 올리겠습니당
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/hooks/common/useDropdown.tsx (2)
11-15: 타입 안전성 개선이 필요합니다
dropdownPosition타입을 별도의 인터페이스로 분리하여 재사용성을 높이는 것이 좋겠습니다.+interface DropdownPosition { + top: number; + left: number; + width: number; +} -const [dropdownPosition, setDropdownPosition] = useState<{ - top: number; - left: number; - width: number; -}>({ top: 0, left: 0, width: 0 }); +const [dropdownPosition, setDropdownPosition] = useState<DropdownPosition>({ + top: 0, + left: 0, + width: 0 +});
51-58: 반환값의 메모이제이션이 필요합니다반환되는 객체가 매 렌더링마다 새로 생성되어 불필요한 리렌더링이 발생할 수 있습니다.
+const returnValue = useMemo( + () => ({ + isOpen, + handleToggle, + handleSelect, + categories: DROPDOWN_CATEGORIES, + dropdownPosition, + }), + [isOpen, handleToggle, handleSelect, dropdownPosition] +); -return { - isOpen, - handleToggle, - handleSelect, - categories: DROPDOWN_CATEGORIES, - dropdownPosition, -}; +return returnValue;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
.pnp.cjs(3 hunks)package.json(1 hunks)src/components/common/Dropdown/Dropdown.variants.ts(1 hunks)src/components/common/Dropdown/index.tsx(1 hunks)src/hooks/common/useDropdown.tsx(1 hunks)tailwind.config.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/common/Dropdown/Dropdown.variants.ts
- src/components/common/Dropdown/index.tsx
🔇 Additional comments (6)
.pnp.cjs (2)
85-85: 의존성 목록에 tailwind-scrollbar-hide 패키지가 올바르게 추가되었습니다.
알파벳 순서에 맞게 정렬되어 있으며, 버전이 일관되게 유지되고 있습니다.
Also applies to: 8157-8157
13290-13298: 패키지 메타데이터 구성이 적절한지 확인이 필요합니다.
패키지 메타데이터가 올바르게 구성되어 있습니다만, 보안을 위해 최신 버전 확인이 필요합니다.
✅ Verification successful
Let me verify the current version in the package metadata by checking the .pnp.cjs file.
패키지 메타데이터가 안전하게 구성되어 있습니다
현재 사용 중인 tailwind-scrollbar-hide 버전(1.1.7)은 최신 버전과 일치하며, npm 보안 취약점 데이터베이스에서 알려진 취약점이 없음을 확인했습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: tailwind-scrollbar-hide 패키지의 최신 버전과 보안 취약점 확인
# npm 레지스트리에서 최신 버전 확인
curl -s https://registry.npmjs.org/tailwind-scrollbar-hide | jq -r '."dist-tags".latest'
# GitHub API를 통해 보안 취약점 확인
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "tailwind-scrollbar-hide") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 453
Script:
#!/bin/bash
# Check the version of tailwind-scrollbar-hide in .pnp.cjs
rg "tailwind-scrollbar-hide" .pnp.cjs -A 2 -B 2
Length of output: 1417
tailwind.config.ts (2)
1-1: 스크롤바 숨김 플러그인 import 구문이 적절히 추가되었습니다.
플러그인 import 구문이 파일 최상단에 올바르게 위치해 있습니다.
34-34: 플러그인 설정이 올바르게 구성되었습니다.
tailwindScrollbarHide 플러그인이 plugins 배열에 정상적으로 추가되었습니다. 하지만 이 변경으로 인한 영향도를 확인해보시기 바랍니다.
스크롤바 관련 스타일이 적용된 컴포넌트들을 확인하기 위해 다음 스크립트를 실행하세요:
✅ Verification successful
tailwindScrollbarHide 플러그인이 적절하게 사용되고 있음을 확인했습니다
scrollbar-hide 클래스는 Dropdown 컴포넌트의 스크롤 영역에서 적절하게 사용되고 있으며, 해당 컴포넌트는 공통 컴포넌트로서 stories와 variants가 잘 구성되어 있습니다. 플러그인 설정이 올바르게 적용되어 있어 추가적인 검증이 필요하지 않습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 스크롤바 관련 클래스 사용 현황 확인
# scrollbar-hide 클래스 사용 검색
echo "스크롤바 숨김 클래스 사용 현황:"
rg -l "scrollbar-hide" "src/"
Length of output: 115
Script:
#!/bin/bash
# Dropdown 컴포넌트에서 scrollbar-hide 클래스 사용 확인
echo "Dropdown 컴포넌트의 scrollbar-hide 사용 컨텍스트:"
rg "scrollbar-hide" -B 2 -A 2 "src/components/common/Dropdown/index.tsx"
# Dropdown 컴포넌트 사용처 확인
echo -e "\nDropdown 컴포넌트 사용 현황:"
rg -l "Dropdown" "src/"
Length of output: 644
src/hooks/common/useDropdown.tsx (1)
21-27: 이전 리뷰 코멘트가 여전히 유효합니다
에러 처리에 대한 이전 리뷰 코멘트가 아직 반영되지 않았습니다.
package.json (1)
39-39: 스크롤바 숨김 패키지의 사용 및 보안 검증이 필요합니다.
새로 추가된 tailwind-scrollbar-hide 패키지와 관련하여 다음 사항들을 확인해 주시기 바랍니다:
- 빌드 시에만 사용되는 경우
devDependencies로 이동을 고려해 보세요. - 현재 버전의 보안 취약점을 확인할 필요가 있습니다.
✅ Verification successful
Let me check how the package is configured in tailwind.config.ts and actually used in the Dropdown component before making the final determination.
tailwind-scrollbar-hide 패키지가 적절하게 구성되어 있습니다.
패키지 검증 결과:
- Tailwind 플러그인으로 올바르게 구성되어 있습니다.
- Dropdown 컴포넌트의 스크롤바 숨김 기능에 실제로 사용되고 있습니다.
- 알려진 보안 취약점이 없습니다.
- 런타임이 아닌 빌드 시에만 사용되므로
devDependencies로 이동하는 것이 좋습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package usage and check for security vulnerabilities
# Check if the package is actually used in the codebase
echo "Checking usage of tailwind-scrollbar-hide..."
rg -l "scrollbar-hide"
# Check for security advisories
echo "Checking for security advisories..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "tailwind-scrollbar-hide") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 632
Script:
#!/bin/bash
# Check tailwind config
echo "=== Tailwind Configuration ==="
cat tailwind.config.ts
# Check dropdown component usage
echo -e "\n=== Dropdown Component Usage ==="
cat src/components/common/Dropdown/index.tsx
Length of output: 2739
src/hooks/common/useDropdown.tsx
Outdated
| useEffect(() => { | ||
| if (isOpen && dropdownRef.current) { | ||
| const updateDropdownPosition = () => { | ||
| if (dropdownRef.current) { | ||
| const rootDropdown = dropdownRef.current.getBoundingClientRect(); | ||
| setDropdownPosition({ | ||
| top: rootDropdown.bottom + window.scrollY, | ||
| left: rootDropdown.left + window.scrollX, | ||
| width: rootDropdown.width, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| updateDropdownPosition(); | ||
| window.addEventListener('resize', updateDropdownPosition); | ||
|
|
||
| return () => { | ||
| window.removeEventListener('resize', updateDropdownPosition); | ||
| }; | ||
| } | ||
| }, [isOpen, dropdownRef]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
드롭다운 위치 계산 로직 개선이 필요합니다
현재 구현에는 몇 가지 개선이 필요한 부분이 있습니다:
- 화면 경계를 벗어나는 경우에 대한 처리가 없습니다.
- 스크롤 이벤트에 대한 처리가 누락되어 있습니다.
- 리사이즈 이벤트 핸들러가 불필요하게 자주 호출될 수 있습니다.
다음과 같은 개선을 제안드립니다:
useEffect(() => {
if (isOpen && dropdownRef.current) {
+ const checkBoundary = (position: typeof dropdownPosition) => {
+ const windowHeight = window.innerHeight;
+ const windowWidth = window.innerWidth;
+
+ return {
+ ...position,
+ top: Math.min(position.top, windowHeight - 200), // 드롭다운 최대 높이 고려
+ left: Math.min(position.left, windowWidth - position.width),
+ };
+ };
+
const updateDropdownPosition = () => {
if (dropdownRef.current) {
const rootDropdown = dropdownRef.current.getBoundingClientRect();
- setDropdownPosition({
+ const newPosition = checkBoundary({
top: rootDropdown.bottom + window.scrollY,
left: rootDropdown.left + window.scrollX,
width: rootDropdown.width,
});
+ setDropdownPosition(newPosition);
}
};
+ const debouncedUpdate = debounce(updateDropdownPosition, 100);
updateDropdownPosition();
- window.addEventListener('resize', updateDropdownPosition);
+ window.addEventListener('resize', debouncedUpdate);
+ window.addEventListener('scroll', debouncedUpdate);
return () => {
- window.removeEventListener('resize', updateDropdownPosition);
+ window.removeEventListener('resize', debouncedUpdate);
+ window.removeEventListener('scroll', debouncedUpdate);
+ debouncedUpdate.cancel();
};
}
}, [isOpen, dropdownRef]);추가로 debounce 유틸리티 함수를 구현하거나 lodash의 debounce를 사용하시면 됩니다.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/common/Dropdown/index.tsx (1)
26-37: 드롭다운 목록의 UI 개선이 필요합니다현재 구현에서 몇 가지 개선이 필요한 부분이 있습니다:
- 드롭다운 버튼과 목록의 너비가 일치하지 않을 수 있습니다.
- 스크롤 시 사용자 경험이 부자연스러울 수 있습니다.
다음과 같은 개선을 제안드립니다:
-<div className="absolute bg-white shadow-lg rounded-lg mt-0 z-modal max-h-[calc(4*2.6rem)] overflow-y-auto scrollbar-hide"> +<div className="absolute bg-white shadow-lg rounded-lg mt-0 z-modal max-h-[calc(4*2.6rem)] overflow-y-auto scrollbar-hide w-full">추가로 스크롤 인디케이터나 페이드 효과를 고려해보시면 좋을 것 같습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
src/components/common/Dropdown/Dropdown.stories.tsx(1 hunks)src/components/common/Dropdown/index.tsx(1 hunks)src/hooks/common/useDropdown.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/common/Dropdown/Dropdown.stories.tsx
- src/hooks/common/useDropdown.tsx
🔇 Additional comments (2)
src/components/common/Dropdown/index.tsx (2)
8-9: 이전 리뷰 제안사항이 잘 반영되었습니다!
불필요한 memo 최적화가 제거되었고, Props 타입이 잘 정의되어 있습니다.
11-24:
접근성 개선이 여전히 필요합니다
이전 리뷰에서 제안된 접근성 개선사항이 아직 반영되지 않았습니다:
<button
type="button"
className={DropdownVariants({ state: 'default' })}
onClick={handleToggle}
+ aria-haspopup="listbox"
+ aria-expanded={isOpen}
+ aria-controls="dropdown-list"
>추가로 키보드 네비게이션을 위한 이벤트 핸들러도 필요합니다:
onKeyDown이벤트로 화살표 키 처리Escape키로 드롭다운 닫기Enter/Space키로 항목 선택
관련 이슈
close #48
📑 작업 내용
💬 리뷰 중점 사항/기타 참고 사항
Summary by CodeRabbit
새로운 기능
useDropdown추가.ArrowDown및ArrowUp추가.타입 정의
Props,Category).스타일 개선
DropdownVariants) 추가.상수 추가
DROPDOWN_CATEGORIES) 추가.