[FEAT] Dropdown 컴포넌트 구현 #78
Conversation
드롭다운 open 상태 그림자용 --shadow-timo CSS 변수를 추가했습니다. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
피그마 디자인 스펙에 맞춰 Dropdown 컴포넌트를 구현했습니다. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughDropdown 공통 컴포넌트와 Storybook 예시가 추가되었고, 컴포넌트 진입점에서 재export됩니다. Tailwind 테마에는 새 그림자 토큰이 정의되고 import 순서가 조정되었습니다. ChangesDropdown 컴포넌트 및 테마 토큰
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ehye1
changed the title
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: 6
🤖 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/tailwind-config/theme.css`:
- Around line 6-8: Tailwind v4 token CSS in theme.css is being flagged by
Biome/Stylelint because the parser/plugin config is still treating `@theme` as
invalid. Update the lint/config setup used for packages/tailwind-config so it
recognizes Tailwind v4 `@theme` syntax, matching the other token CSS files in this
package. Check the Biome and Stylelint configuration entries that parse
CSS/Tailwind directives, and align them with tailwindcss@^4.3.1 rather than
changing the `@theme` block itself.
In `@packages/timo-design-system/src/components/dropdown/Dropdown.stories.tsx`:
- Around line 16-24: Default story state is hardcoded in DefaultStory, so the
Storybook args for Dropdown are ignored and Controls do not reflect the rendered
UI. Update Default and DefaultStory so the rendered dropdown is driven by args
from the Storybook story, using DropdownProps and the existing Dropdown
component props (items, value, onChange) instead of fixed ["기본", "7일"] values,
keeping args and render in sync.
In `@packages/timo-design-system/src/components/dropdown/Dropdown.tsx`:
- Around line 87-112: The Dropdown listbox currently makes every option
tabbable, which breaks the expected WAI-ARIA listbox keyboard pattern. Update
Dropdown.tsx so only a single element participates in tab order (the trigger or
listbox container), and move option focus management to roving tabindex or
aria-activedescendant within the listbox. Keep the option rendering in the
items.map block, but change the interaction logic in the option handling so
arrow keys navigate between items and Enter/Space selects the active item.
- Line 19: Rename the boolean state in Dropdown.tsx from open to isOpen and
update the setter to setIsOpen so it follows the boolean naming convention. Make
sure every reference in the Dropdown component that uses open or setOpen is
updated consistently, including any JSX conditions or event handlers tied to the
state.
- Around line 27-46: The Dropdown close handling in the open-state effect does
not restore focus to the trigger button, so keyboard users can lose focus after
Escape or outside click. Update the Dropdown component to keep a trigger button
ref (for the trigger <button> and any related handler logic), and in both the
Escape key path and the outside-click path call triggerRef.current?.focus()
immediately after setOpen(false). Use the existing useEffect, handleKeyDown, and
handleClickOutside logic in Dropdown.tsx to place the focus restore
consistently.
- Around line 92-93: `Dropdown`의 리스트 key가 `key={item}`이라 중복 문자열이 들어오면 충돌할 수
있습니다. `items.map`에서 렌더링하는 `li`의 key를 문자열 값 대신 고유하게 만들도록 수정하세요: 가능하면
`DropdownProps.items`를 `{ id, label }` 형태로 바꾸고 `Dropdown` 컴포넌트에서 `id`를 key로
사용하며, 구조 변경이 어렵다면 최소한 `item`과 `i`를 조합한 key로 방어적으로 처리하세요.
🪄 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: b0806be5-1ee4-483b-98a6-aee28433358b
📒 Files selected for processing (4)
packages/tailwind-config/theme.csspackages/timo-design-system/src/components/dropdown/Dropdown.stories.tsxpackages/timo-design-system/src/components/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/index.ts
| @theme { | ||
| --shadow-timo: 0px 0px 3px 0px rgba(0, 0, 0, 0.12); | ||
| } No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Tailwind 버전 확인 및 다른 tokens 파일의 `@theme` 사용 여부 확인
fd package.json --exec grep -l '"tailwindcss"' {} \;
fd . packages/tailwind-config -e css --exec grep -n "`@theme`\|`@tailwind`\|`@import`" {} \;Repository: Team-Timo/Timo-client
Length of output: 418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json files =="
sed -n '1,220p' packages/tailwind-config/package.json
echo
sed -n '1,220p' packages/timo-design-system/package.json
echo
echo "== tailwind-config css files =="
for f in packages/tailwind-config/*.css packages/tailwind-config/tokens/*.css; do
[ -f "$f" ] || continue
echo "--- $f ---"
nl -ba "$f" | sed -n '1,220p'
echo
doneRepository: Team-Timo/Timo-client
Length of output: 2160
Biome/Stylelint 설정을 Tailwind v4 기준으로 맞춰주세요.
이 저장소는 tailwindcss@^4.3.1를 사용하고, packages/tailwind-config의 다른 token CSS들도 모두 @theme 문법을 씁니다. 따라서 경고는 코드보다는 파서/플러그인 설정 문제일 가능성이 큽니다.
Tailwind v4 @theme 문서: https://tailwindcss.com/docs/theme
🧰 Tools
🪛 Biome (2.5.1)
[error] 6-8: Tailwind-specific syntax is disabled.
(parse)
🪛 Stylelint (17.14.0)
[error] 6-6: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 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/tailwind-config/theme.css` around lines 6 - 8, Tailwind v4 token CSS
in theme.css is being flagged by Biome/Stylelint because the parser/plugin
config is still treating `@theme` as invalid. Update the lint/config setup used
for packages/tailwind-config so it recognizes Tailwind v4 `@theme` syntax,
matching the other token CSS files in this package. Check the Biome and
Stylelint configuration entries that parse CSS/Tailwind directives, and align
them with tailwindcss@^4.3.1 rather than changing the `@theme` block itself.
Source: Linters/SAST tools
| {items.map((item, i) => ( | ||
| <li key={item} className="flex flex-col gap-1"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
key={item}은 중복 항목이 있으면 충돌합니다.
items가 string[]이라 값이 중복되면(예: ["기본", "기본"]) React key가 중복되어 리스트 렌더링이 꼬일 수 있습니다. 현재는 고유 id가 없는 문자열 배열이므로, 상위에서 { id, label } 형태로 넘기거나 최소한 key={${item}-${i}}로 방어하는 게 안전합니다.
As per path instructions, "동적 리스트의 key prop: 반드시 고유 id 사용 (index 금지)" — 현재 문자열 자체를 key로 쓰고 있어 고유성이 보장되지 않습니다.
🔑 제안 diff (임시 방어)
- {items.map((item, i) => (
- <li key={item} className="flex flex-col gap-1">
+ {items.map((item, i) => (
+ <li key={`${item}-${i}`} className="flex flex-col gap-1">근본적으로는 DropdownProps.items를 { id: string; label: string }[]로 바꾸는 것을 권장합니다.
📝 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.
| {items.map((item, i) => ( | |
| <li key={item} className="flex flex-col gap-1"> | |
| {items.map((item, i) => ( | |
| <li key={`${item}-${i}`} className="flex flex-col gap-1"> |
🤖 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/dropdown/Dropdown.tsx` around
lines 92 - 93, `Dropdown`의 리스트 key가 `key={item}`이라 중복 문자열이 들어오면 충돌할 수 있습니다.
`items.map`에서 렌더링하는 `li`의 key를 문자열 값 대신 고유하게 만들도록 수정하세요: 가능하면
`DropdownProps.items`를 `{ id, label }` 형태로 바꾸고 `Dropdown` 컴포넌트에서 `id`를 key로
사용하며, 구조 변경이 어렵다면 최소한 `item`과 `i`를 조합한 key로 방어적으로 처리하세요.
Source: Path instructions
open을 isOpen으로 변경하고 닫힐 때 트리거 버튼으로 포커스를 복귀했습니다. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/timo-design-system/src/components/dropdown/Dropdown.tsx (2)
99-100: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
key={item}은 여전히 중복 항목에 취약합니다.이전 리뷰에서 동일하게 지적된 이슈가 아직 반영되지 않았습니다.
items가string[]이라 중복 값이 있으면 React key 충돌이 발생할 수 있습니다.As per path instructions, "동적 리스트의
keyprop: 반드시 고유id사용 (index 금지)".🔑 제안 diff (임시 방어)
- {items.map((item, i) => ( - <li key={item} className="flex flex-col gap-1"> + {items.map((item, i) => ( + <li key={`${item}-${i}`} className="flex flex-col gap-1">🤖 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/dropdown/Dropdown.tsx` around lines 99 - 100, The Dropdown item list still uses a non-unique React key, so duplicate string values in items can collide. Update the items.map render in Dropdown to use a truly unique stable identifier for each item instead of key={item}; if the list only has strings, derive or pass a unique id alongside each item rather than relying on the label value or index. Make sure the change is applied in the Dropdown component’s list rendering.Source: Path instructions
99-116: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffListbox 키보드 탐색(roving tabindex/
aria-activedescendant) 여전히 미반영.모든 옵션에
tabIndex={0}을 부여하는 구조가 그대로 유지되어 있습니다. 이전 리뷰에서 지적된 대로 WAI-ARIA Listbox 패턴은 단일 탭스톱 + 방향키 탐색을 권장합니다. 지금 당장 필수는 아니지만 디자인 시스템 컴포넌트인 만큼 추후 개선을 검토해주세요.참고: WAI-ARIA APG Listbox Pattern
🤖 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/dropdown/Dropdown.tsx` around lines 99 - 116, The Dropdown listbox still uses a tabbable span for every option, so the WAI-ARIA listbox keyboard pattern is not implemented. Update Dropdown’s option rendering in the items.map block to use a single tab stop on the listbox container and manage focus with roving tabindex or aria-activedescendant, while keeping handleSelect and the existing option selection behavior intact. Ensure the component’s keyboard handling supports arrow-key navigation through options instead of leaving tabIndex={0} on each item.
🤖 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/dropdown/Dropdown.tsx`:
- Around line 38-45: The outside-click handler in Dropdown is forcing focus back
to the trigger via closeWithFocus(), which can override the user’s intended
focus target. Update handleClickOutside in Dropdown.tsx so outside mouse clicks
only close the menu without restoring focus, and reserve trigger focus
restoration for Escape-driven closes or other explicit keyboard-dismiss paths.
Keep the logic localized around handleClickOutside and closeWithFocus so the
dropdown still closes correctly while preserving browser-default focus movement.
- Around line 23-26: `handleSelect` in `Dropdown` closes the menu without
restoring focus, so keyboard users lose their place after choosing an option.
Update the selection flow to use `closeWithFocus()` instead of only
`setIsOpen(false)`, and make sure `closeWithFocus` is declared before
`handleSelect` so it can be called there. Keep `onChange(item)` in the same
selection path, but ensure focus returns to the trigger after the dropdown
closes.
---
Duplicate comments:
In `@packages/timo-design-system/src/components/dropdown/Dropdown.tsx`:
- Around line 99-100: The Dropdown item list still uses a non-unique React key,
so duplicate string values in items can collide. Update the items.map render in
Dropdown to use a truly unique stable identifier for each item instead of
key={item}; if the list only has strings, derive or pass a unique id alongside
each item rather than relying on the label value or index. Make sure the change
is applied in the Dropdown component’s list rendering.
- Around line 99-116: The Dropdown listbox still uses a tabbable span for every
option, so the WAI-ARIA listbox keyboard pattern is not implemented. Update
Dropdown’s option rendering in the items.map block to use a single tab stop on
the listbox container and manage focus with roving tabindex or
aria-activedescendant, while keeping handleSelect and the existing option
selection behavior intact. Ensure the component’s keyboard handling supports
arrow-key navigation through options instead of leaving tabIndex={0} on each
item.
🪄 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: 30063e32-39ad-4fda-bc46-e8acd2b2a7cf
📒 Files selected for processing (1)
packages/timo-design-system/src/components/dropdown/Dropdown.tsx
| const handleSelect = (item: string) => { | ||
| onChange(item); | ||
| setIsOpen(false); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
옵션 선택 후 포커스가 돌아오지 않아요.
Escape/외부 클릭 시에는 closeWithFocus()로 트리거에 포커스를 복원하지만, handleSelect는 setIsOpen(false)만 호출합니다. 옵션(span[role="option"])에 있던 포커스가 리스트 언마운트와 함께 사라져 키보드 사용자는 포커스를 잃게 됩니다. 접근성 가이드에서도 선택 후 트리거로 포커스를 복귀시키도록 권장합니다.
🎯 제안 diff
const handleSelect = (item: string) => {
onChange(item);
- setIsOpen(false);
+ closeWithFocus();
};(단, closeWithFocus는 handleSelect 아래에 정의되어 있으므로 함수 선언 순서 조정 필요 — closeWithFocus를 handleSelect보다 먼저 선언하세요.)
관련 문서: Accessible Combobox Pattern: ARIA + Keyboard Guide
🤖 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/dropdown/Dropdown.tsx` around
lines 23 - 26, `handleSelect` in `Dropdown` closes the menu without restoring
focus, so keyboard users lose their place after choosing an option. Update the
selection flow to use `closeWithFocus()` instead of only `setIsOpen(false)`, and
make sure `closeWithFocus` is declared before `handleSelect` so it can be called
there. Keep `onChange(item)` in the same selection path, but ensure focus
returns to the trigger after the dropdown closes.
| const handleClickOutside = (e: MouseEvent) => { | ||
| if ( | ||
| containerRef.current && | ||
| !containerRef.current.contains(e.target as Node) | ||
| ) { | ||
| closeWithFocus(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
외부 클릭 시 강제 포커스 복귀가 오히려 사용자 의도를 방해할 수 있어요.
mousedown이 컨테이너 밖에서 발생하면 무조건 closeWithFocus()를 호출해 트리거로 포커스를 되돌립니다. 하지만 사용자가 다른 포커스 가능한 요소(예: 다음 입력창, 다른 버튼)를 클릭해 드롭다운을 닫으려 한 경우에도 포커스가 트리거로 강제 이동해, 사용자가 클릭한 대상으로 포커스가 이동하지 못할 수 있습니다. Escape로 닫을 때만 트리거로 포커스를 복귀시키고, 외부 클릭 시에는 브라우저 기본 포커스 이동에 맡기는 편이 자연스러운 흐름입니다.
💡 제안 diff
const handleClickOutside = (e: MouseEvent) => {
if (
containerRef.current &&
!containerRef.current.contains(e.target as Node)
) {
- closeWithFocus();
+ setIsOpen(false);
}
};📝 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.
| const handleClickOutside = (e: MouseEvent) => { | |
| if ( | |
| containerRef.current && | |
| !containerRef.current.contains(e.target as Node) | |
| ) { | |
| closeWithFocus(); | |
| } | |
| }; | |
| const handleClickOutside = (e: MouseEvent) => { | |
| if ( | |
| containerRef.current && | |
| !containerRef.current.contains(e.target as Node) | |
| ) { | |
| setIsOpen(false); | |
| } | |
| }; |
🤖 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/dropdown/Dropdown.tsx` around
lines 38 - 45, The outside-click handler in Dropdown is forcing focus back to
the trigger via closeWithFocus(), which can override the user’s intended focus
target. Update handleClickOutside in Dropdown.tsx so outside mouse clicks only
close the menu without restoring focus, and reserve trigger focus restoration
for Escape-driven closes or other explicit keyboard-dismiss paths. Keep the
logic localized around handleClickOutside and closeWithFocus so the dropdown
still closes correctly while preserving browser-default focus movement.
| @@ -0,0 +1,24 @@ | |||
| import { useState } from "react"; | |||
|
|
|||
| import { Dropdown } from "./Dropdown"; | |||
| className?: string; | ||
| } | ||
|
|
||
| export const Dropdown = ({ |
There was a problem hiding this comment.
제가 작성해 둔 이전 PR을 구현해 놓고 보니 해당 뷰 드롭다운 컴포넌트도 컴파운드 패턴 적용해서 구현해 볼 수 있을 것 같네요!
해당 PR 머지하고, 뷰 전용 드롭다운으로 네이밍 교체해서 구현해 보면 좋을 것 같아요.
구현해 주신 선택 시 자동 닫힘 & 닫힌 후 트리거로 포커스 복귀 기능은 컴파운드 패턴 전용 컴포넌트에 제가 구현해 놓겠습니다!
There was a problem hiding this comment.
그렇네요 너무 좋습니다🫰🏻💍 폴더도 그에 맞춰 수정하겠습니다!
2 participants
ISSUE 🔗
close #74
What is this PR? 🔍
피그마
dropdown_view스펙에 맞춰 Dropdown 컴포넌트를 구현했습니다.h-8,px-2,rounded-[4px],typo-headline-m-14border-x border-b, 아이템 사이 dividershadow-timo적용--shadow-timo토큰tailwind-config/theme.css에 추가코드래빗 반영
open→isOpen네이밍 컨벤션 수정To Reviewers
Screenshot 📷
Test Checklist ✔
Components/Dropdown렌더링 확인