[FEAT] PrioritySelector & TagSelector & 드롭다운 셸 컴포넌트 구현#75
Conversation
- Color, Typography 컴포넌트를 components/tokens 폴더 하위로 이동했습니다 - index.ts의 Color, Typography import 경로를 갱신했습니다
- Figma 디자인(node-id 894-32397)을 기반으로 우선순위 선택 컴포넌트를 추가했습니다 - PriorityIcon을 priority 폴더 하위로 이동해 관련 컴포넌트를 함께 묶었습니다 - 선택 상태 전환 시 배경·텍스트·아이콘 색상에 트랜지션을 적용했습니다 - Storybook 스토리와 배경 파라미터를 추가했습니다
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDropdown 기반 선택 컴포넌트가 추가되고, PrioritySelector와 TagSelector가 새로 노출됩니다. PriorityIcon과 TagIcon 구성도 정리되며, ChangesSelector 컴포넌트와 공개 API 정리
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Timo Performance ReportBundle Size — timo-web
Lighthouse — timo-web
Image Optimization — timo-web
측정 커밋: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx`:
- Around line 5-14: `PriorityLevel`는 유니언 리터럴 타입 alias 네이밍 규칙에 맞지 않고,
`PriorityIcon.tsx`와 중복 정의될 가능성이 있어 타입 불일치가 생길 수 있습니다. `PrioritySelector`의
`PriorityLevel`을 컨벤션에 맞는 `PriorityLevelTypes`로 정리하고, `PriorityIcon`이 쓰는 우선순위 타입과
함께 공용 타입으로 추출해 두 컴포넌트가 같은 타입을 참조하도록 수정하세요. `PRIORITY_LEVELS`와
`PRIORITY_BG_COLOR`도 새 공용 타입을 기준으로 유지되게 맞춰주세요.
- Around line 30-51: The PrioritySelector button only conveys selection
visually, so add an accessible pressed state to the button rendered in
PrioritySelector by setting aria-pressed based on isSelected. Update the button
element in PrioritySelector so screen readers can announce which priority is
selected, while keeping the existing onClick, PriorityIcon, and label behavior
unchanged.
- Line 3: The PrioritySelector component is using a relative import for
PriorityIcon instead of the required absolute alias. Update the import in
PrioritySelector.tsx to match the existing `@components/`... style used elsewhere,
and keep the symbol PriorityIcon referenced through the alias so imports remain
stable if the folder structure changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eff95a31-2e0c-4024-a2cb-c6d891377b0a
📒 Files selected for processing (9)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/priority/priority-icon/PriorityIcon.stories.tsxpackages/timo-design-system/src/components/priority/priority-icon/PriorityIcon.tsxpackages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.stories.tsxpackages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsxpackages/timo-design-system/src/components/tokens/color/Color.stories.tsxpackages/timo-design-system/src/components/tokens/color/Color.tsxpackages/timo-design-system/src/components/tokens/typography/Typography.stories.tsxpackages/timo-design-system/src/components/tokens/typography/Typography.tsx
- 드롭다운 형태 셀렉터들이 공유하는 패널·행 스타일을 Dropdown/Dropdown.Item으로 분리했습니다 - PrioritySelector가 Dropdown을 사용하도록 리팩토링했습니다
- Tag 컴포넌트를 TagIcon으로 이름을 변경해 tag-icon 폴더로 이동했습니다 - Dropdown을 사용하는 TagSelector 컴포넌트를 새로 추가했습니다 - index.ts의 export를 TagIcon, TagSelector로 갱신했습니다
kimminna
changed the title
- Dropdown이 내부 open 상태와 외부 클릭 감지를 갖도록 확장했습니다 - Trigger 클릭 시 토글, 바깥 클릭 시 자동으로 닫히도록 구현했습니다 - PrioritySelector·TagSelector가 trigger prop을 받아 스스로 트리거를 조합하도록 변경했습니다
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx (1)
21-53: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift우선순위(매우중요/중요/보통/낮음) 도메인 값이 디자인 시스템 패키지에 하드코딩돼 있어요.
timo-design-system패키지 규칙상 범용 UI만 포함되어야 하고 도메인 특화·비즈니스 로직은 배제되어야 하는데, "매우중요/중요/보통/낮음" 같은 업무(Task) 우선순위 값은 순수 UI 컴포넌트라기보다 특정 도메인(할일 관리 등)의 비즈니스 개념에 가깝습니다.PRIORITY_LEVELS/PRIORITY_BG_COLOR를 상위 앱 레이어에서 props로 주입받는 형태로 바꾸고, 이 컴포넌트는 순수 선택 UI만 제공하도록 하면 재사용성이 높아질 거예요.As per path instructions, "범용 UI 컴포넌트만 포함 (Button, Card 등)" / "도메인 특화 로직·비즈니스 로직 포함 금지".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx` around lines 21 - 53, The PrioritySelector component currently hardcodes task-specific priority domain values and styling, which should not live in the design system. Refactor PrioritySelector to accept the available items and any priority-to-color/icon mapping via props from the app layer, and remove PRIORITY_LEVELS and PRIORITY_BG_COLOR from the reusable component. Keep PrioritySelector as a pure Dropdown-based selection UI that only renders the provided options and calls onSelect with the chosen value.Source: Path instructions
♻️ Duplicate comments (1)
packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx (1)
26-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win선택 상태의
aria-pressed노출이 여전히 누락되어 있어요.과거 리뷰에서 시각적 색상 변화만으로는 보조기술 사용자가 선택 항목을 알기 어렵다는 점이 지적됐는데, 현재 코드에서도 반영되지 않았습니다. 단, 현재
Dropdown.Item이onClick,className,children만 받도록 되어 있어aria-pressed전달이 불가능한 상태예요 (Dropdown.tsx코멘트 참고).Dropdown.Item의 props 확장이 선행돼야 적용 가능합니다.♿ 제안 수정 (Dropdown.tsx 수정 후)
<Dropdown.Item key={priority} onClick={() => onSelect?.(priority)} + aria-pressed={isSelected} className={cn(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx` around lines 26 - 52, 선택된 우선순위 항목에 대한 접근성 상태 노출이 빠져 있습니다. PrioritySelector의 Dropdown.Item에 선택 여부를 aria-pressed로 전달할 수 있도록, 먼저 Dropdown.Item props를 확장해 aria-pressed를 받을 수 있게 수정한 뒤 PrioritySelector에서 isSelected에 따라 해당 속성을 설정하세요. Dropdown.Item, PrioritySelector, selected/isSelected 식별자를 기준으로 함께 반영하면 됩니다.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`:
- Around line 23-42: DropdownItemProps is too narrow because DropdownItem only
accepts children, className, and onClick, which blocks passing native button
props like disabled, aria-pressed, aria-label, and type. Update
DropdownItemProps to extend the native button props type and make DropdownItem
forward the remaining props onto the button while keeping the existing cn-based
className merge, so components like PrioritySelector can expose accessibility
and state attributes through DropdownItem.
In `@packages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx`:
- Around line 5-13: `TagSelector`가 "일상/운동/업무/기타"라는 도메인 전용 태그 목록을 내부에 고정으로 들고 있어
범용 UI 원칙에 어긋납니다. `Tag` 타입과 `TAGS` 상수를 제거하거나 외부로 옮기고, `TagSelectorProps`에 태그 목록을
props로 주입받도록 바꿔서 `TagSelector`가 전달된 값만 렌더링하도록 수정하세요.
- Line 5: The `Tag` type alias in `TagSelector` does not follow the project
naming convention for union/literal aliases. Rename `Tag` to `TagTypes` and
update any references in the same module or nearby consumers so the exported
type name matches the required `Types` suffix convention and avoids confusion
with global `Tag`-like names.
---
Outside diff comments:
In
`@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx`:
- Around line 21-53: The PrioritySelector component currently hardcodes
task-specific priority domain values and styling, which should not live in the
design system. Refactor PrioritySelector to accept the available items and any
priority-to-color/icon mapping via props from the app layer, and remove
PRIORITY_LEVELS and PRIORITY_BG_COLOR from the reusable component. Keep
PrioritySelector as a pure Dropdown-based selection UI that only renders the
provided options and calls onSelect with the chosen value.
---
Duplicate comments:
In
`@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx`:
- Around line 26-52: 선택된 우선순위 항목에 대한 접근성 상태 노출이 빠져 있습니다. PrioritySelector의
Dropdown.Item에 선택 여부를 aria-pressed로 전달할 수 있도록, 먼저 Dropdown.Item props를 확장해
aria-pressed를 받을 수 있게 수정한 뒤 PrioritySelector에서 isSelected에 따라 해당 속성을 설정하세요.
Dropdown.Item, PrioritySelector, selected/isSelected 식별자를 기준으로 함께 반영하면 됩니다.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f9384305-a17f-43a4-9a24-d9f4e121cad6
📒 Files selected for processing (8)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/layout/dropdown/Dropdown.stories.tsxpackages/timo-design-system/src/components/layout/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsxpackages/timo-design-system/src/components/tag/tag-icon/TagIcon.stories.tsxpackages/timo-design-system/src/components/tag/tag-icon/TagIcon.tsxpackages/timo-design-system/src/components/tag/tag-selector/TagSelector.stories.tsxpackages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx
kimminna
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx (1)
1-16: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift우선순위 목록(
PRIORITY_LEVELS)이 앱 도메인 데이터로 보여요."매우중요/중요/보통/낮음"은 특정 앱의 업무 분류 개념에 가깝습니다. 디자인 시스템 패키지는 범용 UI만 포함해야 하니, 이 목록을 컴포넌트 내부에 고정하기보다
PrioritySelectorProps로 주입받는 방식을 고려해보시면 어떨까요?TagSelector의TAGS상수에도 동일한 패턴이 보여서, 같이 개선하면 일관성도 챙길 수 있을 것 같아요.As per path instructions, "범용 UI 컴포넌트만 포함... 도메인 특화 로직·비즈니스 로직 포함 금지".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx` around lines 1 - 16, The hardcoded PRIORITY_LEVELS array in PrioritySelector makes this design-system component depend on app-specific domain values. Refactor PrioritySelector and its PrioritySelectorProps to accept the priority list from the caller instead of defining it internally, and update any related mapping such as PRIORITY_BG_COLOR to work with the injected values. Use the same pattern already used by TagSelector/TAGS so the component stays generic and reusable.Source: Path instructions
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx (1)
105-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win항목 선택 후 패널이 닫히도록 처리하세요
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx:105-124
Dropdown.Item이onClick만 호출해서close()가 빠져 있습니다. 지금PrioritySelector와TagSelector는 선택 뒤에도 패널이 열린 채 남으니,onClick?.()뒤에close()를 함께 호출하도록 묶는 편이 셀렉트형 동작에 맞습니다. WAI-ARIA Menu Button Pattern도 참고하면 좋아요: https://www.w3.org/WAI/ARIA/apg/patterns/menubutton/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx` around lines 105 - 124, Dropdown.Item currently only forwards onClick and leaves the menu open after selection. Update the DropdownItem component in Dropdown.tsx so the item action also triggers close() after onClick?.(), using the existing dropdown close handler from the surrounding dropdown context. Make sure the fix applies to the DropdownItem/Dropdown.Item flow used by PrioritySelector and TagSelector so selecting an item closes the panel consistently.
♻️ Duplicate comments (4)
packages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx (2)
7-7: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Tag타입도Types접미사 컨벤션을 따라주세요.여전히
Tag로 선언되어 있어 유니언 리터럴 타입 alias 네이밍 컨벤션과 어긋나요.TagTypes로 바꾸면 전역Tag개념과의 혼동도 줄일 수 있습니다.As per path instructions, "
typealias는 유니언·튜플·리터럴에만 사용, 접미사Types".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx` at line 7, The union type alias currently named Tag should follow the Types suffix convention to avoid clashing with the broader Tag concept. Rename the exported type in TagSelector.tsx from Tag to TagTypes, and update any local references in the TagSelector component and related imports/usages so the new identifier is used consistently.Source: Path instructions
7-16: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift태그 카테고리 목록도 도메인 특화 데이터네요.
"일상/운동/업무/기타"는 특정 앱의 업무 분류 도메인 개념이라, 범용 UI만 포함해야 하는 디자인 시스템 패키지 원칙과 맞지 않아요.
TAGS를 내부 상수로 고정하기보다 props로 주입받는 구조로 바꾸는 걸 권장드립니다.As per path instructions, "범용 UI 컴포넌트만 포함... 도메인 특화 로직·비즈니스 로직 포함 금지".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx` around lines 7 - 16, The TagSelector component currently hardcodes domain-specific tag values in the TAGS constant, which makes the design-system component app-specific instead of reusable. Update TagSelectorProps and TagSelector so the tag list is passed in as a prop rather than defined inside the component, and use that prop wherever TAGS is referenced. Keep the component generic by removing the fixed Tag union values tied to “일상/운동/업무/기타” and preserve the existing selected/onSelect/onAddClick behavior with the new injected tag options.Source: Path instructions
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx (1)
105-124: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
DropdownItemProps가 여전히 네이티브button속성을 확장하지 않는 것 같아요 (이전 리뷰 미해결 추정).과거 리뷰에서 지적된
DropdownItemProps의 확장성 문제(children/className/onClick만 지원)가 이번 diff 요약에도 그대로 남아있는 것으로 보입니다. 이 상태로는PrioritySelector/TagSelector에서 선택 상태를aria-pressed로 노출하고 싶어도 방법이 없어요.ComponentPropsWithoutRef<"button">을 확장해두면 향후 접근성 속성 확장이 훨씬 수월해질 거예요. 관련 문서: React TypeScript cheatsheet - ComponentProps♻️ 제안 수정
-export interface DropdownItemProps { - children: ReactNode; - className?: string; - onClick?: () => void; -} +export interface DropdownItemProps + extends React.ComponentPropsWithoutRef<"button"> { + children: ReactNode; +}정확한 현재 코드를 확인하기 위해 아래 스크립트를 돌려주실 수 있을까요?
#!/bin/bash ast-grep run --pattern 'interface DropdownItemProps { $$$ }' --lang typescript packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx` around lines 105 - 124, `DropdownItemProps` and `DropdownItem` still only support children/className/onClick, so extend the prop type to inherit the native button props via `ComponentPropsWithoutRef<"button">` and forward the remaining props onto the rendered button. Update the `DropdownItem` component to accept and spread those button attributes so consumers like `PrioritySelector` and `TagSelector` can pass accessibility/state props such as `aria-pressed` without type errors.packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx (1)
5-22: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
PriorityLevel타입 네이밍 컨벤션 재확인 부탁드려요.유니언 리터럴 타입 alias는
Types접미사를 붙이도록 되어 있는데(PriorityLevelTypes), 여전히PriorityLevel로 남아있는 것 같습니다. 또한PriorityIcon도 내부적으로 유사한 우선순위 유니언을 갖고 있을 가능성이 높아, 두 파일의 타입이 별도로 정의되면 값이 추가/변경될 때 한쪽만 고쳐서 어긋날 위험이 있어요. 공용 타입으로 추출해 두 컴포넌트가 함께 참조하면 좋을 것 같습니다.As per path instructions, "
typealias는 유니언·튜플·리터럴에만 사용, 접미사Types".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx` around lines 5 - 22, `PrioritySelector` still defines the priority union as `PriorityLevel`, which conflicts with the required `Types` suffix convention for union literal aliases. Rename the alias to `PriorityLevelTypes` and update `PrioritySelectorProps`, `PRIORITY_LEVELS`, and `PRIORITY_BG_COLOR` to use it. Also extract the shared priority union into a common type module and have both `PrioritySelector` and `PriorityIcon` import that same type so the priority values stay synchronized.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`:
- Around line 42-54: The Dropdown component currently only closes on outside
mousedown, so keyboard users have no way to dismiss it. Update the useEffect in
Dropdown.tsx to also listen for Escape keydown while isOpen is true, and call
setIsOpen(false) when the pressed key is Escape; keep the existing outside-click
behavior and clean up the new listener in the effect return alongside
handleOutsideClick.
---
Outside diff comments:
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`:
- Around line 105-124: Dropdown.Item currently only forwards onClick and leaves
the menu open after selection. Update the DropdownItem component in Dropdown.tsx
so the item action also triggers close() after onClick?.(), using the existing
dropdown close handler from the surrounding dropdown context. Make sure the fix
applies to the DropdownItem/Dropdown.Item flow used by PrioritySelector and
TagSelector so selecting an item closes the panel consistently.
In
`@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx`:
- Around line 1-16: The hardcoded PRIORITY_LEVELS array in PrioritySelector
makes this design-system component depend on app-specific domain values.
Refactor PrioritySelector and its PrioritySelectorProps to accept the priority
list from the caller instead of defining it internally, and update any related
mapping such as PRIORITY_BG_COLOR to work with the injected values. Use the same
pattern already used by TagSelector/TAGS so the component stays generic and
reusable.
---
Duplicate comments:
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`:
- Around line 105-124: `DropdownItemProps` and `DropdownItem` still only support
children/className/onClick, so extend the prop type to inherit the native button
props via `ComponentPropsWithoutRef<"button">` and forward the remaining props
onto the rendered button. Update the `DropdownItem` component to accept and
spread those button attributes so consumers like `PrioritySelector` and
`TagSelector` can pass accessibility/state props such as `aria-pressed` without
type errors.
In
`@packages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsx`:
- Around line 5-22: `PrioritySelector` still defines the priority union as
`PriorityLevel`, which conflicts with the required `Types` suffix convention for
union literal aliases. Rename the alias to `PriorityLevelTypes` and update
`PrioritySelectorProps`, `PRIORITY_LEVELS`, and `PRIORITY_BG_COLOR` to use it.
Also extract the shared priority union into a common type module and have both
`PrioritySelector` and `PriorityIcon` import that same type so the priority
values stay synchronized.
In `@packages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx`:
- Line 7: The union type alias currently named Tag should follow the Types
suffix convention to avoid clashing with the broader Tag concept. Rename the
exported type in TagSelector.tsx from Tag to TagTypes, and update any local
references in the TagSelector component and related imports/usages so the new
identifier is used consistently.
- Around line 7-16: The TagSelector component currently hardcodes
domain-specific tag values in the TAGS constant, which makes the design-system
component app-specific instead of reusable. Update TagSelectorProps and
TagSelector so the tag list is passed in as a prop rather than defined inside
the component, and use that prop wherever TAGS is referenced. Keep the component
generic by removing the fixed Tag union values tied to “일상/운동/업무/기타” and
preserve the existing selected/onSelect/onAddClick behavior with the new
injected tag options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 059c4227-5bff-4271-83e5-13669e467c5b
📒 Files selected for processing (6)
packages/timo-design-system/src/components/layout/dropdown/Dropdown.stories.tsxpackages/timo-design-system/src/components/layout/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.stories.tsxpackages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsxpackages/timo-design-system/src/components/tag/tag-selector/TagSelector.stories.tsxpackages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx
- TagSelector에 하드코딩된 태그 목록을 제거하고 tags prop으로 외부에서 주입받도록 변경했습니다 - DropdownItemProps를 ButtonHTMLAttributes로 확장해 aria-pressed, disabled 등 네이티브 버튼 속성을 전달할 수 있도록 했습니다 - PrioritySelector, TagSelector의 선택된 항목에 aria-pressed를 추가해 스크린리더가 선택 상태를 인지할 수 있도록 했습니다
- 바깥 클릭뿐 아니라 Escape 키 입력 시에도 Dropdown이 닫히도록 keydown 리스너를 추가했습니다 - 키보드 사용자도 패널을 닫을 수 있도록 접근성을 개선했습니다
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`:
- Around line 1-8: Dropdown.tsx는 useState, useEffect, document.addEventListener를
사용하므로 App Router에서 클라이언트 컴포넌트로 명시되어야 합니다. Dropdown 컴포넌트 파일의 최상단에 "use client"
지시문을 복원하고, DropdownContext/DropdownProvider 관련 코드가 서버 컴포넌트로 해석되지 않도록 유지하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5ec7ad22-baeb-4aaf-9ff7-d5265626ae0a
📒 Files selected for processing (4)
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/priority/priority-selector/PrioritySelector.tsxpackages/timo-design-system/src/components/tag/tag-selector/TagSelector.stories.tsxpackages/timo-design-system/src/components/tag/tag-selector/TagSelector.tsx
| import { cn } from "@lib"; | ||
| import { | ||
| createContext, | ||
| useContext, | ||
| useEffect, | ||
| useRef, | ||
| useState, | ||
| type ButtonHTMLAttributes, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Next.js 15 App Router client component에서 "use client" 지시어를 제거하면 useState 사용 시 어떤 에러가 발생하나요?
💡 Result:
Next.js App Router에서 컴포넌트는 기본적으로 서버 컴포넌트(Server Component)로 간주됩니다 [1][2]. 서버 컴포넌트 환경에서는 useState와 같은 리액트 클라이언트 훅(React Client Hooks)을 사용할 수 없으며, "use client" 지시어를 제거하면 다음과 같은 에러가 발생합니다. 1. 개발 환경 및 빌드 단계: Next.js는 컴포넌트가 서버 컴포넌트임에도 불구하고 클라이언트 훅을 사용하려고 시도한다는 점을 감지하여 빌드/실행 시 "You are using a React client hook in a Server Component"와 같은 경고 또는 에러를 발생시킵니다 [3][2]. 2. 런타임 에러: "use client" 없이 클라이언트 훅을 호출하면 해당 훅을 서버 환경에서 실행하려고 시도하게 되어, 환경에 따라 "TypeError: Cannot read properties of null (reading 'useState')"와 같은 런타임 에러가 발생할 수 있습니다 [4]. 이는 서버 사이드 렌더링(SSR) 과정에서 리액트의 상태 관리 모듈이 적절하게 로드되지 않았거나 서버 환경에서 정의되지 않은 동작을 수행하려 하기 때문입니다 [4]. 결론적으로, "use client" 지시어는 해당 모듈이 클라이언트 번들에 포함되어야 함을 알리는 진입점 역할을 하며 [5], 이를 누락하면 리액트는 해당 컴포넌트를 서버 컴포넌트로 처리하여 클라이언트 전용 기능인 useState의 사용을 허용하지 않습니다 [1][6].
Citations:
- 1: Why do I get 'useState is not a function' or 'onClick' errors in a Next.js App Router component? Kevinchamplin/til#36
- 2: https://stackoverflow.com/questions/74965849/youre-importing-a-component-that-needs-usestate-it-only-works-in-a-client-comp
- 3: https://nextjs.org/docs/messages/react-client-hook-in-server-component
- 4: https://stackoverflow.com/questions/79435326/cannot-read-properties-of-null-reading-usestate-in-next-js-15-app-router
- 5: https://yceffort.kr/en/2026/05/use-client-deep-dive
- 6: https://react.dev/reference/rsc/use-client
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx"
echo "=== file outline ==="
ast-grep outline "$file" --view expanded || true
echo
echo "=== file content (numbered) ==="
wc -l "$file"
cat -n "$file" | sed -n '1,220p'Repository: Team-Timo/Timo-client
Length of output: 5301
"use client"를 복원하세요. packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx는 useState, useEffect, document.addEventListener를 사용하므로 App Router에서 서버 컴포넌트로 해석되면 오류가 납니다. 공식 문서: https://nextjs.org/docs/app/building-your-application/rendering/client-components
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`
around lines 1 - 8, Dropdown.tsx는 useState, useEffect,
document.addEventListener를 사용하므로 App Router에서 클라이언트 컴포넌트로 명시되어야 합니다. Dropdown
컴포넌트 파일의 최상단에 "use client" 지시문을 복원하고, DropdownContext/DropdownProvider 관련 코드가 서버
컴포넌트로 해석되지 않도록 유지하세요.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx (1)
114-127: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
DropdownItem에{children}를 렌더링하세요. 버튼 속성 확장은 잘 맞췄지만, 현재<button />이라Dropdown.Item안의 라벨/아이콘이 모두 사라집니다.TagSelector와PrioritySelector가 바로 영향받으니,button내부에{children}를 넣어주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx` around lines 114 - 127, `DropdownItem` is currently rendered as a self-closing button, so any nested label/icon content passed through `Dropdown.Item` is dropped. Update the `DropdownItem` component in `Dropdown.tsx` to accept and render `children` inside the `<button>` while keeping the existing `DropdownItemProps`/`cn` className handling intact; this will restore the content used by `TagSelector` and `PrioritySelector`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx`:
- Around line 114-127: `DropdownItem` is currently rendered as a self-closing
button, so any nested label/icon content passed through `Dropdown.Item` is
dropped. Update the `DropdownItem` component in `Dropdown.tsx` to accept and
render `children` inside the `<button>` while keeping the existing
`DropdownItemProps`/`cn` className handling intact; this will restore the
content used by `TagSelector` and `PrioritySelector`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 995f3a20-99a6-4461-bb50-3bd5bd3bac43
📒 Files selected for processing (1)
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx
yumin-kim2
left a comment
yumin-kim2
left a comment
There was a problem hiding this comment.
Dropdown, Dropdown.Trigger, Dropdown.Panel, Dropdown.Item으로 역할을 딱딱 나눠서 만드신 게 좋았어요. PrioritySelector랑 TagSelector 두 군데에서 똑같은 구조를 그대로 재사용할 수 있게 깔끔하게 잘 설계된 것 같습니다!!
그리고 여러 컴포넌트(Dropdown, Trigger, Panel)를 한 세트로 묶어서 Dropdown.Trigger처럼 쓸 수 있게 만드는 방법이 있다는 것도 이번에 처음 알게 됐습니당👀 코드랑 PR 보면서 많이 배운 것 같아요. 너무 수고하셨씁니다!!!!! ✨✨
|
|
||
| export const 기타: Story = { | ||
| args: { trigger: TRIGGER, tags: TAGS, selected: "기타" }, | ||
| }; |
There was a problem hiding this comment.
왼쪽 사이드바에 추가+가 없는데 의도하신게 맞는지 궁금합니다!
| if (event.key === "Escape") { | ||
| setIsOpen(false); |
There was a problem hiding this comment.
Esc 키로 닫히는 기능까지!!👍👍 굿굿입니당
| const rootRef = useRef<HTMLDivElement>(null); | ||
|
|
||
| useEffect(() => { | ||
| if (!isOpen) return; |
There was a problem hiding this comment.
열려있을 때만 이벤트 리스너를 등록하고, cleanup 함수로 확실히 지워주는 패턴도 새롭네요!
이렇게 하면 불필요한 이벤트 감시를 줄이고 메모리 누수도 막을 수 있다는 점이 신기하고 사소하지만 꼭 챙겨야하는 부분이라고 느꼈어요👀
|
|
||
| const DropdownContext = createContext<DropdownContextValue | null>(null); | ||
|
|
||
| const useDropdownContext = (): DropdownContextValue => { |
There was a problem hiding this comment.
방어코드 에러 메세지 처리까지 신경 쓰신 부분이 좋아요!! 잘못 써도 원인을 바로 알 수 있겠네요 👍
There was a problem hiding this comment.
저도 Tab 컴포넌트를 구현할 때 부모 컴포넌트로부터 선택 여부를 props로 받았는데, 왜 제 경우에는 isSelected/onClick을 쓰고 이 컴포넌트에서는 selected/onSelect를 쓰는지 궁금해서 찾아봤습니다.
정리해보니 이 컴포넌트는 여러 항목 중 하나를 고르는 선택기라서 selected 값과 onSelect이 자연스럽고, 제가 만든 Tab은 단일 항목이라 isSelected와 onClick이 더 적절하다는 차이가 있더라구요!! (제가 이해한게 맞을까..요?..)
덕분에 제어 컴포넌트 패턴을 조금 더 이해하게 됐습니다.!! 민아님 코드 보면서 많이 배우네요🫰🏻👍😍
2 participants
ISSUE 🔗
close #73
What is this PR? 🔍
우선순위(
PrioritySelector)와 태그(TagSelector) 선택 드롭다운 컴포넌트를 추가하고, 두 컴포넌트가 공유하는 패널·행·트리거 동작을Dropdowncompound 컴포넌트로 추출했습니다. 작업 중 발견한Color/Typography의 깨진 import 경로도 함께 정리했습니다.배경
PriorityIcon(단일 색상 점)만 있었고, 매우중요/중요/보통/낮음 중 하나를 선택하는 리스트형 UI나 태그를 선택하는 UI는 없었습니다.PrioritySelector구현 후 동일한 형태의 태그 선택 드롭다운(TagSelector)이 필요해지면서 두 컴포넌트 간 패널·행 스타일 중복이 발생했습니다. 처음 추출한Dropdown은 항상 렌더링되는 패널일 뿐 트리거·열림/닫힘 동작이 없어 "드롭다운"이라는 이름에 걸맞지 않았습니다.PrioritySelector를 구현하고, 두 번째 소비 컴포넌트(TagSelector)가 생긴 시점에 공통 셸을Dropdown/Dropdown.Itemcompound 컴포넌트로 추출했습니다. 이후 실제 트리거 클릭 시 토글되고 바깥 클릭 시 닫히는 동작까지Dropdown에 내재화했습니다.PrioritySelector
priority컴포넌트 셋(5개 variant)이 "현재 선택된 우선순위를 배경색으로 강조해 보여주는 리스트"였고, 기존PriorityIcon은 점 하나만 그리는 컴포넌트라 그대로 재사용할 수 없었습니다.selected?: PriorityLevelprop으로 선택 상태를 제어하고,onSelect콜백으로 클릭 이벤트를 상위로 전달하는 controlled 컴포넌트입니다. 매우중요/중요/보통/낮음 4개 행을 순회하며, 선택된 행만PRIORITY_BG_COLOR에 정의된 토큰 색상(bg-timo-red,bg-timo-orange,bg-timo-gray,bg-timo-gray-900)을 배경에 적용하고PriorityIcon에white를 넘겨 점 색상도 함께 바꿉니다. 선택 상태 전환 시 배경·텍스트·아이콘 색상에transition-colors duration-200을 적용했습니다.Dropdown 공통 컴포넌트 추출 및 열림/닫힘 동작
PrioritySelector와TagSelector가 공유하는 패널·행 스타일을Dropdown/Dropdown.Item으로 분리했고, 이후Dropdown.Trigger/Dropdown.Panel과 내부 open 상태를 추가해 실제 트리거-토글-바깥클릭 닫힘 동작을 갖는 드롭다운으로 완성했습니다.Dropdown루트가useState로isOpen을 소유하고DropdownContext로 하위에 전달합니다.Dropdown.Trigger는 클릭 시toggle()을 호출하고,Dropdown.Panel은isOpen이 true일 때만absolute top-full left-0위치에 렌더링됩니다. 루트div에ref를 걸어mousedown이벤트에서ref.current.contains(event.target)이 아니면setIsOpen(false)로 닫는 방식으로 바깥 클릭을 감지합니다.PrioritySelector/TagSelector는 새로 추가된trigger: ReactNodeprop을 받아<Dropdown><Dropdown.Trigger>{trigger}</Dropdown.Trigger><Dropdown.Panel>...</Dropdown.Panel></Dropdown>로 스스로 트리거를 조합합니다.useState/useEffect)와"use client"를 갖는 첫 컴포넌트입니다. 항목을 선택해도 자동으로 닫히지는 않으며(트리거 재클릭 또는 바깥 클릭으로만 닫힘), 이 부분은 이번 PR 범위에서 의도적으로 제외했습니다.TagSelector 및 TagIcon 이름 변경
Tag컴포넌트를TagIcon으로 이름을 바꾸고,Dropdown기반의TagSelector를 새로 추가했습니다.TagSelector가 내부적으로 태그 값 유니언 타입을Tag로 export하면서 기존 컴포넌트명Tag와 충돌해TagIcon으로 구분했습니다.TagSelector는PrioritySelector와 동일하게trigger/selected/onSelectcontrolled 패턴을 따르며, 목록 마지막에onAddClick콜백을 가진 "추가" 행을 포함합니다.Color·Typography 경로 정리
Color,Typography컴포넌트를components/tokens/폴더 하위로 이동하고index.ts참조 경로를 갱신했습니다.tokens/폴더 이동은 되어 있는데index.ts참조가 갱신되지 않아 타입체크가 깨져 있는 상태를 발견해 함께 정리했습니다.To Reviewers
PrioritySelector의 배경색 매핑(매우중요→timo-red,중요→timo-orange,보통→timo-gray,낮음→timo-gray-900)이 Figma에 지정된 hex와 정확히 일치하는지 한 번 더 확인해 주세요.Dropdown이 패키지 내 첫 stateful("use client") 컴포넌트가 됐는데, 이 경계가 적절한지(design-system 패키지 안에 상태를 두는 것이 맞는지) 검토 부탁드립니다.바깥 클릭 감지를
mousedown+ ref containment로 구현했는데, 터치 디바이스나 포털(portal) 안에 트리거/패널이 분리되어 렌더링되는 시나리오는 고려하지 않았습니다 — 현재는 같은 DOM 서브트리 안에 있다는 전제입니다.항목 선택 시 자동으로 닫는 동작은 의도적으로 제외했습니다 — 필요하면 후속 작업으로 추가하겠습니다.
Screenshot 📷
Components/Priority/PrioritySelector,Components/Tag/TagSelector,Components/layout/Dropdown스토리에서 트리거 클릭 → 패널 열림 → 바깥 클릭 시 닫힘까지 직접 확인해 주세요.Test Checklist ✔
pnpm check-types통과 (전체 워크스페이스)pnpm lint통과 (전체 워크스페이스)