Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 5 additions & 1 deletion packages/tailwind-config/theme.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
@import "./tokens/colors.css";
@import "./tokens/typography.css";
@import "./tokens/radius.css";
@import "./scrollbar.css";
@import "./scrollbar.css";

@theme {
--shadow-timo: 0px 0px 3px 0px rgba(0, 0, 0, 0.12);
}
Comment on lines +6 to +8

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 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
done

Repository: 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

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { useState } from "react";

import { Dropdown } from "./Dropdown";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

요거 절대 경로로 통일해 줍시당


import type { Meta, StoryObj } from "@storybook/react";

const meta = {
title: "Components/Dropdown",
component: Dropdown,
parameters: { layout: "centered" },
} satisfies Meta<typeof Dropdown>;

export default meta;
type Story = StoryObj<typeof meta>;

const DefaultStory = () => {
const [value, setValue] = useState("");
return <Dropdown items={["기본", "7일"]} value={value} onChange={setValue} />;
};

export const Default: Story = {
args: { items: [], value: "", onChange: () => {} },
render: () => <DefaultStory />,
};
Comment thread
ehye1 marked this conversation as resolved.
122 changes: 122 additions & 0 deletions packages/timo-design-system/src/components/dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { cn } from "@lib";
import { useEffect, useRef, useState } from "react";

export interface DropdownProps {
items: string[];
value: string;
onChange: (value: string) => void;
placeholder?: string;
className?: string;
}

export const Dropdown = ({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

제가 작성해 둔 이전 PR을 구현해 놓고 보니 해당 뷰 드롭다운 컴포넌트도 컴파운드 패턴 적용해서 구현해 볼 수 있을 것 같네요!
해당 PR 머지하고, 뷰 전용 드롭다운으로 네이밍 교체해서 구현해 보면 좋을 것 같아요.

구현해 주신 선택 시 자동 닫힘 & 닫힌 후 트리거로 포커스 복귀 기능은 컴파운드 패턴 전용 컴포넌트에 제가 구현해 놓겠습니다!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

그렇네요 너무 좋습니다🫰🏻💍 폴더도 그에 맞춰 수정하겠습니다!

items,
value,
onChange,
placeholder = "기본",
className,
}: DropdownProps) => {
const [isOpen, setIsOpen] = useState(false);
const containerRef = useRef<HTMLDivElement>(null);
const triggerRef = useRef<HTMLButtonElement>(null);

const handleSelect = (item: string) => {
onChange(item);
setIsOpen(false);
};
Comment on lines +23 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

옵션 선택 후 포커스가 돌아오지 않아요.

Escape/외부 클릭 시에는 closeWithFocus()로 트리거에 포커스를 복원하지만, handleSelectsetIsOpen(false)만 호출합니다. 옵션(span[role="option"])에 있던 포커스가 리스트 언마운트와 함께 사라져 키보드 사용자는 포커스를 잃게 됩니다. 접근성 가이드에서도 선택 후 트리거로 포커스를 복귀시키도록 권장합니다.

🎯 제안 diff
   const handleSelect = (item: string) => {
     onChange(item);
-    setIsOpen(false);
+    closeWithFocus();
   };

(단, closeWithFocushandleSelect 아래에 정의되어 있으므로 함수 선언 순서 조정 필요 — closeWithFocushandleSelect보다 먼저 선언하세요.)

관련 문서: 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 closeWithFocus = () => {
setIsOpen(false);
triggerRef.current?.focus();
};

useEffect(() => {
if (!isOpen) return;
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === "Escape") closeWithFocus();
};
const handleClickOutside = (e: MouseEvent) => {
if (
containerRef.current &&
!containerRef.current.contains(e.target as Node)
) {
closeWithFocus();
}
};
Comment on lines +38 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 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.

Suggested change
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.

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

return (
<div
ref={containerRef}
className={cn("inline-flex flex-col", isOpen && "shadow-timo", className)}
>
<button
ref={triggerRef}
type="button"
onClick={() => setIsOpen((prev) => !prev)}
aria-haspopup="listbox"
aria-expanded={isOpen}
className={cn(
"flex h-8 items-center gap-2.5 bg-white px-2",
"border-timo-gray-500 border",
isOpen ? "rounded-t-[4px]" : "rounded-[4px]",
)}
>
<span className="typo-headline-m-14 text-timo-gray-900 whitespace-nowrap">
{value || placeholder}
</span>
<div className="flex size-6 shrink-0 items-center justify-center">
<svg
width="16"
height="8"
viewBox="0 0 16 8"
fill="none"
aria-hidden="true"
className="text-timo-gray-900"
>
<path
d={isOpen ? "M1 7L8 1L15 7" : "M1 1L8 7L15 1"}
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
</svg>
</div>
</button>

{isOpen && (
<ul
role="listbox"
className="border-timo-gray-500 flex flex-col gap-1.5 rounded-b-[4px] border-x border-b bg-white px-2 py-1"
>
{items.map((item, i) => (
<li key={item} className="flex flex-col gap-1">
Comment on lines +99 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

key={item}은 중복 항목이 있으면 충돌합니다.

itemsstring[]이라 값이 중복되면(예: ["기본", "기본"]) 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.

Suggested change
{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

<span
role="option"
aria-selected={item === value}
tabIndex={0}
onClick={() => handleSelect(item)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") handleSelect(item);
}}
className="typo-body-m-12 text-timo-gray-900 cursor-pointer"
>
{item}
</span>
{i < items.length - 1 && (
<div className="bg-timo-gray-500 h-px w-full" />
)}
</li>
))}
</ul>
)}
</div>
);
};
1 change: 1 addition & 0 deletions packages/timo-design-system/src/components/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { Checkbox } from "@components/checkbox/Checkbox";
export { Dropdown } from "@components/dropdown/Dropdown";
export { Color } from "@components/color/Color";
export { Typography } from "@components/typography/Typography";
export { Tag } from "@components/tag/Tag";
Expand Down
Loading