[FEAT] 반복 일정/시간 선택 드롭다운 컴포넌트 추가#87
Conversation
- 반복 일정(매일/매주/매달) 선택 드롭다운 컴포넌트를 추가했습니다 - 선택된 빈도에 따라 세부 설정(주간 요일 체크박스 / 월간 반복일 입력) 섹션이 교체되도록 구현했습니다 - 월간 세부 설정 값이 피그마 디자인처럼 우측에 고정되도록 정렬했습니다 - Dropdown 트리거와 패널 사이 여백을 제거해 피그마 디자인과 일치시켰습니다
- 예상 시간을 직접 입력하거나 목록에서 선택할 수 있는 드롭다운 컴포넌트를 추가했습니다 - AI 추천 시간 선택 버튼과 활성화 상태 스타일을 구현했습니다
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Walkthrough디자인 시스템에 ChangesTimeSelector·RepeatSelector 구현
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TimeSelector
participant DropdownItem
User->>TimeSelector: AI 토글 클릭
TimeSelector->>TimeSelector: onSelect?.("ai") 호출
User->>TimeSelector: 시간 입력 변경
TimeSelector->>TimeSelector: onTimeChange?.(value) 호출
User->>DropdownItem: 프리셋 시간 선택
DropdownItem->>TimeSelector: onSelect?.(minute) 호출
sequenceDiagram
participant User
participant RepeatSelector
participant RepeatFrequencyList
participant RepeatWeeklyDetailSection
participant RepeatMonthlyDetailSection
User->>RepeatSelector: 드롭다운 열기
RepeatSelector->>RepeatFrequencyList: 빈도 목록 렌더링
User->>RepeatFrequencyList: 빈도 선택
RepeatFrequencyList->>RepeatSelector: onFrequencyChange 호출
alt weekly
RepeatSelector->>RepeatWeeklyDetailSection: 요일 체크박스 렌더링
User->>RepeatWeeklyDetailSection: 요일 토글
RepeatWeeklyDetailSection->>RepeatSelector: onWeekdayToggle 호출
else monthly
RepeatSelector->>RepeatMonthlyDetailSection: 반복일 입력 렌더링
User->>RepeatMonthlyDetailSection: 반복일 변경
RepeatMonthlyDetailSection->>RepeatSelector: onRepeatDayChange 호출
end
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…to feat/ui/83-repeat-selector
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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/repeat/repeat-selector/RepeatSelector.stories.tsx`:
- Around line 53-153: The Daily, Weekly, and Monthly stories repeat the same
default props, so move the shared values into the story meta-level args and keep
each story focused on only the props it needs to override. Update the
RepeatSelector.stories.tsx setup around the meta export and the
Daily/Weekly/Monthly story objects so common defaults like detailHeading,
options, weekly, and monthly live in one place, while WeeklyDemo and MonthlyDemo
only handle their interactive state.
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx`:
- Around line 41-49: RepeatSelectorProps currently forces weekly and monthly for
every frequency, so daily callers must pass dummy data. Refactor
RepeatSelectorProps in RepeatSelector to a discriminated union keyed by
frequency so daily omits both details, weekly requires only weekly, and monthly
requires only monthly. Update any RepeatSelector usage and internal references
to weekly/monthly to align with the narrowed props shape.
- Around line 64-77: The RepeatSelector dropdown item is using aria-pressed for
a single-choice selection state, which is more appropriate for toggle buttons
than option lists. Update the Dropdown.Item in RepeatSelector to use
aria-current for the currently selected frequency instead, keyed off
selectedFrequency and value, and keep the existing onClick/onSelect behavior
unchanged.
- Around line 105-114: `RepeatMonthlyDetailSectionProps` is currently declared
as a `type` with an intersection, but the Props convention here requires an
`interface` with the `Props` suffix. Update the
`RepeatMonthlyDetailSectionProps` definition in `RepeatMonthlyDetailSection` to
use an `interface` that extends `RepeatMonthlyDetail`, keeping the `ariaLabel`
field, and leave the component signature unchanged.
- Around line 118-124: `RepeatSelector`의 입력 필드가 text라서 숫자가 아닌 값도 그대로 들어갈 수 있습니다.
`RepeatSelector` 컴포넌트의 `repeatDay` 입력 처리와 `onRepeatDayChange` 호출부를 수정해 숫자만 허용하도록
필터링하거나 검증을 추가하고, 필요하면 입력 타입도 숫자 입력에 맞게 조정해 문자·특수문자가 전달되지 않게 해주세요.
- Around line 116-125: `RepeatSelector`의 입력 필드에서 `outline-none`으로 포커스 표시를 완전히
없애고 있어 키보드 접근성이 떨어집니다. `input`의 포커스 스타일은 유지하거나 명시적인 포커스 스타일로 대체하고, `aria-label`만
쓰지 말고 `RepeatSelector` 내 `input`에 고유한 `id`를 부여한 뒤 대응하는 `<label htmlFor>`를 연결해 라벨
연동을 맞춰주세요.
- Around line 63-79: The RepeatSelector option wrapper is an unnecessary div
used only for grouping and keying, so replace the `div` with
`className="contents"` in `RepeatSelector` with a keyed `Fragment` around each
mapped option. Keep the existing `options.map`, `Dropdown.Item`, and separator
rendering logic intact, but use the `Fragment` to preserve the `key={value}`
without adding a non-visual DOM node.
- Line 11: `RepeatFrequency` is a union literal type alias, so it should follow
the naming convention that uses the `Types` suffix. Rename the exported type in
`RepeatSelector` to `RepeatFrequencyTypes`, and update any references in the
same component or nearby imports/exports to match the new identifier.
- Line 155: `Dropdown.Trigger`의 `aria-haspopup="menu"`가 현재 `Dropdown.Panel`과 내부
항목 구조와 맞지 않으므로 수정하세요. `RepeatSelector`에서 메뉴 패턴을 유지할 거라면 `Dropdown.Panel`과 항목 렌더링
쪽에 `role="menu"`/`role="menuitem"`을 맞추고, 그렇지 않다면 `aria-haspopup`를 제거해
`Dropdown.Trigger`의 접근성 의미를 현재 UI 구조에 맞게 정리하세요.
In
`@packages/timo-design-system/src/components/time/time-selector/TimeSelector.tsx`:
- Around line 55-64: The TimeSelector input currently relies only on aria-label,
so add a stable id to the input and connect it to an explicit label using
htmlFor in the surrounding TimeSelector markup. Update the relevant input/label
pairing in TimeSelector so the text field has both an accessible name and a
proper label association, keeping the existing onTimeChange behavior unchanged.
- Around line 55-64: The TimeSelector input is removing the focus indicator via
outline-none, which should be avoided; update the input in TimeSelector to keep
a visible keyboard focus state by removing the outline suppression and adding an
accessible replacement such as focus-visible:ring-* or an equivalent focus
style. Check the input’s className in TimeSelector.tsx and ensure the focus
treatment remains visible for keyboard users, especially when isAiSelected
changes the color styling.
- Around line 68-99: The `TimeSelector` list is using `key={index}` in
`times.map`, which should be replaced with the stable unique `minute` value
already present on each item. Update the `Dropdown.Item` key in `TimeSelector`
so React can track items correctly when the list order changes or items are
added/removed, and keep the existing `isSelected` logic unchanged.
- Line 7: `TimeSelection` is a union type alias, and the naming convention
requires `Types` suffix for union aliases. Update the exported type in
`TimeSelector` to use the `Types` suffix, and then adjust any references that
import or use `TimeSelection` so they point to the renamed alias consistently.
🪄 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: ba63596c-4a54-40ec-9893-e1f11abf2324
📒 Files selected for processing (6)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/layout/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsxpackages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsxpackages/timo-design-system/src/components/time/time-selector/TimeSelector.stories.tsxpackages/timo-design-system/src/components/time/time-selector/TimeSelector.tsx
- 아이템 선택 시 닫히는 기본 동작은 유지하되, closeOnSelect={false}로 개별 소비 컴포넌트가 이를 끌 수 있도록 옵션을 추가했습니다
- 빈도 선택 시 드롭다운이 닫히지 않고 계속 열려 있도록 closeOnSelect={false}를 적용했습니다
- 선택된 빈도를 내부 state로 관리해, 부모의 onFrequencyChange 연결 여부와 무관하게 클릭 즉시 라벨과 상세 섹션이 동기화되도록 고쳤습니다
- 피그마 디자인에 맞춰 트리거 버튼 위에 "반복 일정" 라벨을 추가했습니다
- outline-none으로 사라졌던 키보드 포커스 표시를 focus-visible:ring-2로 복구했습니다
…to feat/ui/83-repeat-selector
- 옵션 항목을 감싸던 div className="contents"를 Fragment로 교체해 불필요한 DOM 노드를 없앴습니다 - 스토리의 중복된 기본 props를 meta 레벨 args로 올리고, 각 스토리는 frequency만 오버라이드하도록 정리했습니다
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/repeat/repeat-selector/RepeatSelector.stories.tsx (1)
75-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
frequency하드코딩으로 Controls 패널의 select가 무력화돼요.
meta.argTypes.frequency에select컨트롤("daily" | "weekly" | "monthly")이 정의되어 있는데(Line 46-49),WeeklyDemo와MonthlyDemo의render에서frequency="weekly"/frequency="monthly"를 각각 하드코딩하고 있어요. 이 때문에 Storybook Controls 패널에서frequency값을 바꿔도 실제 렌더링에는 반영되지 않습니다.디자인 시스템 문서화 목적의 스토리이니만큼, Controls로 상태를 실험해보려는 다른 개발자가 혼란을 겪을 수 있어요.
args.frequency를 그대로 사용하거나, 해당 스토리에서는frequency컨트롤을argTypes: { frequency: { control: false } }로 비활성화해 의도를 명확히 하는 걸 추천합니다.♻️ 제안: args.frequency를 그대로 사용
const WeeklyDemo = (args: RepeatSelectorProps) => { const [selectedWeekdayIds, setSelectedWeekdayIds] = useState<string[]>([]); return ( <RepeatSelector {...args} - frequency="weekly" + frequency={args.frequency} weekly={{ ...args.weekly,const MonthlyDemo = (args: RepeatSelectorProps) => { const [repeatDay, setRepeatDay] = useState("3"); return ( <RepeatSelector {...args} - frequency="monthly" + frequency={args.frequency} monthly={{ ...args.monthly,관련 문서: Storybook - Args
Also applies to: 104-126
🤖 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/repeat/repeat-selector/RepeatSelector.stories.tsx` around lines 75 - 102, `WeeklyDemo`/`MonthlyDemo` are hardcoding `frequency`, which overrides the Storybook `frequency` control and makes `meta.argTypes.frequency` ineffective. Update the `render` logic in these story components to pass through `args.frequency` instead of forcing `"weekly"`/`"monthly"`, or explicitly disable the `frequency` control in the story’s `argTypes` if the value is meant to stay fixed. Keep the behavior consistent with `RepeatSelectorProps` so Controls changes are reflected in `RepeatSelector`.
🤖 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`:
- Line 108: `DropdownPanelProps` is currently declared as a type alias, but this
component should follow the project props convention by using an interface like
`DropdownItemProps`. Update the `DropdownPanelProps` declaration in
`Dropdown.tsx` to an interface that extends `HTMLAttributes<HTMLDivElement>` so
the naming and declaration style stay consistent with the rest of the design
system.
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsx`:
- Line 76: `WeeklyDemo` and `MonthlyDemo` are hardcoding local state defaults
instead of respecting the Storybook `args` defaults, so update the initial state
in `RepeatSelector.stories.tsx` to derive from the passed-in args/props rather
than fixed values. Use the existing `meta.args`-driven values for
`selectedWeekdayIds` and `repeatDay` when initializing the `useState` in those
demo components so the rendered story stays aligned with the Controls defaults
if they change later.
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx`:
- Around line 143-155: `RepeatSelector`의 `selectedFrequency`가
`useState(frequency)`로만 초기화되어 부모의 `frequency` 변경을 반영하지 못하는 상태입니다.
`RepeatSelector`와 `handleSelectFrequency`를 기준으로, 이 컴포넌트를 controlled로 유지할지 아니면
초기값 전용으로 둘지 정리한 뒤 수정하세요: controlled라면 `frequency` 변경 시 내부 상태가 따라가도록 동기화하고, 초기값
전용이라면 `frequency` 대신 `defaultFrequency`처럼 이름을 분리해 의도를 명확히 하세요.
---
Outside diff comments:
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsx`:
- Around line 75-102: `WeeklyDemo`/`MonthlyDemo` are hardcoding `frequency`,
which overrides the Storybook `frequency` control and makes
`meta.argTypes.frequency` ineffective. Update the `render` logic in these story
components to pass through `args.frequency` instead of forcing
`"weekly"`/`"monthly"`, or explicitly disable the `frequency` control in the
story’s `argTypes` if the value is meant to stay fixed. Keep the behavior
consistent with `RepeatSelectorProps` so Controls changes are reflected in
`RepeatSelector`.
🪄 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: 155afd10-ccff-48d8-9eb3-a41c718ad6cb
📒 Files selected for processing (5)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/layout/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsxpackages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsxpackages/timo-design-system/src/components/time/time-selector/TimeSelector.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/repeat/repeat-selector/RepeatSelector.stories.tsx (1)
75-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
frequency하드코딩으로 Controls 패널의 select가 무력화돼요.
meta.argTypes.frequency에select컨트롤("daily" | "weekly" | "monthly")이 정의되어 있는데(Line 46-49),WeeklyDemo와MonthlyDemo의render에서frequency="weekly"/frequency="monthly"를 각각 하드코딩하고 있어요. 이 때문에 Storybook Controls 패널에서frequency값을 바꿔도 실제 렌더링에는 반영되지 않습니다.디자인 시스템 문서화 목적의 스토리이니만큼, Controls로 상태를 실험해보려는 다른 개발자가 혼란을 겪을 수 있어요.
args.frequency를 그대로 사용하거나, 해당 스토리에서는frequency컨트롤을argTypes: { frequency: { control: false } }로 비활성화해 의도를 명확히 하는 걸 추천합니다.♻️ 제안: args.frequency를 그대로 사용
const WeeklyDemo = (args: RepeatSelectorProps) => { const [selectedWeekdayIds, setSelectedWeekdayIds] = useState<string[]>([]); return ( <RepeatSelector {...args} - frequency="weekly" + frequency={args.frequency} weekly={{ ...args.weekly,const MonthlyDemo = (args: RepeatSelectorProps) => { const [repeatDay, setRepeatDay] = useState("3"); return ( <RepeatSelector {...args} - frequency="monthly" + frequency={args.frequency} monthly={{ ...args.monthly,관련 문서: Storybook - Args
Also applies to: 104-126
🤖 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/repeat/repeat-selector/RepeatSelector.stories.tsx` around lines 75 - 102, `WeeklyDemo`/`MonthlyDemo` are hardcoding `frequency`, which overrides the Storybook `frequency` control and makes `meta.argTypes.frequency` ineffective. Update the `render` logic in these story components to pass through `args.frequency` instead of forcing `"weekly"`/`"monthly"`, or explicitly disable the `frequency` control in the story’s `argTypes` if the value is meant to stay fixed. Keep the behavior consistent with `RepeatSelectorProps` so Controls changes are reflected in `RepeatSelector`.
🤖 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`:
- Line 108: `DropdownPanelProps` is currently declared as a type alias, but this
component should follow the project props convention by using an interface like
`DropdownItemProps`. Update the `DropdownPanelProps` declaration in
`Dropdown.tsx` to an interface that extends `HTMLAttributes<HTMLDivElement>` so
the naming and declaration style stay consistent with the rest of the design
system.
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsx`:
- Line 76: `WeeklyDemo` and `MonthlyDemo` are hardcoding local state defaults
instead of respecting the Storybook `args` defaults, so update the initial state
in `RepeatSelector.stories.tsx` to derive from the passed-in args/props rather
than fixed values. Use the existing `meta.args`-driven values for
`selectedWeekdayIds` and `repeatDay` when initializing the `useState` in those
demo components so the rendered story stays aligned with the Controls defaults
if they change later.
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx`:
- Around line 143-155: `RepeatSelector`의 `selectedFrequency`가
`useState(frequency)`로만 초기화되어 부모의 `frequency` 변경을 반영하지 못하는 상태입니다.
`RepeatSelector`와 `handleSelectFrequency`를 기준으로, 이 컴포넌트를 controlled로 유지할지 아니면
초기값 전용으로 둘지 정리한 뒤 수정하세요: controlled라면 `frequency` 변경 시 내부 상태가 따라가도록 동기화하고, 초기값
전용이라면 `frequency` 대신 `defaultFrequency`처럼 이름을 분리해 의도를 명확히 하세요.
---
Outside diff comments:
In
`@packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsx`:
- Around line 75-102: `WeeklyDemo`/`MonthlyDemo` are hardcoding `frequency`,
which overrides the Storybook `frequency` control and makes
`meta.argTypes.frequency` ineffective. Update the `render` logic in these story
components to pass through `args.frequency` instead of forcing
`"weekly"`/`"monthly"`, or explicitly disable the `frequency` control in the
story’s `argTypes` if the value is meant to stay fixed. Keep the behavior
consistent with `RepeatSelectorProps` so Controls changes are reflected in
`RepeatSelector`.
🪄 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: 155afd10-ccff-48d8-9eb3-a41c718ad6cb
📒 Files selected for processing (5)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/layout/dropdown/Dropdown.tsxpackages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsxpackages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsxpackages/timo-design-system/src/components/time/time-selector/TimeSelector.tsx
🛑 Comments failed to post (3)
packages/timo-design-system/src/components/layout/dropdown/Dropdown.tsx (1)
108-108: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Props 타입은
interface로 선언해주세요! (feat. 옆자리DropdownItemProps는 이미 잘 하고 있어요 👍)
DropdownPanelProps가type별칭으로 선언되어 있는데, 바로 아래DropdownItemProps는interface extends ...패턴을 잘 따르고 있어요. 컨벤션 일관성을 위해 동일한 패턴으로 맞춰주시면 좋겠습니다.✏️ 제안 diff
-export type DropdownPanelProps = HTMLAttributes<HTMLDivElement>; +export interface DropdownPanelProps extends HTMLAttributes<HTMLDivElement> {}As per path instructions, "Props 타입은
interface로 선언하고 접미사Props사용 (예:ButtonProps)".📝 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.export interface DropdownPanelProps extends HTMLAttributes<HTMLDivElement> {}🤖 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` at line 108, `DropdownPanelProps` is currently declared as a type alias, but this component should follow the project props convention by using an interface like `DropdownItemProps`. Update the `DropdownPanelProps` declaration in `Dropdown.tsx` to an interface that extends `HTMLAttributes<HTMLDivElement>` so the naming and declaration style stay consistent with the rest of the design system.Source: Path instructions
packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsx (1)
76-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
로컬 state 초기값이
args의 기본값을 무시해요.
WeeklyDemo의selectedWeekdayIds(Line 76)와MonthlyDemo의repeatDay(Line 105)는 각각[],"3"으로 하드코딩되어 있어,meta.args에서 다른 기본값을 지정하더라도 실제로는 반영되지 않아요. 지금은 두 값이 우연히 일치할 가능성이 높지만, 나중에meta.args의 기본값이 바뀌면 스토리 동작과 컨트롤 표시값이 어긋날 수 있습니다.-const WeeklyDemo = (args: RepeatSelectorProps) => { - const [selectedWeekdayIds, setSelectedWeekdayIds] = useState<string[]>([]); +const WeeklyDemo = (args: RepeatSelectorProps) => { + const [selectedWeekdayIds, setSelectedWeekdayIds] = useState<string[]>( + args.weekly?.selectedWeekdayIds ?? [], + );-const MonthlyDemo = (args: RepeatSelectorProps) => { - const [repeatDay, setRepeatDay] = useState("3"); +const MonthlyDemo = (args: RepeatSelectorProps) => { + const [repeatDay, setRepeatDay] = useState(args.monthly?.repeatDay ?? "3");Also applies to: 105-105
🤖 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/repeat/repeat-selector/RepeatSelector.stories.tsx` at line 76, `WeeklyDemo` and `MonthlyDemo` are hardcoding local state defaults instead of respecting the Storybook `args` defaults, so update the initial state in `RepeatSelector.stories.tsx` to derive from the passed-in args/props rather than fixed values. Use the existing `meta.args`-driven values for `selectedWeekdayIds` and `repeatDay` when initializing the `useState` in those demo components so the rendered story stays aligned with the Controls defaults if they change later.packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx (1)
143-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the component and nearby usages fd -a "RepeatSelector.tsx|TimeSelector.tsx|*.stories.tsx|*.story.tsx" packages/timo-design-system src apps 2>/dev/null || true echo "=== Outline RepeatSelector ===" ast-grep outline packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx --view expanded || true echo "=== Read RepeatSelector (relevant slice) ===" nl -ba packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx | sed -n '1,260p' echo "=== Search for RepeatSelector usages ===" rg -n "RepeatSelector|frequency\s*=" packages/timo-design-system src apps --glob '!**/dist/**' --glob '!**/build/**' || true echo "=== Search for TimeSelector pattern ===" fd -a "TimeSelector.tsx" packages/timo-design-system src apps 2>/dev/null || trueRepository: Team-Timo/Timo-client
Length of output: 1636
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect selected props and state-handling patterns in the related component. ast-grep outline packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx --view expanded || true echo "-----" nl -ba packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx | sed -n '1,260p' echo "-----" rg -n "onFrequencyChange|selectedFrequency|useState<RepeatFrequency>|defaultFrequency|frequency" packages/timo-design-system/src/components/repeat -g '*.tsx' || trueRepository: Team-Timo/Timo-client
Length of output: 1565
🏁 Script executed:
#!/bin/bash set -euo pipefail # Check the actual component implementation and its story/demo usage. nl -ba packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx | sed -n '120,220p' echo "-----" rg -n "RepeatSelector|WeeklyDemo|MonthlyDemo|frequency" packages/timo-design-system -g '*.tsx' -g '*.ts' || trueRepository: Team-Timo/Timo-client
Length of output: 199
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the component and any related demos/usages. nl -ba packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx | sed -n '1,260p' echo "-----" rg -n "RepeatSelector|WeeklyDemo|MonthlyDemo|onFrequencyChange|selectedFrequency|frequency" packages/timo-design-system/src/components/repeat -g '*.tsx' -g '*.ts' || trueRepository: Team-Timo/Timo-client
Length of output: 199
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== RepeatSelector.tsx (lines 130-220) ===" sed -n '130,220p' packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx echo "=== RepeatSelector.tsx (full with line numbers via cat) ===" cat -n packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.tsx | sed -n '130,220p' echo "=== Search for RepeatSelector usages and demos ===" rg -n "RepeatSelector|WeeklyDemo|MonthlyDemo|defaultFrequency|onFrequencyChange|selectedFrequency|frequency" packages/timo-design-system -g '*.tsx' -g '*.ts' || true echo "=== Search for TimeSelector pattern ===" rg -n "TimeSelector|selected|on.*Change|useState<" packages/timo-design-system/src/components -g '*.tsx' || trueRepository: Team-Timo/Timo-client
Length of output: 22919
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== RepeatSelector.stories.tsx ===" sed -n '1,220p' packages/timo-design-system/src/components/repeat/repeat-selector/RepeatSelector.stories.tsx echo "=== TimeSelector.tsx ===" sed -n '1,220p' packages/timo-design-system/src/components/time/time-selector/TimeSelector.tsx echo "=== TimeSelector.stories.tsx ===" sed -n '1,220p' packages/timo-design-system/src/components/time/time-selector/TimeSelector.stories.tsxRepository: Team-Timo/Timo-client
Length of output: 7318
frequency를 초기값이 아니라 상태 원본으로 맞춰 주세요.
UI는 깔끔한데,useState(frequency)로 한 번만 읽고 끝나서 부모가frequency를 바꿔도 내부 선택값이 그대로 남습니다. controlled로 갈 거면useEffect로 동기화하고, 초기값 전용이면defaultFrequency처럼 이름을 분리하는 편이 더 명확해요. React의 props/state 패턴은 You Might Not Need an Effect도 같이 보면 좋습니다.🤖 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/repeat/repeat-selector/RepeatSelector.tsx` around lines 143 - 155, `RepeatSelector`의 `selectedFrequency`가 `useState(frequency)`로만 초기화되어 부모의 `frequency` 변경을 반영하지 못하는 상태입니다. `RepeatSelector`와 `handleSelectFrequency`를 기준으로, 이 컴포넌트를 controlled로 유지할지 아니면 초기값 전용으로 둘지 정리한 뒤 수정하세요: controlled라면 `frequency` 변경 시 내부 상태가 따라가도록 동기화하고, 초기값 전용이라면 `frequency` 대신 `defaultFrequency`처럼 이름을 분리해 의도를 명확히 하세요.
ehye1
left a comment
There was a problem hiding this comment.
기존의 dropdown 컴파운드 써서 깔끔하게 구현해주셨네요
수고하셨습니다!!
…to feat/ui/83-repeat-selector
- alias import(@components, @iCons, @lib)를 상대 경로 import로 변경했습니다
) - alias import(@components)를 상대 경로 import로 변경했습니다
jjangminii
left a comment
There was a problem hiding this comment.
전체적으로 꼼꼼하게 작성해주셔서 리뷰하기 편했어요-! 특히 Dropdown 합성 컴포넌트 패턴을 그대로 따라가면서 RepeatSelector/TimeSelector를 만들어주신 덕분에 나중에 다른 주기 타입이나 선택 UI가 추가돼도 확장하기 수월할 것 같아요 👍
사소한 코멘트 몇 개 남겨뒀어요! 확인해주세요-!
|
|
||
| return ( | ||
| <Dropdown.Item | ||
| key={index} |
There was a problem hiding this comment.
minute이 이미 유니크한 식별자니 key={minute}으로 변경하는건 어떤가요?
- outline-none으로 제거되어 있던 입력창 포커스 스타일을 focus-visible:ring-2로 대체했습니다 - input에 useId로 생성한 고유 id를 부여하고 sr-only label과 htmlFor로 연결했습니다
ISSUE 🔗
close #83
What is this PR? 🔍
디자인 시스템에 반복 일정 선택(
RepeatSelector)과 예상 시간 선택(TimeSelector) 드롭다운 컴포넌트를 추가했습니다.배경
Dropdown합성 컴포넌트(Trigger/Panel/Item)만 디자인 시스템에 있었고, 반복 주기·예상 시간을 고르는 UI는 없었습니다.Dropdown을 조합해RepeatSelector,TimeSelector를 만들고, 피그마 노드를 기준으로 세부 정렬을 맞췄습니다.RepeatSelector
isPicking상태로 "주기 선택 목록"과 "세부 설정" 화면을 토글하고, 주기를 선택하면isPicking을 false로 바꿔 세부 설정으로 대체합니다. 세부 설정은frequency에 따라RepeatWeeklyDetailSection(요일 체크박스,justify-between으로 라벨 좌측·체크박스 우측) 또는RepeatMonthlyDetailSection(반복일 입력 + 라벨)을 렌더링합니다. 두 섹션은RepeatSelector.tsx안에 비공개 컴포넌트로 응집시켰습니다 — 별도 파일로 분리했을 때 다른 곳에서 재사용되지 않는데도export가 필요해 캡슐화가 약해지고 import 순서 등 결합도만 늘어,Dropdown.tsx가 이미 쓰고 있는 "합성 컴포넌트를 한 파일에 비공개로 두는" 패턴을 따랐습니다. 피그마 노드(902:31696, monthly 변형)를 확인해 세부 설정 값 행이 항상 우측에 고정되도록items-end로 정렬했고, "세부 설정" 제목은 정렬값과 무관하게w-full로 고정해 제목까지 밀리지 않게 했습니다.RepeatFrequencyList,RepeatWeeklyDetailSection,RepeatMonthlyDetailSection)는RepeatSelector.tsx내부에서만 쓰이는 비공개 구현이라components/index.ts에는RepeatSelector만 공개했습니다.TimeSelector
Dropdown.Item)에서 시간을 선택하면onSelect로 값을 전달합니다.selected === "ai"일 때 배경색과 아이콘(AiWhiteIcon)이 바뀌도록 했습니다.Dropdown 공통 컴포넌트
Dropdown.Panel에 있던mt-1(트리거와 패널 사이 여백)을 제거했습니다.mt-1이 이 두 컴포넌트를 포함한 모든Dropdown소비 컴포넌트에 불필요한 간격을 만들고 있었습니다.To Reviewers
RepeatSelector의 세부 설정 섹션들을 별도 파일로 분리했다가 다시 한 파일로 합쳤습니다 — 다른 곳에서 개별로 재사용될 일이 없는 조각이라 판단해Dropdown.tsx와 같은 패턴(비공개 서브 컴포넌트 응집)으로 되돌렸는데, 이 판단이 맞는지 봐주시면 좋겠습니다.Dropdown.tsx의mt-1제거는PrioritySelector,TagSelector등 기존Dropdown소비 컴포넌트의 간격에도 영향을 주니, 다른 화면에서 레이아웃이 깨지지 않는지 함께 확인해 주세요.로직 자체는 실제 컨테이너 구현 시 연결해 둘 예정이니 UI 위주로 확인해 주시면 될 것 같아요! 컨테이너 자체는 apps/timo-web 내부에 둘 예정입니다.
Screenshot 📷
Test Checklist ✔
pnpm check-types통과pnpm lint통과pnpm build— 미실행: CI에서 확인 예정