Conversation
Walkthrough날짜/시간 입력용 DateTime 컴포넌트를 추가하고, 관련 유틸(DateTimeUtils), 스토리북(stories) 및 컴포넌트 인덱스 재익스포트를 도입했습니다. 컴포넌트는 date/time 모드, 포맷/유효성, 캐럿·삭제·붙여넣기 처리를 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant DT as DateTime 컴포넌트
participant U as DateTimeUtils
User->>DT: 입력/값 변경 (type=date 또는 time)
alt date flow
DT->>U: digitsOnly(value)
U-->>DT: 숫자 문자열
DT->>U: formatDate(digits)
U-->>DT: "YYYY.MM.DD"
DT-->>User: 포맷된 날짜 표시 (pattern 검사)
else time flow
DT->>U: digitsOnly(value/입력)
U-->>DT: up to 4 digits
DT->>U: formatTime12(digits)
U-->>DT: "오전/오후 HH[:MM]"
DT-->>User: 포맷된 시간 표시, 캐럿 위치 계산
DT->>U: mapCaretByDigitsPos(pos, 'time')
U-->>DT: 시각적 캐럿 위치
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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 |
|
✅ Storybook chromatic 배포 확인: |
![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
🧹 Nitpick comments (4)
packages/design-system/src/components/dateTime/DateTime.tsx (3)
171-194: IME 조합 입력 처리 보강 검토
onKeyDown에서isComposing은 확인하지만,onBeforeInput에서는 조합 중 여부를 고려하지 않습니다. 한글/IME 환경에서 의도치 않은 차단이 있을 수 있습니다.handleBeforeInput초반에 조합 중 이벤트를 무시하는 방어 로직을 추가하는 것을 권장합니다.
62-71: 날짜 유효성 검증 부재(월/일 범위, 윤년 등)현재는 숫자만 필터링하고 포맷팅만 수행합니다. 최소한 제출 가능한 완전한 8자리(
YYYYMMDD)가 되었을 때는 유효한 달/일인지 검증하고, 필요 시 보정 또는 에러 상태를 노출하세요(예: 월 13 → 12, 일 32 → 말일로 보정 또는 입력 차단). 구현 복잡도를 낮추려면Date객체를 활용해new Date(y, m-1, d)비교 방식으로 유효성 체크를 추천합니다.
49-57: 공개 API 관점: value는 초기값만 사용되고 외부로 변경사항이 노출되지 않음디자인 시스템 컴포넌트라면 제어형/비제어형 모두 지원하는 게 일반적입니다.
onValueChange(정규화된 값, 예:YYYY-MM-DD/HH:MM),value(제어형),defaultValue(비제어형) 패턴을 도입하는 것을 권장합니다.apps/client/src/App.tsx (1)
20-20: 데모 확장 제안(선택): date 모드/상태별 스냅샷도 함께 노출스토리북 도입 전까지는 App에서 간단히
type="date"나state="disabled"|"error"사례도 보여주면 수동 QA가 수월합니다.- <DateTime state="default" type="time" value="" /> + <DateTime state="default" type="time" value="" /> + <DateTime state="default" type="date" value="20250825" /> + <DateTime state="disabled" type="time" value="0930" />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/client/src/App.tsx(2 hunks)packages/design-system/src/components/dateTime/DateTime.tsx(1 hunks)packages/design-system/src/components/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/App.tsx (1)
packages/design-system/src/components/dateTime/DateTime.tsx (1)
DateTime(49-221)
🔇 Additional comments (2)
apps/client/src/App.tsx (1)
5-5: 디자인 시스템 통합 LGTM
@pinback/design-system/ui에서DateTime임포트하는 방식 적절합니다.packages/design-system/src/components/index.ts (1)
3-3: 배럴 리익스포트 LGTM
DateTime을 기본 익스포트로 노출하는 선택 괜찮습니다. App 사용처와도 일치합니다.
| @@ -0,0 +1,221 @@ | |||
| import { useRef, useState, type ChangeEvent } from 'react'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
커서 제어를 위해 useLayoutEffect 임포트 추가 필요
nextCaretRef에 커서 위치를 저장하지만 실제로 반영하는 이펙트가 없어서 커서가 기대한 위치로 이동하지 않습니다. 커서 반영 로직을 넣기 위해 useLayoutEffect를 임포트하세요.
-import { useRef, useState, type ChangeEvent } from 'react';
+import { useRef, useState, type ChangeEvent, useLayoutEffect } from 'react';📝 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.
| import { useRef, useState, type ChangeEvent } from 'react'; | |
| import { useRef, useState, type ChangeEvent, useLayoutEffect } from 'react'; |
🤖 Prompt for AI Agents
In packages/design-system/src/components/dateTime/DateTime.tsx around line 1,
the component stores the desired caret position in nextCaretRef but never
applies it to the input; import useLayoutEffect and add a layout effect that,
when nextCaretRef has a valid number and the input ref is available, sets the
input selectionStart and selectionEnd to that caret index and then clears
nextCaretRef (or resets it) so the cursor moves to the expected position after
renders/updates.
| export default function DateTime({ type, value = '', state }: DateTimeProps) { | ||
| const inputRef = useRef<HTMLInputElement>(null); | ||
| const nextCaretRef = useRef<number | null>(null); | ||
| const [timeDigits, setTimeDigits] = useState(() => | ||
| digitsOnly(value).slice(0, 4) | ||
| ); | ||
| const [input, setInput] = useState(() => | ||
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | ||
| ); | ||
| function digitsOnly(s: string) { | ||
| return s.replace(/\D/g, ''); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
커서 위치가 저장만 되고 적용되지 않음 — 렌더 후 selectionRange 반영 필요
nextCaretRef를 여러 핸들러에서 설정하지만 실제 DOM 커서로 적용하는 로직이 없습니다. 입력 UX가 무너집니다. input 상태 변경 이후 useLayoutEffect로 커서를 반영하세요.
function digitsOnly(s: string) {
return s.replace(/\D/g, '');
}
+
+ // 입력 문자열 갱신 후 커서 위치를 반영
+ useLayoutEffect(() => {
+ if (!inputRef.current) return;
+ if (nextCaretRef.current != null) {
+ const pos = nextCaretRef.current;
+ inputRef.current.setSelectionRange(pos, pos);
+ nextCaretRef.current = null;
+ }
+ }, [input]);📝 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 default function DateTime({ type, value = '', state }: DateTimeProps) { | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const nextCaretRef = useRef<number | null>(null); | |
| const [timeDigits, setTimeDigits] = useState(() => | |
| digitsOnly(value).slice(0, 4) | |
| ); | |
| const [input, setInput] = useState(() => | |
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | |
| ); | |
| function digitsOnly(s: string) { | |
| return s.replace(/\D/g, ''); | |
| } | |
| export default function DateTime({ type, value = '', state }: DateTimeProps) { | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const nextCaretRef = useRef<number | null>(null); | |
| const [timeDigits, setTimeDigits] = useState(() => | |
| digitsOnly(value).slice(0, 4) | |
| ); | |
| const [input, setInput] = useState(() => | |
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | |
| ); | |
| function digitsOnly(s: string) { | |
| return s.replace(/\D/g, ''); | |
| } | |
| // 입력 문자열 갱신 후 커서 위치를 반영 | |
| useLayoutEffect(() => { | |
| if (!inputRef.current) return; | |
| if (nextCaretRef.current != null) { | |
| const pos = nextCaretRef.current; | |
| inputRef.current.setSelectionRange(pos, pos); | |
| nextCaretRef.current = null; | |
| } | |
| }, [input]); | |
| // …the rest of your handlers and JSX… | |
| } |
🤖 Prompt for AI Agents
In packages/design-system/src/components/dateTime/DateTime.tsx around lines 49
to 61, nextCaretRef is being written in handlers but never applied to the DOM
caret; add a useLayoutEffect that runs after input state updates and, if
inputRef.current and nextCaretRef.current are set, calls
inputRef.current.setSelectionRange(nextCaretRef.current, nextCaretRef.current)
(and inputRef.current.focus() if needed), then clears nextCaretRef.current to
null; ensure the effect depends on the input state (and/or timeDigits/type as
appropriate) so the saved caret is applied immediately after render.
| function formatTime12(digits: string) { | ||
| const v = digits.slice(0, 4); | ||
| if (v.length === 0) return ''; | ||
|
|
||
| const hhDigits = v.slice(0, 2); | ||
| const mmDigits = v.slice(2, 4); | ||
| const hour24 = parseInt(hhDigits || '0', 10); | ||
| const ampm = hour24 >= 12 ? '오후' : '오전'; | ||
|
|
||
| let h12: number; | ||
| if (hhDigits.length < 2) { | ||
| h12 = parseInt(hhDigits || '0', 10) % 12; | ||
| if (h12 === 0) h12 = 12; | ||
| } else { | ||
| h12 = hour24 % 12; | ||
| if (h12 === 0) h12 = 12; | ||
| } | ||
| const displayHour = String(h12).padStart(2, '0'); | ||
|
|
||
| let out = `${ampm} ${displayHour}`; | ||
| if (mmDigits.length > 0) out += `:${mmDigits}`; | ||
| return out; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
시간 포맷은 있으나 범위 검증(00–23/00–59)이 없음
formatTime12는 표시만 담당하고 입력값 검증은 없습니다. 현재는 29:99 같은 값도 허용됩니다. PR 목적(유효성 검증)에 맞춰 시간 범위 검사를 추가하세요. 아래 헬퍼와 함께 사용하면 증분 입력에서도 즉시 차단할 수 있습니다.
function formatTime12(digits: string) {
const v = digits.slice(0, 4);
if (v.length === 0) return '';
@@
if (mmDigits.length > 0) out += `:${mmDigits}`;
return out;
}
+
+ // 0~23 시, 0~59 분 검사 (증분 입력 허용)
+ function isValidTimeDigits(d: string) {
+ const v = d.slice(0, 4);
+ if (v.length === 0) return true;
+ const hh = parseInt(v.slice(0, 2) || '0', 10);
+ if (Number.isNaN(hh) || hh > 23) return false;
+ if (v.length >= 3) {
+ const mm = parseInt(v.slice(2, 4) || '0', 10);
+ if (Number.isNaN(mm) || mm > 59) return false;
+ }
+ return true;
+ }📝 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.
| function formatTime12(digits: string) { | |
| const v = digits.slice(0, 4); | |
| if (v.length === 0) return ''; | |
| const hhDigits = v.slice(0, 2); | |
| const mmDigits = v.slice(2, 4); | |
| const hour24 = parseInt(hhDigits || '0', 10); | |
| const ampm = hour24 >= 12 ? '오후' : '오전'; | |
| let h12: number; | |
| if (hhDigits.length < 2) { | |
| h12 = parseInt(hhDigits || '0', 10) % 12; | |
| if (h12 === 0) h12 = 12; | |
| } else { | |
| h12 = hour24 % 12; | |
| if (h12 === 0) h12 = 12; | |
| } | |
| const displayHour = String(h12).padStart(2, '0'); | |
| let out = `${ampm} ${displayHour}`; | |
| if (mmDigits.length > 0) out += `:${mmDigits}`; | |
| return out; | |
| } | |
| function formatTime12(digits: string) { | |
| const v = digits.slice(0, 4); | |
| if (v.length === 0) return ''; | |
| const hhDigits = v.slice(0, 2); | |
| const mmDigits = v.slice(2, 4); | |
| const hour24 = parseInt(hhDigits || '0', 10); | |
| const ampm = hour24 >= 12 ? '오후' : '오전'; | |
| let h12: number; | |
| if (hhDigits.length < 2) { | |
| h12 = parseInt(hhDigits || '0', 10) % 12; | |
| if (h12 === 0) h12 = 12; | |
| } else { | |
| h12 = hour24 % 12; | |
| if (h12 === 0) h12 = 12; | |
| } | |
| const displayHour = String(h12).padStart(2, '0'); | |
| let out = `${ampm} ${displayHour}`; | |
| if (mmDigits.length > 0) out += `:${mmDigits}`; | |
| return out; | |
| } | |
| // 0~23 시, 0~59 분 검사 (증분 입력 허용) | |
| function isValidTimeDigits(d: string) { | |
| const v = d.slice(0, 4); | |
| if (v.length === 0) return true; | |
| const hh = parseInt(v.slice(0, 2) || '0', 10); | |
| if (Number.isNaN(hh) || hh > 23) return false; | |
| if (v.length >= 3) { | |
| const mm = parseInt(v.slice(2, 4) || '0', 10); | |
| if (Number.isNaN(mm) || mm > 59) return false; | |
| } | |
| return true; | |
| } |
| const pasted = digitsOnly((ne as InputEvent).data ?? ''); | ||
| if (pasted) { | ||
| const next = (timeDigits + pasted).slice(0, 4); | ||
| setTimeDigits(next); | ||
| setInput(formatTime12(next)); | ||
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | ||
| e.preventDefault(); | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
붙여넣기 처리: 브라우저 호환성과 유효성 검사 추가
InputEvent.data가 항상 존재하지 않습니다(Safari 등). clipboardData/DataTransfer 폴백과 범위 검증을 추가하세요.
- const pasted = digitsOnly((ne as InputEvent).data ?? '');
+ const dt = (ne as any).dataTransfer ?? (ne as any).clipboardData ?? null;
+ const pastedRaw =
+ (typeof (ne as any).data === 'string' ? (ne as any).data : '') ||
+ (dt ? dt.getData('text') : '');
+ const pasted = digitsOnly(pastedRaw);
if (pasted) {
- const next = (timeDigits + pasted).slice(0, 4);
+ const next = (timeDigits + pasted).slice(0, 4);
+ if (!isValidTimeDigits(next)) {
+ e.preventDefault();
+ return;
+ }
setTimeDigits(next);
setInput(formatTime12(next));
nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
e.preventDefault();
} else {
e.preventDefault();
}📝 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 pasted = digitsOnly((ne as InputEvent).data ?? ''); | |
| if (pasted) { | |
| const next = (timeDigits + pasted).slice(0, 4); | |
| setTimeDigits(next); | |
| setInput(formatTime12(next)); | |
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | |
| e.preventDefault(); | |
| } else { | |
| const dt = (ne as any).dataTransfer ?? (ne as any).clipboardData ?? null; | |
| const pastedRaw = | |
| (typeof (ne as any).data === 'string' ? (ne as any).data : '') || | |
| (dt ? dt.getData('text') : ''); | |
| const pasted = digitsOnly(pastedRaw); | |
| if (pasted) { | |
| const next = (timeDigits + pasted).slice(0, 4); | |
| if (!isValidTimeDigits(next)) { | |
| e.preventDefault(); | |
| return; | |
| } | |
| setTimeDigits(next); | |
| setInput(formatTime12(next)); | |
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | |
| e.preventDefault(); | |
| } else { | |
| e.preventDefault(); | |
| } |
🤖 Prompt for AI Agents
In packages/design-system/src/components/dateTime/DateTime.tsx around lines 149
to 156, the paste handling assumes InputEvent.data is always available (not true
in Safari); update the handler to fall back to clipboardData/DataTransfer: read
pastedText = (e as ClipboardEvent).clipboardData?.getData('text') ?? (ne as
InputEvent).data ?? '', normalize it with digitsOnly and trim, validate the
result contains digits and has length > 0, then build next = (timeDigits +
pastedDigits).slice(0,4), call setTimeDigits(next),
setInput(formatTime12(next)), update nextCaretRef.current =
mapCaretByDigitsPos(next.length, 'time'), and call e.preventDefault(); if
pastedText is empty or contains no digits, do nothing (allow default) or return
early. Ensure you only access clipboardData on the event types that provide it.
| <input | ||
| ref={inputRef} | ||
| type="text" | ||
| className={dateTimeTxtStyles({ state })} | ||
| value={input} | ||
| onBeforeInput={handleBeforeInput} | ||
| onKeyDown={handleKeyDown} | ||
| onChange={handleChange} | ||
| placeholder={type === 'date' ? 'YYYY.MM.DD' : '오후 HH:MM'} | ||
| inputMode="numeric" | ||
| maxLength={type === 'date' ? 10 : 8} | ||
| pattern={ | ||
| type === 'date' | ||
| ? '\\d{4}\\.\\d{2}\\.\\d{2}' | ||
| : '(오전|오후)\\s\\d{2}:\\d{2}' | ||
| } | ||
| aria-label={type === 'date' ? '날짜 입력' : '시간 입력'} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성/상태 일관성: 비활성화 속성 및 패턴 정규식 불일치 수정
- state가
disabled여도<input>은 실제로 활성화되어 있습니다.disabled/aria-disabled를 설정해야 합니다. - time 패턴은 분이 필수(
...:MM)로 되어 있으나, 현재 UI는 분 없이오전 HH상태도 허용합니다. 정규식을 고쳐 부분 입력을 허용하거나, 최소 제출 시점에서만 전체 형식을 강제하세요. - 에러 상태 시
aria-invalid를 설정하세요.
<input
ref={inputRef}
type="text"
className={dateTimeTxtStyles({ state })}
value={input}
onBeforeInput={handleBeforeInput}
onKeyDown={handleKeyDown}
onChange={handleChange}
placeholder={type === 'date' ? 'YYYY.MM.DD' : '오후 HH:MM'}
inputMode="numeric"
- maxLength={type === 'date' ? 10 : 8}
+ maxLength={type === 'date' ? 10 : 8}
+ disabled={state === 'disabled'}
+ aria-disabled={state === 'disabled'}
+ aria-invalid={state === 'error' ? 'true' : 'false'}
+ autoComplete="off"
pattern={
type === 'date'
- ? '\\d{4}\\.\\d{2}\\.\\d{2}'
- : '(오전|오후)\\s\\d{2}:\\d{2}'
+ ? '^\\d{4}\\.\\d{2}\\.\\d{2}$'
+ : '^(오전|오후)\\s\\d{2}(:\\d{2})?$'
}
aria-label={type === 'date' ? '날짜 입력' : '시간 입력'}
/>📝 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.
| <input | |
| ref={inputRef} | |
| type="text" | |
| className={dateTimeTxtStyles({ state })} | |
| value={input} | |
| onBeforeInput={handleBeforeInput} | |
| onKeyDown={handleKeyDown} | |
| onChange={handleChange} | |
| placeholder={type === 'date' ? 'YYYY.MM.DD' : '오후 HH:MM'} | |
| inputMode="numeric" | |
| maxLength={type === 'date' ? 10 : 8} | |
| pattern={ | |
| type === 'date' | |
| ? '\\d{4}\\.\\d{2}\\.\\d{2}' | |
| : '(오전|오후)\\s\\d{2}:\\d{2}' | |
| } | |
| aria-label={type === 'date' ? '날짜 입력' : '시간 입력'} | |
| /> | |
| <input | |
| ref={inputRef} | |
| type="text" | |
| className={dateTimeTxtStyles({ state })} | |
| value={input} | |
| onBeforeInput={handleBeforeInput} | |
| onKeyDown={handleKeyDown} | |
| onChange={handleChange} | |
| placeholder={type === 'date' ? 'YYYY.MM.DD' : '오후 HH:MM'} | |
| inputMode="numeric" | |
| maxLength={type === 'date' ? 10 : 8} | |
| disabled={state === 'disabled'} | |
| aria-disabled={state === 'disabled'} | |
| aria-invalid={state === 'error' ? 'true' : 'false'} | |
| autoComplete="off" | |
| pattern={ | |
| type === 'date' | |
| ? '^\\d{4}\\.\\d{2}\\.\\d{2}$' | |
| : '^(오전|오후)\\s\\d{2}(:\\d{2})?$' | |
| } | |
| aria-label={type === 'date' ? '날짜 입력' : '시간 입력'} | |
| /> |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (4)
6-6: 스토리 그룹 네이밍 일관화 제안("UI" → "Components")Button 스토리(title: "Components/Button")와 그룹명이 달라 탐색성이 떨어집니다. 디자인 시스템 전반에서 동일한 루트 그룹을 쓰도록 "Components/DateTime"으로 맞추는 것을 추천합니다.
- title: 'UI/DateTime', + title: 'Components/DateTime',
58-76: play 함수에 검증(assert) 추가로 회귀 방지현재 Time_Typing_1620는 입력만 시연하고 기대 결과를 확인하지 않습니다. 기대 출력("오후 04:20")을 명시적으로 검증하면 회귀를 방지할 수 있습니다. 또한 expect를 import에 추가하세요.
- import { within, userEvent } from '@storybook/test'; + import { within, userEvent, expect } from '@storybook/test'; @@ play: async ({ canvasElement }) => { const canvas = within(canvasElement); const input = await canvas.findByLabelText('시간 입력'); - await userEvent.type(input, '1620'); + await userEvent.type(input, '1620'); + await expect(input).toHaveDisplayValue('오후 04:20'); },Also applies to: 1-2
78-95: Backspace 시나리오도 결과 검증 추가삭제 두 번 후 "오후 04"가 표시되는지 확인하는 단언을 추가하면 상호작용 로직이 깨졌을 때 빠르게 감지할 수 있습니다.
play: async ({ canvasElement }) => { const canvas = within(canvasElement); const input = await canvas.findByLabelText('시간 입력'); await userEvent.click(input); await userEvent.keyboard('{Backspace}{Backspace}'); + await expect(input).toHaveDisplayValue('오후 04'); },
51-56: 엣지 케이스 스토리 제안(유효성 검증 목적)요구사항(#21)에 "각 타입 검증 로직"이 포함되어 있습니다. 시간의 경우 불가능한 입력(예: '2960', '2360')에 대한 처리/거부/클램핑 동작을 스토리로 추가해두면 명세가 명확해집니다.
DateTime.tsx에 간단한 검증을 추가하려면 onBeforeInput에서 다음과 같이 시간 범위를 확인하세요:
if (inputType.startsWith('insert') && data) { const add = data.replace(/\D/g, ''); if (!add) { e.preventDefault(); return; } if (timeDigits.length >= 4) { e.preventDefault(); return; } - const next = (timeDigits + add).slice(0, 4); + const next = (timeDigits + add).slice(0, 4); + // 시간 범위 검증(부분 입력 고려) + if (next.length >= 2) { + const hh = parseInt(next.slice(0, 2), 10); + if (hh > 23) { e.preventDefault(); return; } + } + if (next.length === 4) { + const mm = parseInt(next.slice(2, 4), 10); + if (mm > 59) { e.preventDefault(); return; } + } setTimeDigits(next); setInput(formatTime12(next)); nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); e.preventDefault(); return; }packages/design-system/src/components/button/Button.tsx (5)
4-8: isDisabled vs 원래 disabled 속성 중복: API 단순화 제안ButtonHTMLAttributes에 이미 disabled가 있어 isDisabled는 중복 개념입니다. 현재 구현은 {...props}가 마지막에 와서 props.disabled가 isDisabled를 덮어씁니다(혼동 여지). 단일 진실 소스만 두는 편이 명확합니다.
두 가지 방향 중 하나를 권장합니다.
- 옵션 A(권장): isDisabled 제거, 표준 disabled만 사용
- 옵션 B: isDisabled를 유지하되 disabled를 props에서 제거해 타입 상충 방지
옵션 A 예시:
-interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { - variant?: 'primary' | 'secondary'; - size?: 'small' | 'medium' | 'large'; - isDisabled?: boolean; -} +interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { + variant?: 'primary' | 'secondary'; + size?: 'small' | 'medium' | 'large'; +} @@ - isDisabled = false, + disabled, ...props }: ButtonProps) => { return ( <button type="button" - className={cn(buttonVariants({ variant, size }), className)} - disabled={isDisabled} - {...props} + className={cn(buttonVariants({ variant, size }), className)} + disabled={disabled} + {...props} >Also applies to: 42-47
10-12: 접근성: focus-visible 스타일 추가 추천키보드 포커스 표시가 없어 접근성 문제가 생길 수 있습니다. 기본 variant에 focus-visible outline/ring을 추가하세요.
-const buttonVariants = cva( - 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed w-full', +const buttonVariants = cva( + 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed w-full focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary',또는 variant별로 더 구체적인 focus 스타일을 제공해도 좋습니다.
Also applies to: 15-24
11-12: 기본 w-full 강제 여부 확인디자인 시스템의 기본 버튼이 항상 가득 차는 폭(w-full)인지 확인이 필요합니다. 일반적으로 기본은 content-width, 옵션으로 fullWidth를 둡니다.
- 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed w-full', + 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed',필요 시 width: 'auto' | 'full' variant를 추가해 선택적으로 확장하세요.
4-4: React 타입 네임스페이스 사용 시 타입 import 명시TS 설정에 따라 React 네임스페이스 타입을 직접 참조할 때 import가 필요할 수 있습니다. 컴파일러 환경이 바뀌어도 안전하게 가려면 다음을 추가하는 것이 좋습니다.
import type * as React from 'react';
15-19: secondary outline 스타일 확인secondary에 outline만 적용되어 hover/active에서 대비가 충분한지(특히 다크/라이트 테마) 확인 바랍니다. 필요 시 focus/hover 시 outline 색상 강화 또는 배경 대비를 높이세요.
packages/design-system/src/components/button/Button.stories.tsx (2)
5-5: 스토리 타이틀 정리: DateTime와 동일한 그룹 사용DateTime 쪽을 "Components/DateTime"으로 정리한다면 본 타이틀도 그와 일관되므로 OK입니다. 전체 스토리북 내 그룹 네이밍을 일괄 정리해 주세요.
100-108: alert 사용 대신 action/fn 사용 권장테스트 러너/Docs 환경에서 alert는 흐름을 중단(blocking)시킬 수 있습니다. action('clicked') 또는 fn()을 사용해 비차단적으로 확인하세요.
-export const WithAlertOnClick: Story = { +export const WithAlertOnClick: Story = { name: 'onClick with Alert', args: { - children: 'onClick test', - onClick: () => { - alert('버튼이 클릭되었습니다.'); - }, + children: 'onClick test', + onClick: () => { + // Storybook actions 패널에서 확인 가능 + console.log('버튼이 클릭되었습니다.'); + }, }, };또는 '@storybook/test'의 fn을 사용해 호출 횟수를 단언하는 play를 추가할 수도 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/client/src/App.tsx(0 hunks)packages/design-system/src/components/button/Button.stories.tsx(1 hunks)packages/design-system/src/components/button/Button.tsx(1 hunks)packages/design-system/src/components/dateTime/DateTime.stories.tsx(1 hunks)packages/design-system/src/components/dateTime/DateTime.tsx(1 hunks)packages/design-system/src/components/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/client/src/App.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/design-system/src/components/index.ts
- packages/design-system/src/components/dateTime/DateTime.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T11:47:27.279Z
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#30
File: apps/extension/src/components/InfoBox/InfoBox.tsx:10-11
Timestamp: 2025-07-08T11:47:27.279Z
Learning: In React TypeScript components, prefer semantic prop names with union types over abstract version numbers. For example, use `size: 'small' | 'large'` instead of `version: 1 | 2` for better code readability, type safety, and extensibility.
Applied to files:
packages/design-system/src/components/button/Button.tsx
🧬 Code graph analysis (2)
packages/design-system/src/components/button/Button.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (1)
packages/design-system/src/components/dateTime/DateTime.tsx (1)
DateTime(49-223)
🔇 Additional comments (6)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (2)
1-2: Storybook 타입 import 확인(@storybook/react-vite vs @storybook/react)프로젝트의 Storybook 버전에 따라 권장 타입 import 소스가 다릅니다. v7/v8에서는 보통 '@storybook/react'에서 Meta/StoryObj를 가져옵니다. 현재 '@storybook/react-vite' 사용이 의도된 것인지 확인 부탁드립니다. 전역적으로 일관된 import 경로를 사용하세요.
43-49: 날짜 기본 스토리 값 정상 동작 확인 요청'20250819' → '2025.08.19' 포맷을 기대한다면 Storybook 상에서 실제 렌더 값이 일치하는지 한 번 확인 부탁드립니다. 만약 다른 구분자(예: '/')가 요구된다면 문서 설명도 함께 갱신하세요.
packages/design-system/src/components/button/Button.tsx (1)
1-1: cn import 경로 확인'@/lib'에서 cn이 재노출되는지(예: packages/design-system/src/lib/index.ts)가 확실하지 않습니다. 현재 barrel이 없다면 '@/lib/utils'에서 직접 import하거나 barrel을 추가하세요. 경로 alias가 앱/패키지 모두에서 동일하게 해석되는지도 한 번 점검 부탁드립니다.
packages/design-system/src/components/button/Button.stories.tsx (3)
8-16: 스토리 전역 파라미터와 문서 설명 좋습니다레이아웃/설명 구성이 명확합니다. 컴포넌트 사용 방법이 바로 드러나 테스트/도입 시 학습 비용이 낮습니다.
17-37: argTypes/기본 args 구성 적절variant/size/isDisabled/children/onClick 구성이 새 API와 일치합니다. className 테이블 비노출 처리도 적절합니다.
Also applies to: 39-45
83-98: Disabled 스토리: 상호 비교 렌더 👍동일 args로 variant만 바꿔 비교되도록 한 점 좋습니다. 접근성 애드온(a11y) 사용 중이라면 disabled 상태 대비와 탭 포커스 이동도 함께 확인 부탁드립니다.
| parameters: { | ||
| docs: { | ||
| description: { | ||
| component: | ||
| '날짜는 실시간 포맷(YYYY.MM.DD), 시간은 **숫자 버퍼(HHMM)** → 표시(오전/오후 HH:MM)로 렌더. 커서/백스페이스도 커스텀 처리됨.', | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
문서 설명(커서/백스페이스 커스텀)과 구현 불일치 가능성: caret 적용 누락 및 키 이벤트 중복 처리
설명에는 커서/백스페이스 커스텀 처리라고 되어 있으나, DateTime.tsx를 보면 nextCaretRef를 계산만 하고 실제 selectionRange를 적용하는 useEffect가 보이지 않습니다. 또한 Backspace/Delete 처리가 onBeforeInput와 onKeyDown 양쪽에 있어 브라우저/환경에 따라 한 번의 키 입력에 두 번 삭제될 위험이 있습니다(키 이벤트 순서: keydown → beforeinput). caret 적용과 키 처리 중복 제거가 필요합니다.
DateTime.tsx에 다음 보완을 제안드립니다.
- caret 적용 useEffect 추가:
+ useEffect(() => {
+ if (!inputRef.current) return;
+ const caret = nextCaretRef.current;
+ if (caret != null) {
+ inputRef.current.setSelectionRange(caret, caret);
+ nextCaretRef.current = null;
+ }
+ }, [input]);- Backspace/Delete는 onBeforeInput에서만 처리하고, onKeyDown에서는 조기에 반환:
const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
if (type !== 'time') return;
if (e.nativeEvent.isComposing) return;
-
- if (e.key === 'Backspace') {
- if (timeDigits.length > 0) {
- const next = timeDigits.slice(0, -1);
- setTimeDigits(next);
- setInput(formatTime12(next));
- nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
- }
- e.preventDefault();
- }
-
- if (e.key === 'Delete') {
- if (timeDigits.length > 0) {
- const next = timeDigits.slice(0, -1);
- setTimeDigits(next);
- setInput(formatTime12(next));
- nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
- }
- e.preventDefault();
- }
+ if (e.key === 'Backspace' || e.key === 'Delete') {
+ // 삭제 동작은 onBeforeInput(deleteContentBackward/Forward)에서만 처리
+ e.preventDefault();
+ return;
+ }
};원하시면 제가 PR에 바로 적용 가능한 패치로 묶어드릴게요.
🤖 Prompt for AI Agents
packages/design-system/src/components/dateTime/DateTime.stories.tsx around lines
30 to 37: the story text claims custom caret/backspace handling but DateTime.tsx
currently only computes nextCaretRef without ever applying selectionRange and
handles Backspace/Delete in both onBeforeInput and onKeyDown which can cause
double deletions; add a useEffect that watches nextCaretRef/currentInputRef and,
when nextCaretRef is set, calls setSelectionRange(start,end) (and then clears
nextCaretRef) to actually apply caret moves, and refactor key handling so
Backspace/Delete logic runs only in onBeforeInput while onKeyDown returns early
for those keys to avoid duplicate handling. Ensure the useEffect cleans up and
guards for input element existence and readonly/disabled states.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (23)
stories/page.css (3)
24-26: 링크 가시성/접근성 개선: 기본 밑줄 추가 제안링크 색상을 부모로부터 상속하면 본문 텍스트와 구분이 약해질 수 있습니다. 기본 밑줄과 hover 두께를 추가하면 스토리 문서에서 링크 인지성이 좋아집니다.
.storybook-page a { - color: inherit; + color: inherit; + text-decoration: underline; + text-underline-offset: 2px; } + +.storybook-page a:hover { + text-decoration-thickness: 2px; +}
1-9: 타이포 스케일의 유연성 확보(rem/단위 없는 line-height)픽셀 고정값 대신 rem과 단위 없는 line-height를 사용하면 사용자 기본 글꼴 설정/줌에 더 잘 대응합니다. 스토리 전용 스타일이라 영향도는 낮지만, 디자인 시스템 문서 품질 측면에서 권장합니다.
.storybook-page { margin: 0 auto; - padding: 48px 20px; - max-width: 600px; + padding: 3rem 1.25rem; + max-width: 37.5rem; color: #333; - font-size: 14px; - line-height: 24px; + font-size: 0.875rem; + line-height: 1.7; font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif; }
37-48: 배지 텍스트 크기 11px은 가독성이 낮을 수 있음11px은 일부 환경에서 가독성이 떨어질 수 있습니다. 12px(0.75rem) 이상을 권장합니다. 색 대비(배경 #e7fdd8, 전경 #357a14)는 대략 5.45:1로 WCAG AA(일반 텍스트) 기준을 만족하므로 색상은 유지 가능해 보입니다.
.storybook-page .tip { display: inline-block; vertical-align: top; margin-right: 10px; border-radius: 1em; background: #e7fdd8; padding: 4px 12px; color: #357a14; font-weight: 700; - font-size: 11px; + font-size: 0.75rem; /* ≈12px */ line-height: 12px; }stories/Page.tsx (3)
6-8: User 타입 중복 정의 방지Header.tsx에도 동일한 User 타입이 존재합니다. stories/types.ts 등으로 분리하여 양쪽에서 import하면 타입 드리프트를 방지할 수 있습니다.
추가 파일 제안(새 파일):
// stories/types.ts export type User = { name: string };변경:
- type User = { - name: string; - }; + import type { User } from './types';
59-67: 장식용 SVG는 스크린리더에서 숨김 처리해당 아이콘은 설명 텍스트를 보조하는 장식 요소이므로, 접근성 도구에 노출하지 않는 편이 좋습니다.
- <svg width="10" height="10" viewBox="0 0 12 12" xmlns="http://www.w3.org/2000/svg"> + <svg + width="10" + height="10" + viewBox="0 0 12 12" + xmlns="http://www.w3.org/2000/svg" + aria-hidden="true" + focusable="false" + >
10-13: React.FC 사용은 불필요함 — 기본 함수 선언으로 단순화 가능children을 사용하지 않으며, 제네릭 타입 인자가 없는 경우 React.FC를 쓰지 않아도 타입 추론이 충분합니다.
-export const Page: React.FC = () => { +export function Page() { const [user, setUser] = React.useState<User>(); … -}; +}stories/Header.tsx (2)
21-37: 헤더 로고 SVG 접근성: 장식 요소는 숨기기로고 옆에 텍스트 h1이 있어 의미 전달이 충분합니다. 로고 SVG는 장식이므로 스크린리더 제외를 권장합니다.
- <svg width="32" height="32" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg"> + <svg + width="32" + height="32" + viewBox="0 0 32 32" + xmlns="http://www.w3.org/2000/svg" + aria-hidden="true" + focusable="false" + >
6-15: User 타입/Props 정합성: 공통 타입으로 통합Page.tsx와 동일한 User 정의가 중복됩니다. stories/types.ts 등으로 분리해 import하면 유지보수성이 개선됩니다.
추가 파일 제안(새 파일):
// stories/types.ts export type User = { name: string };변경 예:
-type User = { - name: string; -}; +import type { User } from './types';stories/Page.stories.ts (2)
23-32: play 테스트 안정성 향상: DOM 변경 대기 후 검증클릭 후 바로 getByRole로 탐색하면 비동기 상태 전환 타이밍에 따라 간헐 실패할 수 있습니다. findByRole/await와 queryByRole를 조합해 안정화하세요.
export const LoggedIn: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); - const loginButton = canvas.getByRole('button', { name: /Log in/i }); - await expect(loginButton).toBeInTheDocument(); - await userEvent.click(loginButton); - await expect(loginButton).not.toBeInTheDocument(); - - const logoutButton = canvas.getByRole('button', { name: /Log out/i }); - await expect(logoutButton).toBeInTheDocument(); + const loginButton = await canvas.findByRole('button', { name: /Log in/i }); + await userEvent.click(loginButton); + await expect(canvas.queryByRole('button', { name: /Log in/i })).not.toBeInTheDocument(); + const logoutButton = await canvas.findByRole('button', { name: /Log out/i }); + await expect(logoutButton).toBeInTheDocument(); }, };
7-14: PR 목적과 코드 변경 간 불일치 확인 요청PR/이슈(#21)는 “날짜/시간 입력 공통 컴포넌트 및 검증” 구현을 목표로 하나, 본 PR에서 확인 가능한 변경은 Storybook 예제 컴포넌트/스토리 위주입니다. Date/Time Input 컴포넌트, 유효성 검사 로직, 해당 스토리/테스트가 보이지 않습니다. 스코프를 정리해 주시거나, 관련 파일 추가를 부탁드립니다. 필요 시 스켈레톤 구현과 유효성 유틸/스토리까지 초안 제공 가능합니다.
vitest.shims.d.ts (1)
1-1: 테스트 스코프로 한정하거나 tsconfig의 types로 옮기는 것을 고려해주세요.전역 ambient 선언 파일로 삼중 슬래시 참조를 두면, IDE/빌드 전반에 Playwright 타입이 누수될 수 있습니다. 테스트 전용으로 제한하려면 다음 중 하나를 권장합니다:
- vitest 전용 tsconfig(예: tsconfig.vitest.json)의
compilerOptions.types에 추가- 또는 현재 파일이 테스트 전용 tsconfig의
include에만 들어가도록 구성테스트 전용 tsconfig 예시:
{ "extends": "./tsconfig.json", "compilerOptions": { "types": ["vitest", "@vitest/browser/providers/playwright"] }, "include": ["vitest.config.ts", ".storybook/vitest.setup.ts", "vitest.shims.d.ts", "stories/**/*.ts", "stories/**/*.tsx"] }package.json (1)
10-12: 브라우저 테스트 실행 스크립트를 분리해 주세요.루트
test가 터보 파이프라인을 태우는 형태라 브라우저 기반 Vitest가 의도치 않게 생략될 수 있습니다. 명시적으로 브라우저 테스트 스크립트를 추가하면 CI/로컬에서 실행이 명확해집니다.다음 스크립트 추가를 제안드립니다:
"check-types": "turbo run check-types", "test": "turbo run test", + "test:browser": "vitest --browser", "storybook": "storybook dev -p 6006", "build-storybook": "storybook build"Playwright 런타임 설치가 필요한 환경(CI 등)에서는 아래 중 하나도 권장합니다.
+ "postinstall": "playwright install --with-deps"또는 CI 워크플로우에
npx playwright install --with-deps단계 추가..storybook/preview.ts (2)
1-1:Preview타입은 프레임워크 비의존 경로(@storybook/react)에서 가져오는 것을 권장
@storybook/react-vite가 타입을 재노출할 수도 있으나, 구성 파일의 이식성과 IDE 지원을 위해@storybook/react경로가 권장됩니다.다음과 같이 변경 제안:
-import type { Preview } from '@storybook/react-vite'; +import type { Preview } from '@storybook/react';
3-19: 기본 파라미터 구성은 적절합니다controls 매처와 a11y 파라미터의 기본값 구성이 합리적입니다. 이후 디자인 토큰/다크 모드가 들어오면
backgrounds,darkMode파라미터 추가를 고려해 주세요..storybook/main.ts (1)
9-11: ESM 환경에서require.resolve사용 시createRequire필요 가능성패키지 루트에
"type": "module"이 설정되었거나 Storybook이 ESM로 이 파일을 로드하면require가 정의되지 않아 런타임 오류가 발생할 수 있습니다. 호환성을 위해createRequire를 사용하도록 방어적으로 변경하는 것을 권장합니다. 또한 반환 타입을string으로 명시하면 타입 안정성이 좋아집니다.아래 패치를 제안드립니다:
import { join, dirname } from "path" +import { createRequire } from "module" + +const require = createRequire(import.meta.url) /** * This function is used to resolve the absolute path of a package. * It is needed in projects that use Yarn PnP or are set up within a monorepo. */ -function getAbsolutePath(value: string): any { +function getAbsolutePath(value: string): string { return dirname(require.resolve(join(value, 'package.json'))) }추가로, 코드베이스 컨벤션에 맞춰 따옴표/세미콜론 스타일을 통일하면 린트 소음이 줄어듭니다.
stories/button.css (2)
14-18: Secondary 유형은 경계/호버/활성 상태가 필요할 수 있습니다inset box-shadow로만 구분하면 다크 모드/저대비 환경에서 식별이 어려울 수 있습니다. 테마 토큰 기반의
border-color와:hover/:active상호작용 스타일을 추가하는 것을 권장합니다.원하시면 디자인 토큰(예: CSS 변수) 기반으로 클래스 확장안을 제안드리겠습니다.
19-30: 비활성 및 커서 상태 정의 추가 권장크기 변형 외에
:disabled상태와 포인터 커서 억제를 명시하면 사용성이 좋아집니다.아래 패치를 제안드립니다:
.storybook-button--large { padding: 12px 24px; font-size: 16px; } + +.storybook-button:disabled, +.storybook-button[aria-disabled="true"] { + opacity: 0.5; + cursor: not-allowed; + /* 필요 시 포커스 제거: outline: none; */ +}또한 가능한 경우 하드코딩 값 대신 디자인 시스템 토큰(
var(--color-primary),var(--radius-xl)등) 사용을 권장합니다.stories/Configure.mdx (3)
229-231: CSS 선택자 단순화(:not의 복합 선택자 사용 회피)
.sb-section a:not(h1 a, h2 a, h3 a)처럼 복합 선택자를:not()안에서 사용하는 것은 브라우저 호환성 이슈가 있습니다. 여기서는 헤딩 내 링크가 없어 보이므로 단순화하는 편이 안전합니다.적용 제안(diff):
- .sb-section a:not(h1 a, h2 a, h3 a) { + .sb-section a { font-size: 14px; }
278-282: 중복 스타일 제거: background와 background-color 동시 지정동일한 색상에 대해
background-color와background가 중복 지정되어 있습니다. 하나만 유지하세요.적용 제안(diff):
- background-color: #EEF3F8; + background-color: #EEF3F8; @@ - background: #EEF3F8;
199-199: 문구 오타 수정"Follow guided walkthroughs on for key workflows." → 전치사 중복입니다.
적용 제안(diff):
- <p>Follow guided walkthroughs on for key workflows.</p> + <p>Follow guided walkthroughs for key workflows.</p>vitest.config.ts (1)
32-33: setupFiles 경로를 절대경로로 지정해 환경 의존성 제거상대 경로는 워크디렉터리 변화(CI 작업 디렉터리 등)에 취약할 수 있습니다.
path.resolve(dirname, ...)로 고정하는 편이 안전합니다.적용 제안(diff):
- setupFiles: ['.storybook/vitest.setup.ts'], + setupFiles: [path.resolve(dirname, '.storybook/vitest.setup.ts')],stories/Button.tsx (1)
18-37: ref 전달 지원(선택) 및 클래스 결합 개선(선택)
- 포커스 관리/테스트 편의를 위해
forwardRef지원을 고려하세요.- 클래스 결합은 현재로 충분하지만, 프로젝트 전반에서
clsx/cx유틸을 사용 중이라면 일관성을 맞추는 것도 좋습니다.적용 제안(diff: forwardRef 예시):
-export const Button = ({ - primary = false, - size = 'medium', - backgroundColor, - label, - ...props -}: ButtonProps) => { +export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(({ + primary = false, + size = 'medium', + backgroundColor, + label, + ...props +}, ref) => { const mode = primary ? 'storybook-button--primary' : 'storybook-button--secondary'; return ( <button type="button" + ref={ref} className={['storybook-button', `storybook-button--${size}`, mode].join(' ')} style={{ backgroundColor }} {...props} > {label} </button> - ); -}; + ); +});stories/Button.stories.ts (1)
25-54: 스토리 구성은 전반적으로 적절함
tags: ['autodocs'],args,argTypes,satisfies Meta<typeof Button>사용이 깔끔합니다. 버튼 스냅샷/상호작용 테스트가 필요하다면play함수 추가를 고려해도 좋습니다(선택).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (16)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlstories/assets/accessibility.pngis excluded by!**/*.pngstories/assets/accessibility.svgis excluded by!**/*.svgstories/assets/addon-library.pngis excluded by!**/*.pngstories/assets/assets.pngis excluded by!**/*.pngstories/assets/context.pngis excluded by!**/*.pngstories/assets/discord.svgis excluded by!**/*.svgstories/assets/docs.pngis excluded by!**/*.pngstories/assets/figma-plugin.pngis excluded by!**/*.pngstories/assets/github.svgis excluded by!**/*.svgstories/assets/share.pngis excluded by!**/*.pngstories/assets/styling.pngis excluded by!**/*.pngstories/assets/testing.pngis excluded by!**/*.pngstories/assets/theming.pngis excluded by!**/*.pngstories/assets/tutorials.svgis excluded by!**/*.svgstories/assets/youtube.svgis excluded by!**/*.svg
📒 Files selected for processing (16)
.storybook/main.ts(1 hunks).storybook/preview.ts(1 hunks).storybook/vitest.setup.ts(1 hunks)package.json(1 hunks)stories/Button.stories.ts(1 hunks)stories/Button.tsx(1 hunks)stories/Configure.mdx(1 hunks)stories/Header.stories.ts(1 hunks)stories/Header.tsx(1 hunks)stories/Page.stories.ts(1 hunks)stories/Page.tsx(1 hunks)stories/button.css(1 hunks)stories/header.css(1 hunks)stories/page.css(1 hunks)vitest.config.ts(1 hunks)vitest.shims.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- stories/header.css
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T18:42:42.797Z
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#22
File: packages/design-system/package.json:32-35
Timestamp: 2025-08-23T18:42:42.797Z
Learning: Storybook packages have different release cycles and stable versions. As of 2025, storybook/test and storybook/blocks have their latest stable versions in the 8.x line (8.6.14), while other packages like storybook/react-vite are stable in 9.x line (9.1.3). Mixed major versions across Storybook packages can be intentional and correct when using the latest stable version of each package.
Applied to files:
package.json
🧬 Code graph analysis (5)
stories/Page.tsx (1)
stories/Header.tsx (1)
Header(17-56)
stories/Page.stories.ts (2)
stories/Page.tsx (1)
Page(10-73)stories/Header.stories.ts (2)
LoggedOut(34-34)LoggedIn(26-32)
stories/Header.stories.ts (1)
stories/Header.tsx (1)
Header(17-56)
stories/Header.tsx (1)
stories/Button.tsx (1)
Button(19-37)
stories/Button.stories.ts (1)
stories/Button.tsx (1)
Button(19-37)
🔇 Additional comments (16)
stories/Page.tsx (1)
23-56: 외부 링크 보안 속성 적절히 사용됨target="_blank" 사용 시 rel="noopener noreferrer"를 모두 지정하여 보안/성능 이슈를 예방하고 있습니다. 그대로 유지하시길 권장합니다.
stories/Header.stories.ts (2)
16-21: 액션 핸들러에 storybook/test의 fn() 사용 적절상호작용 로깅/테스트 안정성을 위해 기본 args에 fn()을 주입한 부분이 좋습니다. 스토리 자체 실행 시 콘솔 노이즈도 최소화됩니다.
1-1: 타입 import 경로 오류 가능성: '@storybook/react'로 교체 권장Storybook 8 기준으로 타입은 '@storybook/react'에서 가져오는 것이 표준입니다. '@storybook/react-vite'는 프레임워크 패키지이며 타입을 재노출하지 않을 수 있어 TS 에러가 날 수 있습니다.
-import type { Meta, StoryObj } from '@storybook/react-vite'; +import type { Meta, StoryObj } from '@storybook/react';⛔ Skipped due to learnings
Learnt from: jjangminii PR: Pinback-Team/pinback-client#22 File: packages/design-system/package.json:32-35 Timestamp: 2025-08-23T18:42:42.797Z Learning: Storybook packages have different release cycles and stable versions. As of 2025, storybook/test and storybook/blocks have their latest stable versions in the 8.x line (8.6.14), while other packages like storybook/react-vite are stable in 9.x line (9.1.3). Mixed major versions across Storybook packages can be intentional and correct when using the latest stable version of each package.stories/Header.tsx (1)
45-51: Button 사용 및 접근성 역할 적절Button 컴포넌트에 type="button"이 기본 지정되어 있고, onClick 핸들러도 정상 전달됩니다. UI 상태 전환(로그인/로그아웃) 시맨틱이 명확합니다.
vitest.shims.d.ts (1)
1-1: 경로 자체는 올바르게 보입니다.
@vitest/browser/providers/playwright타입 참조는 Vitest Browser + Playwright 조합에서 권장되는 형태와 일치합니다. 현재 구성에서 타입 해상도 오류가 없다면 유지해도 무방합니다.package.json (2)
15-25: Storybook 9.x 구성 버전 정렬 상태 양호애드온/프레임워크를 9.1.3으로 정렬한 점 좋습니다. 이전 PR에서 공유된 러닝(일부 Storybook 패키지는 8.x 안정선, 다른 패키지는 9.x 안정선)도 고려하면 현재 선택은 합리적입니다.
참고: 이전 학습 내용을 반영했습니다(스토리북 패키지의 메이저 혼용이 의도적일 수 있음).
27-31: ✅ Vite 7.1.2 오버라이드 충돌 없음 확인• .pnpm.overrides.vite → 7.1.2
• 모든 하위 패키지(package.json) Vite 요구 버전 → ^7.1.2
→ 버전 범위 상 충돌 없음 확인됨.
• Storybook 9.1.3(@storybook/react-vite)은 Vite 5 이상을 지원하므로 Vite 7.1.2에서도 호환에 문제 없을 것으로 보입니다.추가로 로컬에서 Storybook 실행 및 빌드 테스트만 진행해 주시면 됩니다.
.storybook/preview.ts (1)
12-17:a11y.test값 유효성 확인 필요
a11y.test: 'todo'설정이 현재 사용 중인 애드온 버전에서 허용되는 값인지 확인해 주세요. 일부 버전에서는'warn' | 'error' | 'off'등 다른 리터럴을 기대합니다. 타입 오류 없이 동작한다면 유지해도 되지만, CI 실패 기준을 명확히 하려면'error'로 고정하는 것도 방법입니다.원한다면 Preview 파라미터에 a11y 룰 세분화(예: 특정 규칙 off)도 제안드릴 수 있습니다.
.storybook/main.ts (2)
13-16: stories 경로 설정은 합리적입니다
.storybook이 루트에 있고 별도configDir오버라이드가 없다면../stories/**패턴이 올바르게 상위의stories디렉토리를 가리킵니다.
17-22: 애드온 해상도 방식은 PnP/모노레포에서 유용합니다
getAbsolutePath로 애드온 경로를 고정하는 접근은 Yarn PnP나 monorepo에서 흔히 겪는 모듈 해상도 문제를 예방합니다. 유지하셔도 좋습니다.stories/button.css (1)
10-13: WCAG AA 대비 비율 확인 완료: 색상 기준 충족
- stories/button.css (Lines 10–13):
#555ab9배경색과 흰색 텍스트의 대비 비율이 5.92:1로, WCAG AA(최소 4.5:1) 기준을 충족합니다.
→ 추가 색상 조정이나 폰트 굵기·사이즈 변경은 필요하지 않습니다.stories/Configure.mdx (1)
34-41: PR 목표와 산출물 간 괴리 확인 요청PR 제목/이슈(#21)는 날짜/시간 입력 컴포넌트 추가를 목표로 하나, 본 문서는 Storybook 구성 가이드 페이지입니다. 실제 Date/Time Input 컴포넌트 구현과 스토리가 포함되어 있는지 확인 부탁드립니다. 필요 시 동일 PR에서 파일 추가 또는 별도 PR로 분리하는 방안을 고려해주세요.
.storybook/vitest.setup.ts (2)
1-7: Portable Stories 세팅 적절함
setProjectAnnotations([a11yAddonAnnotations, projectAnnotations])로 테스트 환경에 미리보기 설정을 주입한 점 좋습니다. a11y 애드온을 테스트에도 일관 적용할 수 있습니다.
1-7: 확인됨: Storybook 및 Vitest 설정에 필요한 의존성과 구성 파일이 모두 존재합니다
.storybook/preview.ts(x)파일이 실제로 존재합니다.package.jsondevDependencies에@storybook/addon-a11y,@storybook/react-vite,@storybook/addon-vitest(또는 유사 테스트 러너용 애드온) 등이 모두 등록되어 있습니다.위 점검 스크립트가 모두 정상 통과되었으므로, 추가 조치 없이 현재 코드로도 의도한 Storybook + Vitest 설정이 올바르게 작동할 것으로 보입니다.
vitest.config.ts (1)
26-31: 브라우저 테스트 전제 조건 확인(Playwright / @vitest/browser 설치)
provider: 'playwright'를 사용하고 있어 CI/로컬에 Playwright 브라우저 바이너리 설치가 필요합니다. 또한@vitest/browser(또는 대응 패키지) 의존성 유무를 확인하세요. 없다면 테스트 실행 시 실패합니다.stories/Button.stories.ts (1)
1-1: 타입 임포트 경로 수정: '@storybook/react' 사용스토리 타입(
Meta,StoryObj)은 렌더러 프레임워크 패키지(@storybook/react-vite)가 아니라@storybook/react에서 가져와야 합니다. 현재 경로는 타입 해석 실패 가능성이 높습니다.적용 제안(diff):
-import type { Meta, StoryObj } from '@storybook/react-vite'; +import type { Meta, StoryObj } from '@storybook/react';Likely an incorrect or invalid review comment.
package.json
Outdated
| "vite": "7.1.2", | ||
| "vitest": "^3.2.4", | ||
| "@vitest/browser": "^3.2.4", | ||
| "playwright": "^1.55.0", |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
CI에서 Playwright 브라우저 미설치로 테스트 실패 가능성
playwright만 devDependency로 추가하면, CI 컨테이너에서 브라우저 바이너리가 없어 브라우저 모드 테스트가 실패할 수 있습니다. postinstall 훅 또는 CI 단계에 playwright install을 추가하세요.
아래 검색으로 이미 설치 단계가 있는지 확인해 주세요:
🏁 Script executed:
#!/bin/bash
rg -nP 'playwright\s+install' -C2Length of output: 35
🏁 Script executed:
#!/bin/bash
# package.json에서 postinstall 스크립트 존재 여부 확인
rg -n '"postinstall":' -C2 package.jsonLength of output: 41
Playwright 브라우저 설치 스크립트 추가 필요
패키지 설치 후 Playwright 브라우저 바이너리가 자동으로 설치되지 않아, CI 환경에서 브라우저 모드 테스트가 실패할 수 있습니다.
현재 package.json에서 postinstall 훅이 정의되어 있지 않으며, CI 스크립트에도 playwright install 명령어가 존재하지 않습니다.
수정 제안 (package.json):
{
"scripts": {
+ "postinstall": "playwright install",
// 기존 스크립트 유지
},
"devDependencies": {
"playwright": "^1.55.0",
// 기타 devDependencies
}
}또는 CI 설정에 아래 명령을 추가해 주세요:
npm ci
npm run postinstall # 또는 npx playwright install
npm test위와 같이 설정하면, 로컬 및 CI 환경 모두에서 Playwright 브라우저가 올바르게 설치되어 테스트가 안정적으로 수행됩니다.
🤖 Prompt for AI Agents
In package.json around line 30, Playwright binaries are not being installed
after dependency install, causing CI/browser tests to fail; add a postinstall
script entry that runs Playwright's installer (e.g., "postinstall": "npx
playwright install --with-deps" or "npx playwright install" depending on needs)
or ensure CI pipeline runs `npx playwright install` (or `npm run postinstall`)
after `npm ci` so browsers are present for tests.
stories/button.css
Outdated
| .storybook-button { | ||
| display: inline-block; | ||
| cursor: pointer; | ||
| border: 0; | ||
| border-radius: 3em; | ||
| font-weight: 700; | ||
| line-height: 1; | ||
| font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
키보드 접근성: 포커스 표시 추가 권장
기본 .storybook-button에서 포커스 링이 정의되어 있지 않습니다. 키보드 사용자 접근성을 위해 :focus-visible 스타일을 추가해 주세요.
아래 패치를 제안드립니다:
.storybook-button {
display: inline-block;
cursor: pointer;
border: 0;
border-radius: 3em;
font-weight: 700;
line-height: 1;
font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif;
}
+
+.storybook-button:focus-visible {
+ outline: 2px solid #1f6feb; /* 디자인 토큰 사용 가능 시 교체 */
+ outline-offset: 2px;
+}📝 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.
| .storybook-button { | |
| display: inline-block; | |
| cursor: pointer; | |
| border: 0; | |
| border-radius: 3em; | |
| font-weight: 700; | |
| line-height: 1; | |
| font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif; | |
| } | |
| .storybook-button { | |
| display: inline-block; | |
| cursor: pointer; | |
| border: 0; | |
| border-radius: 3em; | |
| font-weight: 700; | |
| line-height: 1; | |
| font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif; | |
| } | |
| .storybook-button:focus-visible { | |
| outline: 2px solid #1f6feb; /* 디자인 토큰 사용 가능 시 교체 */ | |
| outline-offset: 2px; | |
| } |
🤖 Prompt for AI Agents
In stories/button.css around lines 1 to 9, the .storybook-button lacks a
:focus-visible rule so keyboard users get no visible focus; add a
.storybook-button:focus-visible selector that applies a clearly visible focus
indicator (for example a high-contrast outline or box-shadow), preserve the
existing border-radius and spacing by using outline-offset or matching
border-radius, and avoid changing default mouse focus by using :focus-visible
rather than :focus.
stories/Button.tsx
Outdated
| export interface ButtonProps { | ||
| /** Is this the principal call to action on the page? */ | ||
| primary?: boolean; | ||
| /** What background color to use */ | ||
| backgroundColor?: string; | ||
| /** How large should the button be? */ | ||
| size?: 'small' | 'medium' | 'large'; | ||
| /** Button contents */ | ||
| label: string; | ||
| /** Optional click handler */ | ||
| onClick?: () => void; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ButtonProps는 기본 버튼 속성(접근성/이벤트 포함)을 확장해야 함
현재 onClick?: () => void는 React의 표준 시그니처(MouseEventHandler<HTMLButtonElement>)와 불일치하고, 나머지 버튼 속성들(disabled, aria-*, type, 등)이 타입에 반영되지 않습니다. 소비자 입장에서 타입 안정성이 떨어집니다.
적용 제안(diff):
-export interface ButtonProps {
+export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
/** Is this the principal call to action on the page? */
primary?: boolean;
/** What background color to use */
backgroundColor?: string;
/** How large should the button be? */
size?: 'small' | 'medium' | 'large';
/** Button contents */
label: string;
- /** Optional click handler */
- onClick?: () => void;
}📝 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 ButtonProps { | |
| /** Is this the principal call to action on the page? */ | |
| primary?: boolean; | |
| /** What background color to use */ | |
| backgroundColor?: string; | |
| /** How large should the button be? */ | |
| size?: 'small' | 'medium' | 'large'; | |
| /** Button contents */ | |
| label: string; | |
| /** Optional click handler */ | |
| onClick?: () => void; | |
| } | |
| export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { | |
| /** Is this the principal call to action on the page? */ | |
| primary?: boolean; | |
| /** What background color to use */ | |
| backgroundColor?: string; | |
| /** How large should the button be? */ | |
| size?: 'small' | 'medium' | 'large'; | |
| /** Button contents */ | |
| label: string; | |
| } |
🤖 Prompt for AI Agents
In stories/Button.tsx around lines 5-16, the ButtonProps definition currently
declares a custom onClick and omits standard button attributes; update
ButtonProps to extend React.ButtonHTMLAttributes<HTMLButtonElement> (or
React.ComponentProps<'button'>) so built-in attributes like disabled, type,
aria-*, and the correct onClick signature
(React.MouseEventHandler<HTMLButtonElement>) are included, keep/override
specific props (primary, backgroundColor, size, label) as needed but remove the
standalone onClick type to avoid conflict and ensure label remains required.
stories/Configure.mdx
Outdated
| @@ -0,0 +1,364 @@ | |||
| import { Meta } from "@storybook/addon-docs/blocks"; | |||
There was a problem hiding this comment.
Storybook 8 호환: Meta import 경로 교체 필요
@storybook/addon-docs/blocks는 최신 버전에서 더 이상 권장되지 않습니다. MDX 문서 컴포넌트는 @storybook/blocks에서 가져오는 것이 안전합니다.
적용 제안(diff):
-import { Meta } from "@storybook/addon-docs/blocks";
+import { Meta } from "@storybook/blocks";📝 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.
| import { Meta } from "@storybook/addon-docs/blocks"; | |
| import { Meta } from "@storybook/blocks"; |
🤖 Prompt for AI Agents
In stories/Configure.mdx around line 1, the file imports Meta from the
deprecated path "@storybook/addon-docs/blocks"; update the import to use the
supported package by replacing that path with "@storybook/blocks" so the line
reads an import of Meta from "@storybook/blocks", keeping the same named import
and ensuring any bundler/storybook config accepts the new package.
stories/Configure.mdx
Outdated
| href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure" | ||
| target="_blank" | ||
| >Learn more<RightArrow /></a> |
There was a problem hiding this comment.
보안: target="_blank" 링크에 rel="noopener noreferrer" 추가
외부 링크에 target="_blank"만 사용하면 opener를 통해 현재 페이지가 조작될 수 있습니다. 모든 외부 링크에 rel="noopener noreferrer"를 추가하세요.
예시(diff, 50–53행 블록):
- <a
- href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure"
- target="_blank"
- >Learn more<RightArrow /></a>
+ <a
+ href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure"
+ target="_blank"
+ rel="noopener noreferrer"
+ >Learn more<RightArrow /></a>위와 동일한 방식으로 본 코멘트에 표시된 모든 앵커에 적용해주세요.
Also applies to: 63-65, 75-77, 97-99, 106-108, 116-118, 126-128, 135-137, 144-146, 156-158, 171-173, 181-183, 192-194, 202-204
🤖 Prompt for AI Agents
In stories/Configure.mdx around lines 51-53 (also apply to 63-65, 75-77, 97-99,
106-108, 116-118, 126-128, 135-137, 144-146, 156-158, 171-173, 181-183, 192-194,
202-204), anchor tags using target="_blank" are missing rel="noopener
noreferrer"; update each of those <a> elements to include rel="noopener
noreferrer" alongside target="_blank" to prevent window.opener based attacks and
ensure you apply this change consistently to every anchor in the listed line
ranges.
stories/Page.stories.ts
Outdated
| @@ -0,0 +1,33 @@ | |||
| import type { Meta, StoryObj } from '@storybook/react-vite'; | |||
There was a problem hiding this comment.
타입 import 경로 오류 가능성: '@storybook/react'로 교체 권장
Vite 사용 여부와 무관하게 타입은 '@storybook/react'에서 가져오는 것이 권장/기본입니다. 현재 경로는 TS 타입 해석 실패 위험이 있습니다.
-import type { Meta, StoryObj } from '@storybook/react-vite';
+import type { Meta, StoryObj } from '@storybook/react';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In stories/Page.stories.ts around line 1, the type import currently uses
'@storybook/react-vite' which can cause TypeScript resolution issues; change the
import to pull Meta and StoryObj from '@storybook/react' instead, update any
import statements accordingly, and run type-check/TS server to confirm the types
resolve correctly.
There was a problem hiding this comment.
너무 깔끔하게 구현하셨어요-! 어려운 부분이었을 텐데 고생 많으셨습니다 🙌
다만 한 가지 UX 제안이 있어요.
지금 09 입력 흐름에서 0만 입력해도 바로 “오전 12시”로 포맷되어 표시되는데, 사용자는 아직 시간을 완성하지 않았는데 포맷되었다고 느낄 수 있어 혼란스러울 수 잇을거같아요
두 자리(HH)가 완성된 시점부터 포맷을 적용하는 건 어떨까요?
0 → 그대로 0(포맷 미적용)
09 → “오전 09”
0930 → “오전 09:30”
한 자리일 때는 원문 유지, 두 자리 이상부터 포맷 시작하는 규칙이면 입력 중간 상태가 더 자연스럽게 느껴질 것 같아요.
| e.preventDefault(); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
포맷 구현 너무 잘하셨어요-!! 테스트 해보는데 진짜 너무 신기했어요 다만 한파일에 있으니 가독성이 떨어지는 느낌이라 파일 분리하는건 어떤가요? 아니면 공통 컴포넌트 부분이라 따로 분리 안해도될지 고민되네요
There was a problem hiding this comment.
맞아요! 파일 분리 안그래도 어떻게 나눠서 분리할 지 고민중이였어요!
말씀 주신 0과 같은 한자리 수일 경우에는 포맷팅이 불가하도록 로직 수정해둘게요! 이 부분은 생각 못했는데 감사합니당
There was a problem hiding this comment.
좋은 의견 감사합니다! 적용해두었습니ㄷ당
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (1)
33-36: 스토리 문서의 '커서/백스페이스 커스텀' 설명이 구현과 불일치 가능성현재 DateTime.tsx는 nextCaretRef만 계산하고 selectionRange 적용(useEffect)과 삭제 키의 단일 처리 경로가 보이지 않아 문서 설명(커서 이동 및 삭제 커스텀)이 오도될 수 있습니다. 이전 코멘트와 동일한 이슈입니다. 문서 문구를 일단 현실화하거나, 컴포넌트 쪽 보완 후 문서를 유지하는 방향 중 하나를 택해 주세요.
문구만 조정하는 임시안:
- '날짜는 실시간 포맷(YYYY.MM.DD), 시간은 **숫자 버퍼(HHMM)** → 표시(오전/오후 HH:MM)로 렌더. 커서/백스페이스도 커스텀 처리됨.', + '날짜는 실시간 포맷(YYYY.MM.DD), 시간은 **숫자 버퍼(HHMM)** → 표시(오전/오후 HH:MM)로 렌더. (삭제 동작은 커스텀 처리, 커서 재배치는 추후 반영 예정)',컴포넌트 보완(별도 파일)이 가능하다면, 이전 제안대로 caret 적용 useEffect 추가 및 Backspace/Delete의 단일 처리(onBeforeInput로 일원화, onKeyDown에서는 조기 반환)를 권장합니다. 원하시면 패치 PR로 묶어드릴게요.
🧹 Nitpick comments (6)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (6)
2-2: 상호작용 스토리에 단언 추가해 회귀 방지타이핑만 하고 결과 검증이 없어 회귀를 못 잡습니다. 기대 포맷을 단언해 주세요.
-import { within, userEvent } from '@storybook/test'; +import { within, userEvent, expect } from '@storybook/test'; @@ play: async ({ canvasElement }) => { const canvas = within(canvasElement); const input = await canvas.findByLabelText('시간 입력'); await userEvent.type(input, '1620'); + await expect(input).toHaveValue('오후 04:20'); },Also applies to: 65-69
80-90: Backspace 데모의 안정성 확보(플레이 단언 + 키 입력 축약)현재는 키 입력만 있어 실패 여부가 불명확합니다. 단언을 추가하고 키 입력을 간결하게 표현하세요. 컴포넌트가 keydown/beforeinput 중복 처리 상태라면 브라우저별로 1키에 2삭제가 발생할 수 있어 플래키해질 수 있습니다(상위 코멘트 참고).
const input = await canvas.findByLabelText('시간 입력'); await userEvent.click(input); - await userEvent.keyboard('{Backspace}{Backspace}'); + await userEvent.keyboard('{Backspace>2}'); + await expect(input).toHaveValue('오후 04');
44-50: 날짜 포맷 산출을 검증하는 플레이 추가초깃값으로 포맷팅이 맞게 되는지 단언을 넣어 주세요.
export const Date_Default: Story = { args: { type: 'date', value: '20250819', }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const input = await canvas.findByLabelText('날짜 입력'); + await expect(input).toHaveValue('2025.08.19'); + }, };참고: 위 코멘트에서
expectimport를 추가했다면 중복 import는 제거해 주세요.
52-57: 빈 시간 입력에서 진행형 포맷 검증 스토리 보강“17” 입력 시 즉시 “오후 05”로 포맷팅되는 요구사항을 스토리에서 검증해 주세요.
export const Time_Empty: Story = { args: { type: 'time', value: '', }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const input = await canvas.findByLabelText('시간 입력'); + await userEvent.type(input, '17'); + await expect(input).toHaveValue('오후 05'); + }, };
5-5: 타입 선언을 'satisfies Meta<...>' 패턴으로 강화TS 4.9+라면
satisfies를 사용해 과/부족 프로퍼티를 컴파일 타임에 잡을 수 있습니다. 런타임 영향은 없습니다.-const meta: Meta<typeof DateTime> = { +const meta = { title: 'Components/DateTime', component: DateTime, tags: ['autodocs'], args: { state: 'default', type: 'date', value: '', }, argTypes: { state: { control: { type: 'select' }, options: ['default', 'disabled', 'error'], description: '스타일 상태', }, type: { control: { type: 'inline-radio' }, options: ['date', 'time'], description: '입력 모드', }, value: { control: 'text', description: '초기값(숫자만 입력해도 됨)', }, }, parameters: { layout: 'centered', docs: { description: { component: '날짜는 실시간 포맷(YYYY.MM.DD), 시간은 **숫자 버퍼(HHMM)** → 표시(오전/오후 HH:MM)로 렌더. 커서/백스페이스도 커스텀 처리됨.', }, }, }, -}; +} satisfies Meta<typeof DateTime>;TS 버전이 4.9 미만이면 이 변경은 보류하세요(위 스크립트로 버전 확인).
Also applies to: 39-39
98-111: States_Showcase에 접근성·상태 단언 추가(특히 disabled)disabled 상태가 실제로 비활성화되는지 단언을 추가하면 리그레션을 잡는 데 유용합니다.
export const States_Showcase: Story = { render: (args) => ( <div style={{ display: 'grid', gap: 12 }}> <DateTime {...args} state="default" type="date" value="20250101" /> <DateTime {...args} state="disabled" type="date" value="20250101" /> <DateTime {...args} state="error" type="time" value="0930" /> </div> ), parameters: { controls: { exclude: ['state', 'type', 'value'] }, docs: { description: { story: '상태별 스타일 한눈에 보기.' } }, }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const dateInputs = await canvas.findAllByLabelText('날짜 입력'); + // 두 번째 날짜 입력은 disabled여야 함 + await expect(dateInputs[1]).toBeDisabled(); + }, };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/design-system/src/components/dateTime/DateTime.stories.tsx(1 hunks)packages/design-system/src/components/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/src/components/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (1)
packages/design-system/src/components/dateTime/DateTime.tsx (1)
DateTime(49-223)
🔇 Additional comments (1)
packages/design-system/src/components/dateTime/DateTime.stories.tsx (1)
1-1:@storybook/react-viteimport 유지 필요 – 변경 불필요패키지
packages/design-system의package.json에@storybook/react-vite@9.1.3만 정의되어 있으며,@storybook/react는 아예 의존성에 없습니다. Vite 빌더 환경에서는@storybook/react-vite사용이 올바른 방식이며, 제안된@storybook/react로의 교체는 유효하지 않습니다. 또한 Storybook 패키지들은 v8/v9 라인별로 안정 버전이 다르게 릴리스되고 있으므로(blocks·test는 8.x, 그 외는 9.x), 현재 상태가 의도된 설정입니다.
- 파일:
packages/design-system/src/components/dateTime/DateTime.stories.tsx(1행)- 기존 코드 유지:
import type { Meta, StoryObj } from '@storybook/react-vite';Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/design-system/src/components/dateTime/DateTime.tsx (4)
75-99: 시간 범위(00–23/00–59) 검증 헬퍼 추가 권장표시 포맷은 훌륭하나, 현재는
25:99같은 값도 표시 가능합니다. PR의 유효성 관련 결정을 존중하되(최종 저장 시 상위에서 검증), 입력 단계에서 명백히 불가능한 범위만 최소 차단하면 UX가 크게 안정됩니다.function formatTime12(digits: string) { @@ return out; } + + // 24시간 기준 최소 범위 검증 (증분 입력 허용) + function isValidTimeDigits(d: string) { + const v = d.slice(0, 4); + if (v.length === 0) return true; + const hh = parseInt(v.slice(0, 2) || '0', 10); + if (Number.isNaN(hh) || hh > 23) return false; + if (v.length >= 3) { + const mm = parseInt(v.slice(2, 4) || '0', 10); + if (Number.isNaN(mm) || mm > 59) return false; + } + return true; + }
208-226: 접근성/패턴 정합성 개선: aria 속성, 패턴 앵커, placeholder 일치
state === 'error'일 때aria-invalid지정 필요.- 비활성화 시
aria-disabled를 함께 노출.- HTML
pattern은 앵커(^...$)가 없어 엉뚱한 문자열에도 매칭될 수 있습니다.- 시간 placeholder가 실제 렌더 문자열(“오전 HH[:MM]”)과 불일치합니다.
<input ref={inputRef} type="text" className={dateTimeTxtStyles({ state })} value={input} onBeforeInput={handleBeforeInput} onKeyDown={handleKeyDown} onChange={handleChange} - placeholder={type === 'date' ? 'YYYY.MM.DD' : 'HH:MM'} + placeholder={type === 'date' ? 'YYYY.MM.DD' : '오전 HH:MM'} inputMode="numeric" - disabled={isDisabled} + disabled={isDisabled} + aria-disabled={isDisabled} + aria-invalid={state === 'error' ? 'true' : 'false'} + autoComplete="off" + spellCheck={false} maxLength={type === 'date' ? 10 : 8} pattern={ type === 'date' - ? '\\d{4}\\.\\d{2}\\.\\d{2}' - : '(오전|오후)\\s\\d{2}:\\d{2}' + ? '^\\d{4}\\.\\d{2}\\.\\d{2}$' + : '^(오전|오후)\\s\\d{2}(:\\d{2})?$' } aria-label={type === 'date' ? '날짜 입력' : '시간 입력'} />
1-1: 커서 저장만 되고 적용되지 않음 — useLayoutEffect로 selectionRange 반영 필요
nextCaretRef에 저장한 커서 위치가 실제 DOM에 적용되지 않아 입력 UX가 흔들립니다. 렌더 직후 커서를 강제로 이동시키는 로직이 필요합니다. 또한 prop 변경 동기화(아래 코멘트)와 함께 동작하도록input/type에 의존시키는 것을 권장합니다.다음 패치를 적용하세요.
-import { useRef, useState, type ChangeEvent } from 'react'; +import { useRef, useState, type ChangeEvent, useLayoutEffect, useEffect } from 'react'; @@ const [input, setInput] = useState(() => type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) ); + + // 렌더 이후 저장된 커서 위치 반영 + useLayoutEffect(() => { + const el = inputRef.current; + const pos = nextCaretRef.current; + if (!el || pos == null) return; + const clamped = Math.max(0, Math.min(pos, el.value.length)); + // iOS 대응을 위해 focus 후 selectionRange 설정 + if (document.activeElement !== el) el.focus(); + el.setSelectionRange(clamped, clamped); + nextCaretRef.current = null; + }, [input, type]);Also applies to: 56-62
115-166: beforeInput 처리: 합성 입력/붙여넣기/삭제 forward 및 범위 검증 보완
- IME 합성 입력(
isComposing) 무시 필요.- 붙여넣기 시
InputEvent.data가 비어있는 브라우저(Safari 등)를 위해clipboardData/dataTransfer폴백 필요.Delete(forward)도deleteContentForward로 통합 처리 권장.- 시간 범위 검증(
isValidTimeDigits)을 insert/paste 직전에 적용하면 잘못된 값 유입을 최소화할 수 있습니다.const handleBeforeInput = (e: React.FormEvent<HTMLInputElement>) => { if (type !== 'time') return; const ne = e.nativeEvent as InputEvent; const inputType = ne.inputType || ''; - const data = (ne as InputEvent).data; + if ((ne as any).isComposing) return; + const data = (ne as any).data as string | null | undefined; @@ - if (inputType.startsWith('insert') && data) { - const add = data.replace(/\D/g, ''); + // 붙여넣기 전용 처리 (일부 브라우저에서 data가 비어있음) + if (inputType === 'insertFromPaste') { + const dt = (ne as any).dataTransfer ?? (ne as any).clipboardData ?? null; + const pastedRaw = + (typeof data === 'string' ? data : '') || (dt ? dt.getData('text') : ''); + const pasted = digitsOnly(pastedRaw); + if (!pasted) { + e.preventDefault(); + return; + } + const next = (timeDigits + pasted).slice(0, 4); + if (!isValidTimeDigits(next)) { + e.preventDefault(); + return; + } + setTimeDigits(next); + setInput(formatTime12(next)); + nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); + e.preventDefault(); + return; + } + + if (inputType.startsWith('insert')) { + const add = digitsOnly(typeof data === 'string' ? data : ''); if (!add) { e.preventDefault(); return; } if (timeDigits.length >= 4) { // 타입이 4글자 넘어가면 더이상 받지 않기 e.preventDefault(); return; } const next = (timeDigits + add).slice(0, 4); + if (!isValidTimeDigits(next)) { + e.preventDefault(); + return; + } setTimeDigits(next); setInput(formatTime12(next)); nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); e.preventDefault(); return; } - // 백스페이스 인풋일 때, 포맷팅 조절 - if (inputType === 'deleteContentBackward') { + // 삭제(Backspace/Forward) 통합 처리 + if (inputType === 'deleteContentBackward' || inputType === 'deleteContentForward') { if (timeDigits.length === 0) { e.preventDefault(); return; } const next = timeDigits.slice(0, -1); setTimeDigits(next); setInput(formatTime12(next)); nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); e.preventDefault(); return; } - // paste 이벤트일 때 - const pasted = digitsOnly((ne as InputEvent).data ?? ''); - if (pasted) { - const next = (timeDigits + pasted).slice(0, 4); - setTimeDigits(next); - setInput(formatTime12(next)); - nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); - e.preventDefault(); - } else { - e.preventDefault(); - } + // 그 외 입력 유형은 기본 편집 방지 + e.preventDefault(); };
🧹 Nitpick comments (2)
packages/design-system/src/components/dateTime/DateTime.tsx (2)
203-207: 레이블 요소 연결로 접근성 향상 제안시각적
<span>대신 실제<label htmlFor>를 사용해 입력과 연계하면 스크린리더/클릭 포커스가 개선됩니다.useId()로 내부 id를 만들거나idprop을 받아 연결하는 방식을 권장합니다.적용 예시(요약):
import { useId } from 'react'const inputId = useId();<label htmlFor={inputId} ...>...</label><input id={inputId} ... />
4-8: 타입/API 표면 정리: DateTimeProps export 및 onValueChange 검토
DateTimeProps를export하면 소비자 측에서 타입 재사용이 편해집니다.- 현재 컴포넌트는 외부로 변경 이벤트를 내보내지 않습니다. “저장 시 검증” 전략을 따르더라도 상위에서 값 수집이 필요하다면
onValueChange(표준화된 digits:YYYYMMDD/HHmm) 제공을 권장합니다.필요 시
updateDate(digits)/updateTime(digits)헬퍼로 상태 업데이트와 콜백 호출을 한 곳에서 처리하도록 리팩터 제안 드릴게요. 이 방향으로 진행할까요?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/design-system/src/components/dateTime/DateTime.tsx(1 hunks)
🔇 Additional comments (3)
packages/design-system/src/components/dateTime/DateTime.tsx (3)
63-74: 날짜 포맷팅 로직은 간결하고 명확합니다숫자만 추출 → 자리수 슬라이싱 → 구분자 삽입 플로우가 깔끔합니다.
101-113: 커서 매핑 로직 적절날짜/시간 각 포맷 구분자(
.,:및오전프리픽스) 기준으로 커서가 튀지 않도록 잘 매핑되어 있습니다.
10-47: 스타일 변형 설계(cva) 좋습니다상태별 변형이 명확하고, 비활성화에 커서 스타일까지 반영되어 일관적입니다.
| export default function DateTime({ type, value = '', state }: DateTimeProps) { | ||
| const inputRef = useRef<HTMLInputElement>(null); | ||
| const isDisabled = state === 'disabled'; | ||
| const nextCaretRef = useRef<number | null>(null); | ||
| const [timeDigits, setTimeDigits] = useState(() => | ||
| digitsOnly(value).slice(0, 4) | ||
| ); | ||
| const [input, setInput] = useState(() => | ||
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
value/type prop 변경 시 내부 상태 동기화 누락
현재 value는 초기값으로만 사용되며 이후 prop 변경이 반영되지 않습니다. 상위 컨텍스트(팝업 등)에서 값/타입을 교체하는 시나리오에서 UI가 뒤처질 수 있습니다.
const [timeDigits, setTimeDigits] = useState(() =>
digitsOnly(value).slice(0, 4)
);
const [input, setInput] = useState(() =>
type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits)
);
+
+ // 외부 value/type 변경 시 동기화
+ useEffect(() => {
+ const only = digitsOnly(value ?? '');
+ if (type === 'date') {
+ setInput(formatDate(only.slice(0, 8)));
+ } else {
+ const nextDigits = only.slice(0, 4);
+ setTimeDigits(nextDigits);
+ setInput(formatTime12(nextDigits));
+ }
+ nextCaretRef.current = null;
+ }, [value, type]);📝 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 default function DateTime({ type, value = '', state }: DateTimeProps) { | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const isDisabled = state === 'disabled'; | |
| const nextCaretRef = useRef<number | null>(null); | |
| const [timeDigits, setTimeDigits] = useState(() => | |
| digitsOnly(value).slice(0, 4) | |
| ); | |
| const [input, setInput] = useState(() => | |
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | |
| ); | |
| export default function DateTime({ type, value = '', state }: DateTimeProps) { | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const isDisabled = state === 'disabled'; | |
| const nextCaretRef = useRef<number | null>(null); | |
| const [timeDigits, setTimeDigits] = useState(() => | |
| digitsOnly(value).slice(0, 4) | |
| ); | |
| const [input, setInput] = useState(() => | |
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | |
| ); | |
| // 외부 value/type 변경 시 동기화 | |
| useEffect(() => { | |
| const only = digitsOnly(value ?? ''); | |
| if (type === 'date') { | |
| setInput(formatDate(only.slice(0, 8))); | |
| } else { | |
| const nextDigits = only.slice(0, 4); | |
| setTimeDigits(nextDigits); | |
| setInput(formatTime12(nextDigits)); | |
| } | |
| nextCaretRef.current = null; | |
| }, [value, type]); | |
| // …rest of component… | |
| } |
| const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { | ||
| if (type !== 'time') return; | ||
| if (e.nativeEvent.isComposing) return; | ||
|
|
||
| if (e.key === 'Backspace') { | ||
| if (timeDigits.length > 0) { | ||
| const next = timeDigits.slice(0, -1); | ||
| setTimeDigits(next); | ||
| setInput(formatTime12(next)); | ||
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | ||
| } | ||
| e.preventDefault(); | ||
| } | ||
|
|
||
| if (e.key === 'Delete') { | ||
| if (timeDigits.length > 0) { | ||
| const next = timeDigits.slice(0, -1); | ||
| setTimeDigits(next); | ||
| setInput(formatTime12(next)); | ||
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | ||
| } | ||
| e.preventDefault(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
삭제 로직 중복 처리 제거 (keydown vs beforeinput)
현재 Backspace/Delete를 onBeforeInput과 onKeyDown에서 모두 처리합니다. 이벤트 순서상 중복 업데이트 위험이 있고 브라우저별 차이가 생길 수 있습니다. 삭제는 onBeforeInput에서만 처리하고, onKeyDown에서는 관여하지 않도록 단순화하세요.
- const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
- if (type !== 'time') return;
- if (e.nativeEvent.isComposing) return;
-
- if (e.key === 'Backspace') {
- if (timeDigits.length > 0) {
- const next = timeDigits.slice(0, -1);
- setTimeDigits(next);
- setInput(formatTime12(next));
- nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
- }
- e.preventDefault();
- }
-
- if (e.key === 'Delete') {
- if (timeDigits.length > 0) {
- const next = timeDigits.slice(0, -1);
- setTimeDigits(next);
- setInput(formatTime12(next));
- nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
- }
- e.preventDefault();
- }
- };
+ const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
+ if (type !== 'time') return;
+ if (e.nativeEvent.isComposing) return;
+ // 삭제 키는 onBeforeInput에서만 처리하여 중복 방지
+ if (e.key === 'Backspace' || e.key === 'Delete') return;
+ };📝 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 handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { | |
| if (type !== 'time') return; | |
| if (e.nativeEvent.isComposing) return; | |
| if (e.key === 'Backspace') { | |
| if (timeDigits.length > 0) { | |
| const next = timeDigits.slice(0, -1); | |
| setTimeDigits(next); | |
| setInput(formatTime12(next)); | |
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | |
| } | |
| e.preventDefault(); | |
| } | |
| if (e.key === 'Delete') { | |
| if (timeDigits.length > 0) { | |
| const next = timeDigits.slice(0, -1); | |
| setTimeDigits(next); | |
| setInput(formatTime12(next)); | |
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | |
| } | |
| e.preventDefault(); | |
| } | |
| }; | |
| const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { | |
| if (type !== 'time') return; | |
| if (e.nativeEvent.isComposing) return; | |
| // 삭제 키는 onBeforeInput에서만 처리하여 중복 방지 | |
| if (e.key === 'Backspace' || e.key === 'Delete') return; | |
| }; |
🤖 Prompt for AI Agents
In packages/design-system/src/components/dateTime/DateTime.tsx around lines 178
to 201, remove the duplicated Backspace/Delete handling from the handleKeyDown
handler so deletion is only processed in onBeforeInput; specifically delete the
if-blocks that check e.key === 'Backspace' and e.key === 'Delete' (including
their timeDigits slicing, setTimeDigits, setInput, nextCaretRef updates and
e.preventDefault calls), keep the early returns (type !== 'time' and
isComposing) and leave the rest of keydown logic intact so keydown no longer
intercepts deletion events.
There was a problem hiding this comment.
이 부분 확인해보니 중복되는 부분이 있는 것 같은데 한번 체크해주세요!
만약 중복으로 사용이 되어야 한다면 backspace 누르면 작동하는 함수를 따로 분리해서 재사용해도 좋을 것 같아요. 그리고 굳이 재사용을 안하더라도,
if (e.key === 'Backspace') {
if (timeDigits.length > 0) {
const next = timeDigits.slice(0, -1);
setTimeDigits(next);
setInput(formatTime12(next));
nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
}
e.preventDefault();
}위와 같은 로직에 이름을 붙여서 함수로 분리해주면 가독성이 좋아질 것 같아요.
해당 handleKeyDown 함수가 너무 많은 책임/역할을 가지고 있는 것 같다고 생각이 듭니다!
There was a problem hiding this comment.
@constantly-dev 맞습니다! 중복 처리 문제를 정확히 짚어주셨네요. onBeforeInput의 deleteContentBackward와 onKeyDown의 Backspace/Delete 처리가 동일한 로직을 중복 실행할 위험이 있습니다.
말씀하신 대로 공통 로직을 분리하여 가독성과 유지보수성을 개선하는 것이 좋겠습니다:
// 삭제 로직을 별도 함수로 분리
const handleTimeDelete = useCallback(() => {
if (timeDigits.length > 0) {
const next = timeDigits.slice(0, -1);
setTimeDigits(next);
setInput(formatTime12(next));
nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
}
}, [timeDigits]);
const handleBeforeInput = (e: React.FormEvent<HTMLInputElement>) => {
if (type !== 'time') return;
const ne = e.nativeEvent as InputEvent;
const inputType = ne.inputType || '';
// ... 기존 insert 로직 ...
// 백스페이스 처리를 공통 함수로 위임
if (inputType === 'deleteContentBackward') {
handleTimeDelete();
e.preventDefault();
return;
}
// ... 나머지 로직 ...
};
const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
if (type !== 'time') return;
if (e.nativeEvent.isComposing) return;
if (e.key === 'Backspace' || e.key === 'Delete') {
handleTimeDelete();
e.preventDefault();
}
};이렇게 하면 중복 제거와 함께 각 핸들러의 책임도 명확해집니다!
🧠 Learnings used
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/design-system/src/components/dateTime/DateTime.tsx(1 hunks)packages/design-system/src/components/dateTime/utils/DateTimeUtils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/src/components/dateTime/DateTime.tsx
🔇 Additional comments (2)
packages/design-system/src/components/dateTime/utils/DateTimeUtils.ts (2)
7-16: formatDate: 의도에 부합하는 단순 포맷터, LGTM최대 8자리 제한과 단계적 구분자 삽입(YYYY.MM.DD)이 명확하고 과입력 방지에도 적절합니다. 추가 검증/클램프는 상위 컨텍스트로 위임한다는 PR 결정과도 일치합니다.
2-4: tsconfig에서lib나target설정이 발견되지 않아, NFKC 정규화 지원 여부를 자동으로 판단할 수 없습니다.
상위 설정(extends) 혹은 기본값을 확인하여 사용 환경(브라우저/Node)에서String.prototype.normalize가 실제로 지원되는지 직접 검토해주세요.
| export function mapCaretByDigitsPos(digitsPos: number, kind: 'date' | 'time') { | ||
| if (kind === 'date') { | ||
| if (digitsPos <= 4) return digitsPos; | ||
| if (digitsPos <= 6) return digitsPos + 1; | ||
| return Math.min(digitsPos + 2, 10); | ||
| } else { | ||
| const PREFIX = 3; | ||
| if (digitsPos === 0) return PREFIX; | ||
| if (digitsPos <= 2) return PREFIX + 2; | ||
| return Math.min(PREFIX + 3 + (digitsPos - 2), PREFIX + 5); | ||
| } | ||
| } |
There was a problem hiding this comment.
mapCaretByDigitsPos: 날짜·시간 모두 오프바이원 버그로 커서가 구분자(., :) 위에 위치함
- 날짜: Line 46–49의 분기에서
digitsPos === 4일 때 4를 반환하여 커서가 첫 번째 점(.) 위에 위치합니다. 또한digitsPos === 6일 때 기대값 8(두 번째 점을 건너뜀) 대신 7을 반환합니다. 기대 매핑은 다음과 같습니다.- digitsPos → caret: 0→0, 1→1, 2→2, 3→3, 4→5, 5→6, 6→8, 7→9, 8→10
- 시간:
digitsPos === 1(시의 첫 자리 뒤)에서도 고정으로PREFIX + 2(시 두 자리 뒤)로 점프합니다. 또한digitsPos === 2(시 입력 완료) 시 콜론을 건너뛰어PREFIX + 3(콜론 뒤)로 가야 UX가 자연스럽습니다.
아래처럼 명시적 매핑으로 수정하면 경계값이 명확해지고 구분자를 안정적으로 건너뜁니다.
export function mapCaretByDigitsPos(digitsPos: number, kind: 'date' | 'time') {
- if (kind === 'date') {
- if (digitsPos <= 4) return digitsPos;
- if (digitsPos <= 6) return digitsPos + 1;
- return Math.min(digitsPos + 2, 10);
- } else {
- const PREFIX = 3;
- if (digitsPos === 0) return PREFIX;
- if (digitsPos <= 2) return PREFIX + 2;
- return Math.min(PREFIX + 3 + (digitsPos - 2), PREFIX + 5);
- }
+ const pos = Math.max(0, Math.floor(digitsPos));
+ if (kind === 'date') {
+ // YYYY.MM.DD (길이 10). 구분자(.)를 건너뛰는 명시 매핑
+ const map = [0, 1, 2, 3, 5, 6, 8, 9, 10]; // index: digitsPos 0..8
+ return map[Math.min(pos, 8)];
+ } else {
+ // '오전 ' / '오후 ' 모두 길이 3
+ const PREFIX = '오전 '.length; // 3
+ switch (Math.min(pos, 4)) {
+ case 0: return PREFIX; // AM/PM 뒤
+ case 1: return PREFIX + 1; // 시 첫 자리 뒤
+ case 2: return PREFIX + 3; // 시 두 자리 뒤 → 콜론(:) 건너뜀
+ case 3: return PREFIX + 4; // 분 첫 자리 뒤
+ default: return PREFIX + 5; // 분 두 자리 뒤(문자열 끝)
+ }
+ }
}추가 확인 포인트(선택):
- 실제 렌더 문자열 길이와 커서 상한을 동기화하기 위한 단위 테스트(예: vitest 기반 스냅숏) 추가를 권장합니다.
📝 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 function mapCaretByDigitsPos(digitsPos: number, kind: 'date' | 'time') { | |
| if (kind === 'date') { | |
| if (digitsPos <= 4) return digitsPos; | |
| if (digitsPos <= 6) return digitsPos + 1; | |
| return Math.min(digitsPos + 2, 10); | |
| } else { | |
| const PREFIX = 3; | |
| if (digitsPos === 0) return PREFIX; | |
| if (digitsPos <= 2) return PREFIX + 2; | |
| return Math.min(PREFIX + 3 + (digitsPos - 2), PREFIX + 5); | |
| } | |
| } | |
| export function mapCaretByDigitsPos(digitsPos: number, kind: 'date' | 'time') { | |
| const pos = Math.max(0, Math.floor(digitsPos)); | |
| if (kind === 'date') { | |
| // YYYY.MM.DD (길이 10). 구분자(.)를 건너뛰는 명시 매핑 | |
| const map = [0, 1, 2, 3, 5, 6, 8, 9, 10]; // index: digitsPos 0..8 | |
| return map[Math.min(pos, 8)]; | |
| } else { | |
| // '오전 ' / '오후 ' 모두 길이 3 | |
| const PREFIX = '오전 '.length; // 3 | |
| switch (Math.min(pos, 4)) { | |
| case 0: return PREFIX; // AM/PM 뒤 | |
| case 1: return PREFIX + 1; // 시 첫 자리 뒤 | |
| case 2: return PREFIX + 3; // 시 두 자리 뒤 → 콜론(:) 건너뜀 | |
| case 3: return PREFIX + 4; // 분 첫 자리 뒤 | |
| default: return PREFIX + 5; // 분 두 자리 뒤(문자열 끝) | |
| } | |
| } | |
| } |
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
어후.. 진짜 어려운 구현이셨을텐데... 보기만 해도 복잡한데 수고하셨습니다!
다만 조금 코드가 길어지고, 함수의 역할이 너무 많은 부분이 보여서 함수/모듈 분리 등을 더 신경 써주시면 좋을 것 같아요! 커스텀 훅을 적극 활용하는 것도 추천드립니다 :)
코멘트 확인해보시고 반영해주세요! 해당 로직이랑 컴포넌트는 계속해서 서로 고민해보고 리팩토링하면 좋을 것 같습니다 👍
| <span className={dateTimeLabelStyles({ state })}> | ||
| {type === 'date' ? '날짜' : '시간'} | ||
| </span> |
There was a problem hiding this comment.
해당 스타일에 줄바꿈 안되게 white-space: nowrap;을 줘야 할 것 같아요!
There was a problem hiding this comment.
사진과 같이 type이 시간인 경우에도 placeholder가 date로 뜨는 것 같아요! story 혹은 실제 구현 부도 이상이 있다면 같이 고쳐주세요 👍
| export function formatDate(d: string) { | ||
| const v = d.slice(0, 8); | ||
| const y = v.slice(0, 4); | ||
| const m = v.slice(4, 6); | ||
| const dd = v.slice(6, 8); | ||
| let out = y; | ||
| if (m) out += '.' + m; | ||
| if (dd) out += '.' + dd; | ||
| return out; | ||
| } |
There was a problem hiding this comment.
v, y같은 변수보다 value, year과 같이 변수명 신경써주시면 좋을 것 같아요! 가독성과 유지보수를 위해 :)
| const handleChange = (e: ChangeEvent<HTMLInputElement>) => { | ||
| if (type === 'time') return; |
There was a problem hiding this comment.
어차피 handleChange가 날짜에만 쓰인다면 함수 내부에서 분기처리하는 것이 아니라 외부에서 속성 존재 유무를 분기하는 것은 어떨까요??
예를 들어 위 코드의 경우는 type이 time인 경우도 onChange에 handleChange가 달려있어서 해당 함수가 일단 실행되고 early return 되는 형태에요. 물론 빠르게 return 돼서 성능 상 이슈는 없지만, 애초에 time의 경우에 해당 handleChange가 필요없다면 외부에서 분기 처리해서 없게 하면 어떨까하는 생각입니다!
onChange={type === 'date' ? handleChange : undefined}사실 이것보다도 해당 로직을 커스텀 훅으로 분리해서 내부에서 return을 조절하도록 하는 설계도 고민해보면 좋을 것 같아요!
There was a problem hiding this comment.
애초에 함수를 타지 않도록 해버리는 좋은 의견 감사합니다! 외부에서 분기 확인하도록 적용해두었습니다!
| const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { | ||
| if (type !== 'time') return; | ||
| if (e.nativeEvent.isComposing) return; | ||
|
|
||
| if (e.key === 'Backspace') { | ||
| if (timeDigits.length > 0) { | ||
| const next = timeDigits.slice(0, -1); | ||
| setTimeDigits(next); | ||
| setInput(formatTime12(next)); | ||
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | ||
| } | ||
| e.preventDefault(); | ||
| } | ||
|
|
||
| if (e.key === 'Delete') { | ||
| if (timeDigits.length > 0) { | ||
| const next = timeDigits.slice(0, -1); | ||
| setTimeDigits(next); | ||
| setInput(formatTime12(next)); | ||
| nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time'); | ||
| } | ||
| e.preventDefault(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
이 부분 확인해보니 중복되는 부분이 있는 것 같은데 한번 체크해주세요!
만약 중복으로 사용이 되어야 한다면 backspace 누르면 작동하는 함수를 따로 분리해서 재사용해도 좋을 것 같아요. 그리고 굳이 재사용을 안하더라도,
if (e.key === 'Backspace') {
if (timeDigits.length > 0) {
const next = timeDigits.slice(0, -1);
setTimeDigits(next);
setInput(formatTime12(next));
nextCaretRef.current = mapCaretByDigitsPos(next.length, 'time');
}
e.preventDefault();
}위와 같은 로직에 이름을 붙여서 함수로 분리해주면 가독성이 좋아질 것 같아요.
해당 handleKeyDown 함수가 너무 많은 책임/역할을 가지고 있는 것 같다고 생각이 듭니다!
| <input | ||
| ref={inputRef} | ||
| type="text" | ||
| className={dateTimeTxtStyles({ state })} | ||
| value={input} | ||
| onBeforeInput={handleBeforeInput} | ||
| onKeyDown={handleKeyDown} | ||
| onChange={handleChange} | ||
| placeholder={type === 'date' ? 'YYYY.MM.DD' : 'HH:MM'} | ||
| inputMode="numeric" | ||
| disabled={isDisabled} | ||
| maxLength={type === 'date' ? 10 : 8} | ||
| pattern={ | ||
| type === 'date' | ||
| ? '\\d{4}\\.\\d{2}\\.\\d{2}' | ||
| : '(오전|오후)\\s\\d{2}:\\d{2}' | ||
| } | ||
| aria-label={type === 'date' ? '날짜 입력' : '시간 입력'} | ||
| /> |
There was a problem hiding this comment.
하나의 input에서 type에 따라 너무 많은 분기처리가 필요하다면 애초에 위에서 return 하는 컴포넌트를 type에 따라 위에서 분기처리하는 방법도 좋을 것 같은데 어떻게 생각하시나요??
예시로
if (type === 'time') {
return <input/>; // time type에 맞는 interface 제공
}
if (type === 'date') {
return <input/>; // date type에 맞는 interface 제공
} There was a problem hiding this comment.
아하, 이렇게 하나의 태그 내에서 분기처리를 하는 게 더 좋을 지, 아예 조건문으로 두개 태그를 나눠서 구현에 적용하는 게 좋을 지 고민이 드네요! 제시해주신 대로 input 컴포넌트 ui를 나누게 되면
function DateInput({ inputRef, state, value, onChange }: any) {
return (
<input
ref={inputRef}
type="text"
className={dateTimeTxtStyles({ state })}
value={value}
onChange={onChange}
placeholder="YYYY.MM.DD"
inputMode="numeric"
disabled={state === 'disabled'}
maxLength={10}
pattern="\d{4}\.\d{2}\.\d{2}"
aria-label="날짜 입력"
/>
);
}
function TimeInput({
inputRef,
state,
value,
onBeforeInput,
onKeyDown,
}: any) {
return (
<input
ref={inputRef}
type="text"
className={dateTimeTxtStyles({ state })}
value={value}
onBeforeInput={onBeforeInput}
onKeyDown={onKeyDown}
placeholder="HH:MM"
inputMode="numeric"
disabled={state === 'disabled'}
maxLength={8}
pattern="(오전|오후)\s\d{2}:\d{2}"
aria-label="시간 입력"
/>
);
}
이렇게 분리 되면서
input이 공통으로 쓰는 스타일링(className, disabled 같은 속성)이 중복되거나, 공통적으로 다시 분리해서 관리해야하는 어려움에 대한 고민이 있습니다!
There was a problem hiding this comment.
너무 어려운 로직이고 분기 처리도 많아서 너무 고생하셨을 것 같아요...
잘 구현해주셨지만 꽤 코드가 길어져서 분리하는 것이 필요할 것 같아요! 커스텀 훅으로 상태랑 로직을 따로 빼서 비즈니스 로직은 커스텀 훅에, 해당 컴포넌트에는 UI요소를 많이 배치해서 선언적으로 리팩토링 해보는 것은 어떨까요??
안에서 type 분기 처리도 더 생각해서 설계해도 좋을 것 같네요 👍
| const [input, setInput] = useState(() => | ||
| type === 'date' ? formatDate(digitsOnly(value)) : formatTime12(timeDigits) | ||
| ); |
There was a problem hiding this comment.
state 초기 값에 함수 호출이 아니라 선언을 넣어주셔서, 첫 랜더링때만 한번 실행되는 효과를 볼 수 있네요! 굳굳 👍
음.. 작성하는 중간에 검사 판단이 조금 힘들다면 저장 버튼에 trigger가 되어도 좋지만, react hook form등 form 관리하는 라이브러리에서도 |
|
추가로 너무 잘 구현해주셨는데, 해당 formatting 등 제공해주는 라이브러리 코드를 까보면서 참고할 것이 있다면 같이 보면서 공부해도 좋을 것 같아요!! 아니면 Stack Overflow등에서 다른 개발자들이 구현한 코드 참고도 굿굿 👍 하나 찾은 라이브러리는 https://imask.js.org/ 요거 였어요! 더 많은 것 같아서 단순 참고 정도 ~~ |
3 participants
📌 Related Issues
✅ 체크 리스트
📄 Tasks
⭐ PR Point (To Reviewer)
type이 date, time에 따른 포맷팅 로직을 적용했어요.
시간일 경우엔, 17만 쳐도 오후 05:~ 로 실시간 포맷팅이 바로바로 적용되게 했어요. (데이트일경우엔 2025.08.19 자동)
👩🏻💻 포맷팅 로직 분기
각 date, time 타입에 맞춰서 포맷팅이 되는데, 이에 사용되는 이벤트 시점이 달라요!
###1. handleBeforeInput → 시간 입력 전용 (강제 포맷팅 & 커서 제어 필요)
###2. handleChange → 날짜 입력 전용 (입력 다 받은 후에 단순 포맷팅)
👩🏻💻 유효성 검사 고민
근데!! 포맷팅은 입력 실시간 중에 적용해도 자연스럽지만.. 유효성 검사는 어느 타이밍에 해야할 지 고민이 있었어요.
우리 추가적으로 유효성 검사 로직이 있는데
=> 유효성 검사 로직을 이 컴포넌트 작업에서 추가하려고 했는데, 이게 onChange 기준으로 검사하기엔 유저가 입력 한 자 한 자 할때마다 에러로 인지하게 돼서 ex. 2025 치려고 숫자 '2'만 쳐도 바로 에러 뜨는 등..
유효성 검사 삽입 타이밍이 애매해지더라고요..
그렇다고 인풋창 하나짜리인 컴포넌트라 버튼도 없고 외부 클릭할 영역도 없어서.. 입력 끝난 시점을 확인하기도 어려울 것 같았어요.
그래서, 이거는 추후에 팝업창 내에서 "저장" 버튼 클릭 시점에 이 각 입력창에 대한 유효성 검사를 그때 하는 게 좋을 것 같다고 생각했어요.!
혹시 다른 좋은 방향성이 있다면 꼭 공유해주세요!!
📷 Screenshot
Summary by CodeRabbit
새로운 기능
문서