Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough재사용 가능한 DatePicker UI(버튼, 팝오버, 캘린더, 커스텀 드롭다운) 컴포넌트들이 다수 추가되었고, Tailwind 텍스트 토큰명이 시맨틱 명칭으로 교체되었으며 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 사용자
participant Page as 페이지
participant DP as DatePicker
participant POP as Popover
participant CAL as Calendar
participant State as 페이지 상태
U->>Page: 페이지 방문
Page->>State: 초기 날짜 상태
Page->>DP: 렌더(value, onChange)
U->>DP: 팝오버 트리거 클릭
DP->>POP: 팝오버 열기 (open=true)
POP->>CAL: 캘린더 렌더
U->>CAL: 날짜 선택
CAL->>DP: 선택된 날짜 전달
DP->>State: onChange 호출 또는 내부 상태 업데이트
State->>Page: 리렌더(선택 반영)
DP->>POP: 팝오버 닫기 (open=false)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 근거: 새로운 UI 컴포넌트들이 여러 파일에 분산되어 있으며 접근성·키보드 처리, CVA 스타일링, Popover 클라이언트/동적 로딩 등 검토 포인트가 다수 존재함. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
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 |
|
🏷️ Labeler has automatically applied labels based on your PR title, branch name, or commit message. |
jjangminii
changed the title
![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 (9)
src/pages/index.tsx (1)
6-13: i18n/Hydration 관점의 날짜 표기 정규화 제안
toLocaleDateString()기본 로케일은 환경에 따라 달라져(서버/클라이언트) 미세한 표기 차이로 수화 경고가 날 수 있습니다. 한국어 서비스 기준이면 고정 로케일을 지정하세요.예:
- {selected ? selected.toLocaleDateString() : 'Select date'} + {selected ? selected.toLocaleDateString('ko-KR') : 'Select date'}또는
date-fns/format을 사용해 포맷을 명시하는 것도 좋습니다.로컬에서 선택 후 새로고침 시 경고 유무(콘솔)만 확인 부탁드립니다.
src/shared/components/datePicker/button.tsx (1)
12-14: cva variants 확장 여지(선택 사항)
ghost외에 hover/active/disabled 상태를 cva의 compoundVariants로 정의하면 캘린더 버튼/트리거의 상태 일관성 유지가 수월합니다. 추후 디자인 토큰 확정 시 반영 추천.src/shared/components/datePicker/popover.tsx (1)
24-37: PopoverContent 기본 표면(배경/테두리/라운드) 부여 고려현재 Content에는 애니메이션/텍스트색만 있고 배경/테두리가 없어 내부
Calendar가 표면 역할을 합니다. 재사용성을 위해 Content 자체에 최소 표면(예:bg-popover border rounded-md)을 두는 것을 권장합니다.src/shared/components/datePicker/DatePicker.tsx (3)
41-46: 표시 날짜 포맷 고정화(선택 사항)수화/로케일 차이를 줄이려면 로케일을 명시하거나
date-fns/format을 사용하세요.- {selected ? selected.toLocaleDateString() : 'Select date'} + {selected ? selected.toLocaleDateString('ko-KR') : 'Select date'}
49-59: 연/월 드롭다운 범위 확인
fromYear={new Date().getFullYear()}로 과거 연도 선택이 막힙니다. 요구사항이 “미래 일정 선택”이면 OK, 아니라면fromYear를 넉넉히 낮추거나fromMonth/toMonth로 UX를 조정하세요.
49-60: 모바일 폭 대응
PopoverContent와Calendar가 고정폭(w-[24rem])이라 모바일에서 오버플로우 가능성이 있습니다.max-w-[90vw] sm:w-[24rem]등 반응형 폭을 권장합니다.예:
- <PopoverContent className='w-[24rem] overflow-hidden p-0' align='start'> + <PopoverContent className='w-[24rem] max-w-[90vw] overflow-hidden p-0' align='start'> ... - className='w-[24rem] h-auto p-3 [--cell-size:2.8rem]' + className='w-[24rem] max-w-[90vw] h-auto p-3 [--cell-size:2.8rem]'src/shared/components/datePicker/calendar.tsx (3)
47-56: Nav 버튼 크기/상태: 접근성/터치 영역 확인
size-(--cell-size)와aria-disabled:opacity-50 p-0는 깔끔하지만 모바일 터치 영역(최소 40px) 보장을 확인해 주세요. 필요 시min-w/min-h를 추가하세요.간단 확인: 실제 렌더 크기 측정 후 40×40px 이상인지 DevTools에서 확인.
Also applies to: 52-56
131-142: Chevron 컴포넌트: className 미적용 및 RTL 로직 중복 가능성
- 전달된
className이 전혀 사용되지 않습니다(스타일 확장성 제한).orientation === 'right'에서 180도 회전 + 상단의rtl:**:[.rdp-button_next>svg]:rotate-180가 함께 적용되면 RTL에서 중복 회전 가능성이 있습니다. 실제 RTL 모드로 확인 필요.수정 예:
- Chevron: ({ className, orientation, ...props }) => { + Chevron: ({ className, orientation, ...props }) => { if (orientation === 'left') { - return <Icon name={'Caret'} color='pink-800' size={16} />; + return <Icon name='Caret' color='pink-800' size={16} className={className} />; } else if (orientation === 'right') { - return ( - <Icon name={'Caret'} rotate={180} color='pink-800' size={16} /> - ); + return ( + <Icon name='Caret' rotate={180} color='pink-800' size={16} className={className} /> + ); } - return ( - <Icon name={'Caret'} rotate={270} color='pink-800' size={12} /> - ); + return <Icon name='Caret' rotate={270} color='pink-800' size={12} className={className} />; },
31-35: 월 드롭다운 포맷터 로케일
formatMonthDropdown이default로케일을 사용합니다. 한국어 월 표기를 원하면toLocaleString('ko-KR', { month: 'short' })로 고정하거나 DayPicker의locale을 전달하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.json(1 hunks)src/pages/index.tsx(1 hunks)src/shared/components/datePicker/DatePicker.tsx(1 hunks)src/shared/components/datePicker/button.tsx(1 hunks)src/shared/components/datePicker/calendar.tsx(1 hunks)src/shared/components/datePicker/popover.tsx(1 hunks)src/shared/lib/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/shared/components/datePicker/calendar.tsx (2)
src/shared/components/datePicker/button.tsx (2)
Button(42-42)buttonVariants(42-42)src/shared/lib/utils.ts (1)
cn(71-73)
src/pages/index.tsx (2)
src/shared/components/datePicker/DatePicker.tsx (1)
DatePicker(16-64)src/shared/icons/components/icon.tsx (1)
Icon(54-101)
src/shared/components/datePicker/button.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/popover.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/DatePicker.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
🔇 Additional comments (2)
src/shared/components/datePicker/calendar.tsx (1)
168-176: 포커스 이동 로직은 Button forwardRef에 의존합니다
modifiers.focused시 포커스 설정은 적절합니다. 다만 Button이 forwardRef로 수정되기 전까지는 동작하지 않습니다(별도 코멘트 참조). Button 수정 후 다시 확인 부탁드립니다.src/shared/components/datePicker/popover.tsx (1)
31-33: Tailwind v4origin-(--var)구문은 유효함 - 우려 해소Tailwind v4에서
utility-(--custom-prop)형태는 공식 지원되는 shorthand로, 자동으로var()로 래핑됨. 검증 결과:
- 구문 확인:
origin-(--radix-popover-content-transform-origin)(라인 31)는 정상 작동- 규칙 일관성:
size-(--cell-size),h-(--cell-size),w-(--cell-size),px-(--cell-size)등이 프로젝트 전체에서 일관되게 사용 중- 버전 호환: Tailwind 4.1.14에서 공식 지원하는 구문
- 대안 불필요:
origin-[var(--radix-popover-content-transform-origin)]형태로 변경할 이유 없음제안된 리팩토링 불필요. 현재 구현이 Tailwind v4 모범 사례에 부합합니다.
package.json
Outdated
| "class-variance-authority": "^0.7.1", | ||
| "@radix-ui/react-popover": "^1.1.15", | ||
| "date-fns": "^4.1.0", | ||
| "react-day-picker": "^9.11.1", | ||
| "@radix-ui/react-slot": "^1.2.3", | ||
| "lucide-react": "^0.540.0" |
There was a problem hiding this comment.
🧩 Analysis chain
런타임 의존성이 devDependencies에 들어가 있어 배포 시 누락 위험
@radix-ui/react-popover, @radix-ui/react-slot, react-day-picker, date-fns, class-variance-authority, lucide-react는 런타임에 import되는 UI/유틸입니다. prod 설치(예: CI/CD에서 --prod, Docker multi-stage)에서는 devDependencies가 제외되어 앱이 빌드/실행 중 터질 수 있습니다. 또한 반대로 svgo는 빌드 스크립트용이므로 devDependencies가 적절합니다.
권장 수정:
- 위 6개 패키지를 dependencies로 이동
svgo는 devDependencies로 이동
"dependencies": {
"axios": "^1.12.2",
"clsx": "^2.1.1",
"next": "15.5.4",
"react": "19.1.0",
"react-dom": "19.1.0",
"react-hook-form": "^7.64.0",
- "svgo": "^4.0.0",
- "tailwind-merge": "^3.3.1"
+ "tailwind-merge": "^3.3.1",
+ "class-variance-authority": "^0.7.1",
+ "@radix-ui/react-popover": "^1.1.15",
+ "@radix-ui/react-slot": "^1.2.3",
+ "react-day-picker": "^9.11.1",
+ "date-fns": "^4.1.0",
+ "lucide-react": "^0.540.0"
},
"devDependencies": {
+ "svgo": "^4.0.0",
...
- "class-variance-authority": "^0.7.1",
- "@radix-ui/react-popover": "^1.1.15",
- "date-fns": "^4.1.0",
- "react-day-picker": "^9.11.1",
- "@radix-ui/react-slot": "^1.2.3",
- "lucide-react": "^0.540.0"
}의존성 위치를 점검하는 빠른 확인 스크립트입니다(런타임 import되는 패키지가 devDependencies에 남아있는지 검사).
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 런타임 import 탐지 =="
rg -nP --type=ts --type=tsx -C0 \
-e "from\s+'@radix-ui/react-popover'" \
-e "from\s+'@radix-ui/react-slot'" \
-e "from\s+'react-day-picker'" \
-e "from\s+'date-fns'" \
-e "from\s+'class-variance-authority'" \
-e "from\s+'lucide-react'"
echo
echo "== package.json devDependencies 위치 검사 =="
jq -r '.devDependencies | keys[]?' package.json | \
rg -n "^(class-variance-authority|@radix-ui/react-popover|@radix-ui/react-slot|react-day-picker|date-fns|lucide-react)$" || trueLength of output: 47
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== package.json 현재 상태 확인 =="
cat package.json | jq '.dependencies, .devDependencies' | head -60
echo
echo "== 런타임 import 탐지 (6개 패키지) =="
rg -nP --type=ts -C0 \
-e "from\s+'@radix-ui/react-popover'" \
-e "from\s+'@radix-ui/react-slot'" \
-e "from\s+'react-day-picker'" \
-e "from\s+'date-fns'" \
-e "from\s+'class-variance-authority'" \
-e "from\s+'lucide-react'" || echo "No matches found"
echo
echo "== svgo 빌드 스크립트 사용 확인 =="
cat package.json | jq '.scripts'
echo
echo "== svgo 코드 사용 확인 =="
rg -nP "svgo" --type=ts -C2 || echo "No svgo usage found in TypeScript files"
Length of output: 72
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json 현재 상태 확인 =="
jq '.dependencies, .devDependencies' package.json 2>/dev/null || echo "package.json not found"
echo
echo "== 런타임 import 탐지 (6개 패키지) =="
echo "Searching for runtime imports..."
rg -nP "from\s+['\"](@radix-ui/react-popover|@radix-ui/react-slot|react-day-picker|date-fns|class-variance-authority|lucide-react)['\"]" \
--type=ts --type=js || echo "No matches found"
echo
echo "== package.json 스크립트 확인 =="
jq '.scripts' package.json
echo
echo "== svgo 사용 패턴 확인 =="
rg -n "svgo" --type=ts --type=js -C2 || echo "No svgo usage found"
echo
echo "== devDependencies 상세 확인 =="
jq '.devDependencies | keys | map(select(. == "class-variance-authority" or . == "@radix-ui/react-popover" or . == "@radix-ui/react-slot" or . == "react-day-picker" or . == "date-fns" or . == "lucide-react"))' package.jsonLength of output: 1810
런타임 의존성이 devDependencies에 있어 프로덕션 배포 시 누락 위험 – 즉시 수정 필요
검증 결과 @radix-ui/react-popover, @radix-ui/react-slot, class-variance-authority, react-day-picker는 src/shared/components/datePicker/ 파일들에서 런타임에 직접 import되고 있습니다. --prod 플래그나 Docker multi-stage 빌드에서는 devDependencies가 제외되어 앱 실행 중 모듈을 찾을 수 없는 오류가 발생합니다.
동시에 svgo는 package.json 스크립트에서만 사용되므로(build 시 icons:clean 실행) 현재 dependencies에 있는 것이 잘못되었습니다.
필수 수정:
- 6개 패키지를 dependencies로 이동
svgo를 devDependencies로 이동
"dependencies": {
"axios": "^1.12.2",
"clsx": "^2.1.1",
"next": "15.5.4",
"react": "19.1.0",
"react-dom": "19.1.0",
"react-hook-form": "^7.64.0",
- "svgo": "^4.0.0",
- "tailwind-merge": "^3.3.1"
+ "tailwind-merge": "^3.3.1",
+ "class-variance-authority": "^0.7.1",
+ "@radix-ui/react-popover": "^1.1.15",
+ "@radix-ui/react-slot": "^1.2.3",
+ "react-day-picker": "^9.11.1",
+ "date-fns": "^4.1.0",
+ "lucide-react": "^0.540.0"
},
"devDependencies": {
+ "svgo": "^4.0.0",
"@eslint/eslintrc": "^3",
...
- "class-variance-authority": "^0.7.1",
- "@radix-ui/react-popover": "^1.1.15",
- "date-fns": "^4.1.0",
- "react-day-picker": "^9.11.1",
- "@radix-ui/react-slot": "^1.2.3",
- "lucide-react": "^0.540.0"
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In package.json around lines 37 to 42, several runtime libraries are incorrectly
listed as devDependencies which will be omitted in production builds and break
imports from src/shared/components/datePicker; move these
packages—class-variance-authority, @radix-ui/react-popover, date-fns,
react-day-picker, @radix-ui/react-slot, lucide-react—into "dependencies", and
move svgo from "dependencies" into "devDependencies"; update package.json
accordingly and run your package manager (npm/yarn/pnpm) to update the lockfile
so production installs include the runtime libs.
| function Button({ | ||
| className, | ||
| variant, | ||
| asChild = false, | ||
| ...props | ||
| }: React.ComponentProps<'button'> & | ||
| VariantProps<typeof buttonVariants> & { | ||
| asChild?: boolean; | ||
| }) { | ||
| const Comp = asChild ? Slot : 'button'; | ||
|
|
||
| return ( | ||
| <Comp | ||
| data-slot='button' | ||
| className={cn(buttonVariants({ variant, className }))} | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
ref 미전달로 Calendar의 포커스 관리가 깨질 수 있습니다
CalendarDayButton에서 ref.current?.focus()를 호출하지만, 현재 Button이 forwardRef가 아니어서 실제 DOM button에 ref가 연결되지 않습니다. 키보드 내비게이션 시 포커스 이동이 동작하지 않을 수 있습니다.
수정(diff, 타입 포함):
-import { cva, type VariantProps } from 'class-variance-authority';
+import { cva, type VariantProps } from 'class-variance-authority';
type Props =
React.ComponentProps<'button'> &
VariantProps<typeof buttonVariants> & {
asChild?: boolean;
};
-function Button({
- className,
- variant,
- asChild = false,
- ...props
-}: React.ComponentProps<'button'> &
- VariantProps<typeof buttonVariants> & {
- asChild?: boolean;
- }) {
- const Comp = asChild ? Slot : 'button';
-
- return (
- <Comp
- data-slot='button'
- className={cn(buttonVariants({ variant, className }))}
- {...props}
- />
- );
-}
+const Button = React.forwardRef<HTMLButtonElement, Props>(function Button(
+ { className, variant, asChild = false, ...props },
+ ref,
+) {
+ const Comp = asChild ? Slot : 'button';
+ return (
+ <Comp
+ data-slot='button'
+ ref={ref}
+ className={cn(buttonVariants({ variant }), className)}
+ {...props}
+ />
+ );
+});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/shared/components/datePicker/button.tsx around lines 22 to 40, the Button
component isn’t forwarding refs so CalendarDayButton’s ref.current?.focus()
never reaches the real DOM button; change Button to use
React.forwardRef<HTMLButtonElement, React.ComponentProps<'button'> &
VariantProps<typeof buttonVariants> & { asChild?: boolean }> and accept (props,
ref) => { const Comp = asChild ? Slot : 'button'; return <Comp ref={ref}
data-slot='button' className={cn(buttonVariants({ variant, className }))}
{...props} />; }; export the forwarded component (set displayName if desired) so
callers receive the actual button DOM ref for proper focus management.
| 'relative rounded-md bg-color-pink', | ||
| defaultClassNames.dropdown_root, | ||
| ), | ||
| dropdown: cn( | ||
| 'absolute bg-popover inset-0 opacity-0 text-label-lg text-pink-800', | ||
| defaultClassNames.dropdown, |
There was a problem hiding this comment.
bg-color-pink는 Tailwind 클래스가 아닙니다
dropdown_root에 bg-color-pink가 있어 컴파일되지 않을 가능성이 큽니다. 디자인 토큰을 쓰려면 CSS 변수 기반 임의값을 사용하세요.
수정 예(토큰이 --color-pink라 가정):
- dropdown_root: cn(
- 'relative rounded-md bg-color-pink',
- defaultClassNames.dropdown_root,
- ),
+ dropdown_root: cn(
+ 'relative rounded-md bg-[var(--color-pink)]',
+ defaultClassNames.dropdown_root,
+ ),📝 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.
| 'relative rounded-md bg-color-pink', | |
| defaultClassNames.dropdown_root, | |
| ), | |
| dropdown: cn( | |
| 'absolute bg-popover inset-0 opacity-0 text-label-lg text-pink-800', | |
| defaultClassNames.dropdown, | |
| 'relative rounded-md bg-[var(--color-pink)]', | |
| defaultClassNames.dropdown_root, | |
| ), | |
| dropdown: cn( | |
| 'absolute bg-popover inset-0 opacity-0 text-label-lg text-pink-800', | |
| defaultClassNames.dropdown, |
🤖 Prompt for AI Agents
In src/shared/components/datePicker/calendar.tsx around lines 66 to 71, the
class "bg-color-pink" is not a valid Tailwind class and will break compilation;
remove that token and replace it with a Tailwind arbitrary value that uses the
CSS variable (e.g. bg-[color:var(--color-pink)]) or set the background via
inline style/backgroundColor: 'var(--color-pink)' so the design token
(--color-pink) is applied correctly; update the dropdown_root class list
accordingly.
There was a problem hiding this comment.
우리 디자인 시스템 컬러 토큰이에요 오타가 있었네요 수정했습니다
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Tailwind configuration file
fd -t f "tailwind.config" --max-depth 3
# Search for color-pink definitions and usage
rg -n "color-pink" --type-add 'config:*.{js,ts,cjs,mjs}' --type config
# Check how bg- color classes are defined in the codebase
rg -n "bg-.*pink" -A 2 -B 2 | head -50
# Look at the full utils.ts file to see tailwind-merge config
cat src/shared/lib/utils.ts
Length of output: 4470
🏁 Script executed:
#!/bin/bash
# Search for --color-pink CSS variable definition (base color without shade)
rg -n "color-pink[^-]|color-pink\s*:|color-pink\)" src/styles/
# Check if bg-color-pink is defined anywhere as a utility
rg -n "bg-color-pink" src/styles/
# Look at the complete color variable definitions in globals.css
rg -n "^\s*--color-pink" src/styles/globals.css | head -20
Length of output: 318
🏁 Script executed:
#!/bin/bash
# Check what background colors are used for similar dropdown/popover elements
rg -n "dropdown|popover" src/shared/components/datePicker/ -A 3 -B 1 | rg "bg-"
# See the complete utility class definitions for bg-pink
rg -n "@utility bg-pink" src/styles/globals.css
Length of output: 1022
@jjangminii 코드베이스를 확인해봤는데, bg-color-pink 클래스가 실제로 정의되어 있지 않은 것 같습니다.
현재 디자인 시스템에 정의된 pink 색상 유틸리티:
globals.css에는bg-pink-50~bg-pink-900만 정의되어 있습니다- CSS 변수도
--color-pink-50~--color-pink-900만 존재합니다 - 기본
--color-pink변수는 정의되지 않았습니다
다른 곳에서 사용된 예시:
// DatePicker.tsx Line 42
className='... bg-pink-50 border-pink-100 text-pink-300'
// calendar.tsx Line 25
className='bg-gray-50 border-pink-100'
// calendar.tsx Line 70
className='... bg-popover text-pink-800'Line 66의 bg-color-pink를 어떤 shade로 변경하시면 될까요? (예: bg-pink-50, bg-pink-100 등)
| 'display-lg', | ||
| 'display-md', | ||
| 'display-sm', | ||
| 'headline-lg', | ||
| 'headline-md', | ||
| 'headline-sm', | ||
| 'title-lg', | ||
| 'title-md', | ||
| 'title-sm', | ||
| 'label-lg', | ||
| 'label-md', | ||
| 'label-sm', | ||
| 'body-lg', | ||
| 'body-md', | ||
| 'body-sm', | ||
| 'display-serif', | ||
| 'headline-lg-serif', | ||
| 'headline-md-serif', | ||
| 'headline-sm-serif', | ||
| 'label-serif', | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
twMerge 확장: 현재 방식은 충돌을 피하지만, 장기적으로는 그룹 분리를 고려하세요
theme.text에 타이포그래피 토큰(display-lg, title-sm 등)을 등록하고 색상 토큰은 theme.color로 분리해 text-label-*와 text-pink-*의 병합 충돌을 우회한 점은 단기적으로 유효합니다. 다만 이 접근은:
- twMerge가 해당 타이포그래피 클래스를 “알 수 없는 클래스”로 취급해, 중복/우선순위 정리가 전혀 되지 않습니다.
- 팀 확장 시 예기치 않은 잔여 클래스가 누적될 수 있습니다.
권장:
- twMerge의
classGroups를 사용해 “타이포그래피(text-size)”와 “텍스트 색상(text-color)”를 별도 그룹으로 명시적으로 분리해 충돌 테이블을 정의. - 혹은 타이포그래피 접두사를
text-가 아닌 디자인시스템 전용 접두사로 교체(예:ty-)하여 의미적 충돌 자체를 제거.
원하시면 현재 토큰 세트를 기준으로 extendTailwindMerge 구성 예시를 드릴게요.
타이포그래피/색상 그룹 분리 관련 최신 예제를 확인해 주세요:
🌐 Web query:
tailwind-merge classGroups로 text-color와 font-size를 분리하는 권장 구성 예제가 있는지?
💡 Result:
네—권장하는 방식은 tailwind-merge의 classGroups를 이용해 "텍스트 색상"과 "폰트 크기"를 서로 다른 그룹으로 분리하고, 필요하면 서로 충돌하지 않도록 conflictingClassGroups를 설정하는 것입니다. 예시는 아래와 같습니다.
코드 예시 (JavaScript/TypeScript):
import { extendTailwindMerge } from 'tailwind-merge'
import tailwindConfig from './tailwind.config.js' // 또는 theme 직접 참조
const { theme } = tailwindConfig
const twMerge = extendTailwindMerge({
theme: {
// 필요시 theme의 일부를 병합
fontSize: Object.keys(theme.fontSize ?? {}),
colors: Object.keys(theme.colors ?? {}),
},
classGroups: {
// font-size 그룹: text-<size> (ex: text-sm, text-lg, text-body(커스텀) 등)
'font-size': [{ text: Object.keys(theme.fontSize ?? {}) }],
// text-color 그룹: text-<color>, text-<color>-500 등
'text-color': [{ text: Object.keys(theme.colors ?? {}) }],
// (추가로 다른 text- 접두어 커스텀들이 있다면 여기 명시)
// 예: 'text-custom': ['text-body', 'text-foo'],
},
// text-* 클래스 접두어가 서로 충돌한다고 인식될 때 그룹 간 충돌을 명시적으로 분리
conflictingClassGroups: {
'font-size': [], // font-size가 다른 그룹과 충돌 안 나게
'text-color': [], // 필요하면 서로 충돌로 안 묶기
// 또는 'font-size': ['text-color'] 와 같이 충돌 관계를 정의할 수 있음
},
})
export { twMerge }핵심 포인트
- tailwind-merge는 기본적으로 클래스 접두사(text-)를 동일 그룹으로 처리하므로, 커스텀 유틸(예: text-body)을 폰트사이즈로 취급하려면 classGroups에 명시적으로 추가해야 의도치 않은 제거를 방지합니다. [1][2]
- tailwind-variants 등 라이브러리 예제에서도 theme 값을 읽어 classGroups에 병합하는 패턴을 사용합니다. [3]
- 한국어 예제 및 설명(문제 사례와 해결법)은 블로그/기술문서에서 실무 예시로 잘 정리되어 있습니다. [4]
참고자료
[1] tailwind-merge 공식 리포지토리 및 README.
[2] tailwind-merge NPM/문서 (classGroups 설명).
[3] tailwind-variants config 예시 (classGroups 병합 패턴).
[4] 실무 블로그(예: Zenn) — text-body 같은 커스텀 클래스가 색상으로 오인되어 제거되는 문제와 classGroups로 해결하는 예.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and display the full utils.ts file
fd -e ts -p "utils\.ts" --type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'
Length of output: 1490
🏁 Script executed:
#!/bin/bash
# Find tailwind config files
fd -e "(js|ts)" -p "tailwind\.config\.(js|ts)" --type f | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for extendTailwindMerge usage
rg -n "extendTailwindMerge|twMerge|classGroups" --type ts --type js -A 10 -B 2Length of output: 897
🏁 Script executed:
#!/bin/bash
# Search for theme configuration related to text, typography, colors
rg -n "theme\.(text|color|fontSize)" --type ts --type js -C 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check package.json for tailwind-merge version
fd package.json -x cat | jq '.dependencies."tailwind-merge", .devDependencies."tailwind-merge"'Length of output: 13
---
🌐 Web query:
```web
tailwind-merge 3.3.1 extend.theme API documentation
💡 Result:
Summary — extendTailwindMerge: theme API (tailwind-merge v3.3.1)
-
Use extendTailwindMerge to extend the default tailwind-merge config (returns a custom twMerge function). Pass a configExtension object with an optional theme key: extendTailwindMerge({ theme: { : [/* keys */] } }). Example: spacing: ['sm','md','lg']. [1][2]
-
Supported theme scales (only scales used across multiple class groups are supported by default): colors, spacing, blur, brightness, borderColor, borderRadius, borderSpacing, borderWidth, contrast, grayscale, hueRotate, invert, gap, gradientColorStops, inset, margin, opacity, padding, saturate, scale, sepia, skew, space, translate. Add your keys under the matching scale. [1][2]
-
fromTheme(key) helper: use fromTheme('my-scale') inside classGroups definitions to reference values from your theme scale (it returns a theme getter used internally). Example in classGroups: { 'my-group': [{ 'my-group': [fromTheme('my-scale'), fromTheme('spacing')] }] }. [1]
-
If you need more control or want a fully custom config, use createTailwindMerge (instead of extendTailwindMerge) to build from scratch. You can also pass plugin-style createConfig functions to extendTailwindMerge. [1][2]
Sources:
[1] tailwind-merge v3.3.1 (npm) — package readme / API reference.
[2] tailwind-merge GitHub README (configuration & usage).
🌐 Web query:
tailwind-merge classGroups vs extend theme which is correct
💡 Result:
Short answer: use classGroups (extendTailwindMerge) to teach tailwind-merge about custom classes; use theme.extend only to add classes to Tailwind at build time. tailwind-merge does not automatically read your Tailwind config at runtime, so configuring its classGroups (or theme keys it supports) is the correct way to make twMerge treat your custom utilities as conflicting the same way built‑ins are. [1][2][3]
Sources:
- tailwind-merge docs: configuration, classGroups and theme behavior. [1]
- tailwind-merge GitHub / npm README (notes about runtime config vs Tailwind build config). [2]
- Community note showing breakage and the fix: add custom classes to classGroups via extendTailwindMerge. [3]
🌐 Web query:
tailwind-merge text- prefix conflict font-size color
💡 Result:
Short answer — Why it happens
- tailwind-merge treats both font-size (e.g. text-lg, text-[...length]) and text-color (e.g. text-red-500, text-white) as members of different "class groups" that share the same prefix "text-". By default tw‑merge will consider two classes with the same prefix conflicting and remove one, so custom/ambiguous text-* classes (like text-button or arbitrary values) can get dropped. [1][2][3]
How to fix it (pick one)
-
Configure tailwind-merge to understand your custom classes (recommended)
- Extend the default config / classGroups so font-size classes and color classes are distinct (add your custom text-* sizes or colors). Use createTailwindMerge/extendTailwindMerge and update classGroups or theme keys. [1][4]
-
Disambiguate arbitrary values with CSS data-type labels
- Use Tailwind’s datatype labels inside arbitrary values so tw‑merge can tell length vs color: e.g. text-[length:theme(myScale.foo)] vs text-[color:var(--c)]. [2]
-
Force a class to win with "!" (important)
- Prefix a class with "!" so twMerge preserves it: twMerge('text-black', '!text-xxs'). Useful as a quick workaround. [5]
-
Use a Tailwind prefix (if appropriate)
- If you globally prefix Tailwind (e.g. prefix: 'tw-'), tw‑merge can be configured to use that prefix too; this avoids collisions with non-Tailwind class names. [6]
Pointers / links
- tailwind-merge behavior & config details. [1]
- Ambiguous arbitrary-value handling and data-type labels. [2]
- GitHub issue showing this exact text- prefix problem. [3]
- Examples of extending twMerge config / extendTailwindMerge. [4][5]
If you want, tell me:
- the exact classes (e.g. text-button + text-black) and whether any are arbitrary/custom, and I’ll produce the exact extend/createTailwindMerge config you can drop into your project.
🌐 Web query:
tailwind-merge apply theme typography custom classes
💡 Result:
Short answer — yes. Use tailwind-merge (twMerge) to merge runtime class strings and configure it so it understands your custom typography/prose classes and any theme overrides. Steps:
-
Create/extend a twMerge config so it knows your custom class groups and theme keys (use extendTailwindMerge or createTailwindMerge). Add any custom prose modifier names (e.g. prose-brand, wysiwyg-*) and any custom theme tokens you added to Tailwind (colors/spacing/etc.). Example API references: extendTailwindMerge/createTailwindMerge. [1][2]
-
Define your typography custom themes in Tailwind (tailwind.config.js or CSS-first v4). Use theme.extend.typography to add a named theme that sets --tw-prose-* vars (or use the plugin’s className option to rename prose). Docs show using typography: ({ theme }) => ({ mytheme: { css: { '--tw-prose-body': theme('colors.brand.700'), ... } } }). [3][4]
-
If you use Tailwind v4 CSS-first, include @plugin and (if needed) @config in your main CSS so custom themes/classes are emitted. Otherwise configure via tailwind.config.js as usual. [3][4]
-
Use twMerge at runtime to combine classes safely. If you added custom prose modifier names or custom theme tokens, register them in your twMerge config so twMerge resolves conflicts (e.g. prose, prose-brand, wysiwyg, custom color keys). Example:
- extendTailwindMerge({ theme: { spacing: [...], colors: [...] }, classGroups: { prose: ['prose', 'prose-\w+'], /* ... */ } }) then use the returned merge function. See docs for patterns and examples. [1][2]
References
- tailwind-merge usage, extend/create config and theme mapping. [1][2]
- Tailwind Typography plugin: adding custom color/themes and className option; CSS-first v4 notes (use @plugin / @config). [3][4]
If you want, tell me: (A) your Tailwind version (v3 vs v4), (B) your custom prose class names or theme tokens, and I’ll give the exact extendTailwindMerge config + sample code.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search tailwind-merge documentation to see if arbitrary theme keys are mentioned anywhere
rg -i "arbitrary.*theme|custom.*scale|unknown|text.*scale|color.*scale" --type md --type txt 2>/dev/null || echo "No markdown/txt files found"
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if there are any TypeScript definitions or type hints in node_modules
fd -e d.ts "tailwind-merge" 2>/dev/null | head -3 || echo "Checking inline JSDoc..."
# Alternative: look for JSDoc comments about theme in the utils file
rg -n "theme|classGroups|conflictingClassGroups" src/shared/lib/utils.tsLength of output: 14
🏁 Script executed:
#!/bin/bash
# Verify: does the current code actually work by checking test files or usage
fd -e spec.ts -e test.ts -e ".test.ts" --type f -x grep -l "cn\|twMerge" {} \; 2>/dev/null | head -3Length of output: 0
extend.theme의 임의 키(text, color)는 tailwind-merge에서 인식되지 않습니다. classGroups로 교체 필요
확인 결과, 현재 코드는 tailwind-merge v3.3.1의 문서화된 API와 맞지 않습니다.
extend.theme는colors,spacing,blur등 사전 정의된 Tailwind 스케일만 지원합니다text,color같은 임의 키는 무시되어,display-lg,title-sm등 타이포그래피 클래스가 twMerge에 등록되지 않음- 결과적으로 이 클래스들은 "알 수 없는 클래스"로 취급되며, 의도치 않은 중복/제거 문제 발생 가능
수정 방안:
const twMerge = extendTailwindMerge({
classGroups: {
'font-size': [
{ text: ['display-lg', 'display-md', 'display-sm', 'headline-lg', 'headline-md', 'headline-sm', 'title-lg', 'title-md', 'title-sm', 'label-lg', 'label-md', 'label-sm', 'body-lg', 'body-md', 'body-sm', 'display-serif', 'headline-lg-serif', 'headline-md-serif', 'headline-sm-serif', 'label-serif'] }
],
'text-color': [
{ text: ['mint-50', 'mint-100', /* ... 색상 토큰 */, 'background', 'foreground'] }
]
}
});🤖 Prompt for AI Agents
In src/shared/lib/utils.ts around lines 8 to 28, the current use of extend.theme
with arbitrary keys like "text" and "color" is ignored by tailwind-merge v3.3.1;
replace that config by calling extendTailwindMerge and adding a classGroups map
that explicitly registers your custom typography tokens (e.g., display-lg,
display-md, …, label-serif) under a logical group name (e.g., "font-size") and
your color tokens (e.g., mint-50, mint-100, background, foreground) under
another group name (e.g., "text-color"); ensure you remove the unsupported
extend.theme entries, use the extendTailwindMerge API to define classGroups with
objects/arrays listing the custom class names, and export/use the resulting
twMerge so those custom classes are recognized and deduped by tailwind-merge.
KongMezu
left a comment
KongMezu
left a comment
There was a problem hiding this comment.
데이트 피커 진짜..고생많으셨습니다. 겸사겸사 PR내용에서 언급한 twmerge 관련 코멘트도 남겨놨으니 한번 읽어보심 좋을거 같습니다 :)
| theme: { | ||
| text: [ |
There was a problem hiding this comment.
twmerge 관련해서 조금 찾아보니까 저희 같은 고충 겪으신 분들이 많더라고요 :)
보통 공식문서랑 다른 블로그들 권장 방법은
extendTailwindMerge에서 classGroups를 분리해 text-color, text-size를 별도 그룹으로 정의해서 사용하는게 근본적인 해결방법이라고 하네요
지금 유틸에서는
theme 확장이지, classGroups가 없어요
extend: {
theme: {
// 유지
},
classGroups: {
'text-color': [
'text-mint-50', 'text-mint-100',
],
'text-size': [
'text-label-lg',
],
이런식으로 해결할 수 있다고 하네요
저도 찾아본 링크 같이 남겨둘게요 :)
충돌이나 클래스 순서 바뀔 때 안정성때문에서라도
참고해서 바꿔보는게 좋을거 같아요!
There was a problem hiding this comment.
이 부분은 좀 더 공부해본 뒤 따로 이슈 파서 작업하겠습니다-!! 감사합니다
| showOutsideDays={showOutsideDays} | ||
| className={cn( | ||
| ' bg-gray-50 border rounded-[5px] border-pink-100 group/calendar p-3 [--cell-size:--spacing(8)] ', | ||
| String.raw`rtl:**:[.rdp-button\_next>svg]:rotate-180`, |
There was a problem hiding this comment.
제가 알기론 이 구문 Tailwind v4 에서 출력 못하는거 아는데 잘 되나요..?
Tailwind 공식은 이렇게 쓰도록 안내하고 있어서
보시고 고쳐보면 좋을거 같습니다 :)
[dir=rtl]: or data-[dir=rtl]:
There was a problem hiding this comment.
라이브러리 사용해서 따로 인지하지 못하고 있었는데 감사합니다-! 참고해서 수정할게요
| return <Icon name={'Caret'} color='pink-800' size={16} />; | ||
| } else if (orientation === 'right') { | ||
| return ( | ||
| <Icon name={'Caret'} rotate={180} color='pink-800' size={16} /> |
There was a problem hiding this comment.
rotate prop 있었군요..제 아이콘도 수정해야겠네요
| if (!d) return; | ||
| onChange ? onChange(d) : setInnerDate(d); | ||
| setOpen(false); | ||
| }; |
There was a problem hiding this comment.
이 부분 value랑 deafultvalue 있어서 물어봐요..
sync 타이밍 안꼬이고 잘 호출하나요..? ineerDate잘 업데이트 되는지 궁금합니다
There was a problem hiding this comment.
엇.. 네 따로 문제는 없었어요 한번 더 체크해볼게요
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/shared/components/datePicker/calendar.tsx (1)
171-179: 포커스 로직은 Button의 ref 전달에 의존합니다이 컴포넌트의 포커스 이동은 Button이 DOM ref를 전달해야만 동작합니다. button.tsx를 forwardRef로 수정해야 합니다. 위 코멘트의 diff 적용이 선행되어야 합니다.
Also applies to: 176-196
src/shared/components/datePicker/button.tsx (1)
22-40: Button이 ref를 전달하지 않아 CalendarDayButton의 포커스가 동작하지 않습니다CalendarDayButton에서 ref.current?.focus()를 호출하지만, 현재 Button은 forwardRef가 아니라 실제 DOM button에 ref가 연결되지 않습니다. 또한 폼 내 사용 시 예기치 않은 submit을 막기 위해 기본 type='button'을 지정하세요.
적용 diff:
-function Button({ - className, - variant, - asChild = false, - ...props -}: React.ComponentProps<'button'> & - VariantProps<typeof buttonVariants> & { - asChild?: boolean; - }) { - const Comp = asChild ? Slot : 'button'; - - return ( - <Comp - data-slot='button' - className={cn(buttonVariants({ variant }), className)} - {...props} - /> - ); -} +const Button = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<'button'> & VariantProps<typeof buttonVariants> & { asChild?: boolean } +>(function Button({ className, variant, asChild = false, type, ...props }, ref) { + const Comp = asChild ? Slot : 'button'; + return ( + <Comp + data-slot='button' + ref={ref} + className={cn(buttonVariants({ variant }), className)} + type={asChild ? undefined : (type ?? 'button')} + {...props} + /> + ); +});
🧹 Nitpick comments (5)
src/shared/components/datePicker/calendar.tsx (1)
89-93:defaults.day중복 병합
classNames.day에서 이미defaults.day를 합친 뒤, CalendarDayButton에서도 다시defaults.day를 합칩니다. 의미상 중복이므로 한쪽만 유지해도 됩니다.- day: cn( - 'relative w-full h-full p-0 text-center group/day aspect-square select-none', - defaults.day, - ), + day: cn( + 'relative w-full h-full p-0 text-center group/day aspect-square select-none', + ),또는 CalendarDayButton 쪽에서
defaults.day를 제거하세요.Also applies to: 175-193
src/shared/components/datePicker/DatePicker.tsx (2)
41-46: 정적 id 충돌 가능성:id='date'동일 컴포넌트 다중 사용 시 id 충돌 위험이 있습니다.
useId()로 고유 id를 생성하거나 id를 제거하세요.-import { Icon } from '@/shared/icons'; +import { Icon } from '@/shared/icons'; +const uid = React.useId(); ... - <Button - id='date' + <Button + id={uid}
56-58:toYear계산 간소화동일 결과이므로 단순화하여 가독성을 높이세요.
- toYear={new Date(new Date().getFullYear() + 5, 0, 1).getFullYear()} + toYear={new Date().getFullYear() + 5}src/shared/components/datePicker/CustomDropdown.tsx (2)
5-10: 타입과 구현 불일치: onChange는 필수인데?.로 호출onChange가 필수라면 optional chaining을 제거하세요. 반대로 선택 값으로 둘 거면 타입을 optional로 바꾸세요(현재 사용처는 전달하고 있음).
- onChange: (next: number) => void; + onChange: (next: number) => void; ... - if (opt && !opt.disabled) { - onChange?.(opt.value); + if (opt && !opt.disabled) { + onChange(opt.value); setOpen(false); } ... - if (!opt.disabled) { - onChange?.(opt.value); + if (!opt.disabled) { + onChange(opt.value); setOpen(false); }Also applies to: 70-77, 121-126
18-21: 트리거-리스트 연결 ARIA 보강listbox에 id를 부여하고 button에 aria-controls를 연결하면 보조기기 호환성이 좋아집니다.
- const buttonRef = React.useRef<HTMLButtonElement>(null); + const buttonRef = React.useRef<HTMLButtonElement>(null); + const listId = React.useId(); ... - <button + <button ref={buttonRef} type='button' aria-haspopup='listbox' aria-expanded={open} + aria-controls={open ? listId : undefined} ... - {open && ( + {open && ( <ul + id={listId} ref={listRef} role='listbox'Also applies to: 80-97, 99-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/shared/components/datePicker/CustomDropdown.tsx(1 hunks)src/shared/components/datePicker/DatePicker.tsx(1 hunks)src/shared/components/datePicker/button.tsx(1 hunks)src/shared/components/datePicker/calendar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/shared/components/datePicker/CustomDropdown.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/button.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/DatePicker.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/calendar.tsx (2)
src/shared/lib/utils.ts (1)
cn(71-73)src/shared/components/datePicker/CustomDropdown.tsx (1)
CustomDropdown(12-145)
🔇 Additional comments (1)
src/shared/components/datePicker/calendar.tsx (1)
48-59: 리뷰 댓글은 부정확합니다. Tailwind v4 문법이 올바르게 사용되고 있습니다.프로젝트에서 사용 중인 Tailwind CSS 버전은
4.1.14이며, 코드의size-(--cell-size),h-(--cell-size),w-(--cell-size),px-(--cell-size)문법은 Tailwind v4에서 CSS 커스텀 속성을 임의값에 사용할 때 대괄호에서 괄호로 변경된 단축 문법입니다.bg-(--brand-color)는bg-[var(--brand-color)]의 단축형이며, 이는 Tailwind v4의 권장 문법입니다.제안된
size-[var(--cell-size)]형식은 더 긴 형태일 뿐, 현재 코드는 유효하고 최적화되어 있습니다. 검증이나 리팩토링이 필요하지 않습니다.Likely an incorrect or invalid review comment.
…date-picker-components
KongMezu
left a comment
KongMezu
left a comment
There was a problem hiding this comment.
진짜 수고하셨습니다.. 달력 너무 예뻐요(제취향) 리뷰 한번 확인해보시고
쾌적한..리팩토링 하셨음 합니다~
| root: cn('w-fit', defaults.root), | ||
| months: cn('flex gap-4 flex-col md:flex-row relative', defaults.months), | ||
| month: cn('flex flex-col w-full gap-3', defaults.month), | ||
| nav: cn( | ||
| 'flex items-center gap-1 w-full absolute top-0 inset-x-0 justify-between', | ||
| defaults.nav, | ||
| ), | ||
|
|
||
| // 네비 버튼 | ||
| button_previous: cn( | ||
| buttonVariants({ variant: buttonVariant }), | ||
| 'size-(--cell-size) aria-disabled:opacity-50 p-0 select-none', | ||
| defaults.button_previous, | ||
| ), | ||
| button_next: cn( | ||
| buttonVariants({ variant: buttonVariant }), | ||
| 'size-(--cell-size) aria-disabled:opacity-50 p-0 select-none', | ||
| defaults.button_next, | ||
| ), | ||
|
|
||
| month_caption: cn( | ||
| 'flex items-center justify-center h-(--cell-size) w-full px-(--cell-size)', | ||
| defaults.month_caption, | ||
| ), | ||
|
|
||
| // 드롭다운 래퍼들(우리 컴포넌트가 자체 스타일을 가지므로 가볍게만) | ||
| dropdowns: cn( | ||
| 'w-full flex items-center justify-center h-(--cell-size) gap-1.5', | ||
| defaults.dropdowns, | ||
| ), | ||
| dropdown_root: cn( | ||
| 'relative flex items-center gap-2', | ||
| defaults.dropdown_root, | ||
| ), | ||
| dropdown: cn('', defaults.dropdown), | ||
|
|
||
| table: 'w-full border-collapse', | ||
| weekdays: cn('flex', defaults.weekdays), | ||
| weekday: cn( | ||
| 'rounded-md flex-1 text-pink-900 text-label-lg select-none', | ||
| defaults.weekday, | ||
| ), | ||
| week: cn('flex w-full mt-2', defaults.week), | ||
| week_number_header: cn( | ||
| 'select-none w-(--cell-size)', | ||
| defaults.week_number_header, | ||
| ), | ||
| week_number: cn( | ||
| 'text-[0.8rem] select-none text-muted-foreground', | ||
| defaults.week_number, | ||
| ), | ||
|
|
||
| day: cn( | ||
| 'relative w-full h-full p-0 text-center group/day aspect-square select-none', | ||
| defaults.day, | ||
| ), | ||
| range_start: cn('rounded-l-md bg-accent', defaults.range_start), | ||
| range_middle: cn('rounded-none', defaults.range_middle), | ||
| range_end: cn('rounded-r-md bg-accent', defaults.range_end), | ||
| today: cn( | ||
| 'bg-accent text-accent-foreground rounded-md data-[selected=true]:rounded-none', | ||
| defaults.today, | ||
| ), | ||
| outside: cn( | ||
| 'text-muted-foreground aria-selected:text-muted-foreground', | ||
| defaults.outside, | ||
| ), | ||
| disabled: cn('text-muted-foreground opacity-50', defaults.disabled), | ||
| hidden: cn('invisible', defaults.hidden), | ||
|
|
||
| ...classNames, | ||
| }} |
There was a problem hiding this comment.
와우...진짜 코드에서 고생한 흔적이 여실히 느껴지네요 아무래도 가독성 부분때문에 정말 리팩토링이 필요해보여서
혹시 이 부분 classNmae 파일 만들어서 함수 관리 하는거 어떤가요..??
There was a problem hiding this comment.
라이브러리로 만들다보니 따로 파일분리할 생각은 못했네요 수정하겠습니다-!
| Dropdown: ({ value, onChange, options, 'aria-label': a11y }) => { | ||
| const opts = ( | ||
| options as { | ||
| value: number; | ||
| label: string; | ||
| disabled?: boolean; | ||
| }[] | ||
| ).map((o) => ({ ...o, disabled: Boolean(o.disabled) })); | ||
|
|
||
| return ( | ||
| <CustomDropdown | ||
| value={value as number} | ||
| options={opts} | ||
| onChange={(next: number) => { | ||
| const evt = { | ||
| target: { value: String(next) }, | ||
| } as unknown as React.ChangeEvent<HTMLSelectElement>; |
There was a problem hiding this comment.
여기 부분도 inline으로 말고 따로 컴포넌트 파일 분리.. 하면 어떨까요?
| modifiers, | ||
| ...props | ||
| }: React.ComponentProps<typeof DayButton>) { | ||
| const defaults = getDefaultClassNames(); |
There was a problem hiding this comment.
이 부분 코드 20번 줄이랑 중복 호출되는거 같은데 한번 확인 부탁드립니다..!
There was a problem hiding this comment.
CalendarDayButton은 우리 컴포넌트고, React.ComponentProps은 react-day-picker가 주입해주는 DayButton의 props 타입을 그대로 재사용하는 것뿐이라 중복 선언이 아닙니다-! 이름이 비슷해서 헷갈리기 쉬운것 같네요..
| React.useEffect(() => { | ||
| if (!open) return; | ||
| const onDocClick = (e: MouseEvent) => { | ||
| const t = e.target as Node; |
There was a problem hiding this comment.
여기 mousedown 으로 하면 저희가 모바일에서 이용할때 동작 안될 수도 있을거 같아요!
pointdown 으로 바꿔보면 어떨까요??
그럼 마우스, 터치 다 될거 같아요 :)
| const selectedIndex = Math.max( | ||
| 0, | ||
| options.findIndex((o) => o.value === value), | ||
| ); | ||
| const [activeIndex, setActiveIndex] = React.useState(selectedIndex); |
There was a problem hiding this comment.
만약 value 값이 변경되면 activeIndex도 같이 동기화 되어야 하지 않을까요????
지금 코드 보면 초기 렌더링 값만 받는거 같아서
확인 부탁드립니다!!
There was a problem hiding this comment.
확인해보니 지금 코드는 초기값만 반영하고, value(혹은 options)가 바뀌어도 activeIndex는 그대로네요.. value/options 변화에 맞춰 동기화 해주면 좋을 것같아 수정해두겠습니다
| const [open, setOpen] = React.useState(false); | ||
| const [innerDate, setInnerDate] = React.useState<Date | undefined>( | ||
| defaultValue, | ||
| ); | ||
|
|
||
| const selected = value ?? innerDate; |
There was a problem hiding this comment.
여기 지금 내부(innerDate)랑 외부(value)상태 다 받는거 같은데 맞나요..???
2개다 사용해야하는거면
useEffect로 동기화 하는게 좋을거 같습니다!!
| function PopoverAnchor({ | ||
| ...props | ||
| }: React.ComponentProps<typeof PopoverPrimitive.Anchor>) { | ||
| return <PopoverPrimitive.Anchor data-slot='popover-anchor' {...props} />; | ||
| } |
There was a problem hiding this comment.
위에서부터
Popver, PopverTrigger, PopverAnchor 코드 패턴이 비슷해보이는데
추상화 해보시면 어떨까요 :)
참고로 전 추상화를 아주 좋아해서 건의해봅니다..~
| sideOffset={sideOffset} | ||
| className={cn( | ||
| ' text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 w-72 origin-(--radix-popover-content-transform-origin) ', |
There was a problem hiding this comment.
여기도 클래스 분리 해보실 생각 없나요..??
There was a problem hiding this comment.
캘린더는 스타일이 많아서 분리하는 것에 괜찮다고 생각했지만 이부분은 스타일이 한곳에만 적용하는 부분이라 오히려 분리하면 더 복잡할 것 같다는 생각이 듭니다.. 대신 가독성 좋게 수정해보겠습니다-!
| )} | ||
| {...props} | ||
| /> | ||
| </PopoverPrimitive.Portal> |
There was a problem hiding this comment.
<PopoverPrimitive.Portal>
<PopoverPrimitive.Content ... />
</PopoverPrimitive.Portal>
요 구조가 Next SSR 에서 조금 취약한 구조라
하이드레이션이 발생할 수 있을거 같아요
(Next가 이 부분이..예민해서)
블로그에도 나와있지만 참고하셔서
SSR-safe wrapper로 감싸서 코드 리팩토링 해봐도 좋을거 같아요!
# 활용 예시..
import dynamic from 'next/dynamic';
const PopoverContentNoSSR = dynamic(
() => import('./PopoverContent'),
{ ssr: false }
);
There was a problem hiding this comment.
이 부분은 몰랐던 부분이네요.. 참고해서 수정하겠습니다 감사합니다-!
…date-picker-components
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/shared/components/datePicker/popover.tsx (1)
5-21: 이전 리뷰에서 제안된 추상화 패턴을 고려해보세요.세 개의 래퍼 컴포넌트(
Popover,PopoverTrigger,PopoverAnchor)가 동일한 패턴을 따르고 있습니다:
- Radix primitive 래핑
data-slot속성 추가- props 전달
이전 리뷰에서 언급된 것처럼 공통 팩토리 함수로 추상화를 고려해볼 수 있습니다:
function createPopoverComponent<T extends keyof typeof PopoverPrimitive>( primitive: T, slotName: string, ) { return (props: React.ComponentProps<typeof PopoverPrimitive[T]>) => { const Component = PopoverPrimitive[primitive]; return <Component data-slot={slotName} {...props} />; }; } export const Popover = createPopoverComponent('Root', 'popover'); export const PopoverTrigger = createPopoverComponent('Trigger', 'popover-trigger'); export const PopoverAnchor = createPopoverComponent('Anchor', 'popover-anchor');다만 현재 구현도 명확하고 타입 안전성이 보장되므로, 추상화로 인한 복잡도 증가를 고려하여 선택적으로 적용하시면 됩니다.
🧹 Nitpick comments (2)
src/shared/components/datePicker/PopoverContent.client.tsx (1)
1-1: 'use client' 디렉티브가 중복될 수 있습니다.이 컴포넌트는 이미
popover.tsx에서dynamic(..., { ssr: false })로 import되고 있어 클라이언트 사이드에서만 렌더링됩니다.'use client'디렉티브가 추가로 필요하지 않을 수 있지만, 명시적으로 선언하는 것이 해를 끼치지는 않으며 이 파일을 독립적으로 사용할 경우를 대비할 수 있습니다.src/shared/components/datePicker/CalendarDropdownAdapter.tsx (1)
13-19: 타입 안전성 검증 추가를 권장합니다Lines 13-14와 17-19에서
as number,as any,as unknown as등의 타입 단언을 사용하고 있는데, 이는 TypeScript의 타입 안전성을 우회합니다.react-day-picker의DropdownProps가 예상과 다른 타입을 전달할 경우 런타임 에러가 발생할 수 있습니다.다음과 같이 런타임 검증을 추가하는 것을 고려해보세요:
export function CalendarDropdownAdapter({ value, onChange, options, 'aria-label': ariaLabel, }: DropdownProps) { + // Runtime validation + if (value !== undefined && typeof value !== 'number') { + console.warn('CalendarDropdownAdapter: value is not a number', value); + } + return ( <CustomDropdown value={value as number} options={options as any} aria-label={ariaLabel} onChange={(next) => { const evt = { target: { value: String(next) }, } as unknown as React.ChangeEvent<HTMLSelectElement>; onChange?.(evt); }} /> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/pages/index.tsx(1 hunks)src/shared/components/datePicker/CalendarDropdownAdapter.tsx(1 hunks)src/shared/components/datePicker/CustomDropdown.tsx(1 hunks)src/shared/components/datePicker/DatePicker.tsx(1 hunks)src/shared/components/datePicker/PopoverContent.client.tsx(1 hunks)src/shared/components/datePicker/calendar.classes.ts(1 hunks)src/shared/components/datePicker/calendar.tsx(1 hunks)src/shared/components/datePicker/popover.tsx(1 hunks)src/shared/components/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/shared/components/index.ts
- src/pages/index.tsx
- src/shared/components/datePicker/DatePicker.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
src/shared/components/datePicker/PopoverContent.client.tsx (2)
src/shared/components/datePicker/popover.tsx (1)
PopoverContent(23-29)src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/popover.tsx (1)
src/shared/components/datePicker/PopoverContent.client.tsx (1)
PopoverContent(7-33)
src/shared/components/datePicker/CustomDropdown.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/CalendarDropdownAdapter.tsx (1)
src/shared/components/datePicker/CustomDropdown.tsx (1)
CustomDropdown(12-187)
src/shared/components/datePicker/calendar.classes.ts (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/calendar.tsx (3)
src/shared/components/datePicker/calendar.classes.ts (1)
createCalendarClassNames(11-78)src/shared/lib/utils.ts (1)
cn(71-73)src/shared/components/datePicker/CalendarDropdownAdapter.tsx (1)
CalendarDropdownAdapter(5-24)
🔇 Additional comments (10)
src/shared/components/datePicker/popover.tsx (1)
23-29: SSR 이슈 대응이 적절합니다.이전 리뷰에서 지적된 Next.js SSR 하이드레이션 문제를
dynamicimport와{ ssr: false }옵션으로 해결했습니다.PopoverContent를 별도의 client 파일로 분리하고 클라이언트 사이드에서만 로드되도록 구성한 접근 방식이 올바릅니다.src/shared/components/datePicker/PopoverContent.client.tsx (2)
7-13: forwardRef 구현이 올바릅니다.
- 타입이 정확하게 지정되었습니다 (
React.ElementRef,React.ComponentPropsWithoutRef)- 함수에 이름(
PopoverContent)을 제공하여 디버깅이 용이합니다- ref 전달이 올바르게 구현되었습니다
- 기본값(
align='center',sideOffset={4})이 적절합니다
14-31: Portal과 애니메이션 구현이 적절합니다.
PopoverPrimitive.Portal로 콘텐츠를 감싸서 stacking context 문제를 해결하고, Radix의 data attributes를 활용한 Tailwind 애니메이션 클래스들이 올바르게 적용되었습니다:
- Enter/exit 애니메이션 (
animate-in,animate-out)- Fade 효과 (
fade-in-0,fade-out-0)- Zoom 효과 (
zoom-in-95,zoom-out-95)- 방향별 slide 애니메이션 (4방향 모두 지원)
className이cn()호출의 마지막에 위치하여 외부에서 전달된 스타일이 우선 적용되도록 한 점도 좋습니다.src/shared/components/datePicker/calendar.tsx (3)
41-48: 아이콘 회전 로직이 깔끔합니다
orientation에 따라Caret아이콘의 회전값을 적절히 설정하여 left/right/down 방향을 표현하고 있습니다. 조건문 구조가 명확하고 읽기 쉽습니다.
57-89: 접근성과 상태 관리가 잘 구현되었습니다
CalendarDayButton구현이 우수합니다:
- Lines 64-66:
modifiers.focused에 따른 자동 포커스로 키보드 내비게이션 지원- Lines 72-81: data 속성을 통한 날짜 상태 추적으로 스타일링 훅 제공
- Ref 전달과 의존성 배열이 정확함
29-32: CSS 변수 구문이 올바릅니다. 문제 없음.
[--cell-size:--spacing(8)]문법은 Tailwind CSS v4.0의 정식 지원 기능입니다. Tailwind 공식 문서에서py-[calc(--spacing(4)-1px)]처럼 임의 값 내에서--spacing()함수를 사용하는 예제를 제시하고 있습니다. Tailwind v4에서mt-8,w-17같은 spacing 유틸리티는calc(var(--spacing) * 8)형태로 동작하며,--spacing()함수는 spacing scale에서 값을 곱하여 반환합니다.따라서
[--cell-size:--spacing(8)]은 CSS 변수--cell-size를 0.25rem × 8 = 2rem 값으로 설정하는 유효한 구문이며, 이후size-(--cell-size)등으로 참조되어 올바르게 작동합니다.src/shared/components/datePicker/CustomDropdown.tsx (4)
30-38: activeIndex 동기화가 올바르게 구현되었습니다이전 리뷰 코멘트에서 지적된
value변경 시activeIndex동기화 문제가 해결되었습니다.useEffect가selectedIndex와options.length변화를 추적하여 적절히activeIndex를 업데이트합니다.
43-86: 포인터 이벤트 감지로 모바일 호환성이 개선되었습니다Lines 53-85에서
PointerEvent지원 여부를 감지하고, 지원하지 않는 환경에서는mousedown+touchstart로 폴백하는 패턴이 우수합니다. 이전 리뷰에서 언급된 모바일 호환성 문제를 해결했습니다.Lines 64, 77, 82의
as any타입 단언은 TypeScript의EventListenerOptions타입 불일치 때문인 것으로 보이며, 큰 문제는 아닙니다.
88-122: 키보드 내비게이션이 접근성 표준을 잘 따릅니다키보드 제어 구현이 우수합니다:
- ArrowUp/ArrowDown: 비활성화된 옵션을 건너뛰며 탐색 (Lines 107, 112)
- Space: 드롭다운 열기
- Enter: 현재 활성 옵션 선택 (비활성화 확인 포함)
- Escape: 드롭다운 닫기
- preventDefault로 스크롤 방지
ARIA 권장사항을 충실히 따르고 있습니다.
124-186: ARIA 구현과 렌더 로직이 훌륭합니다드롭다운 렌더링이 WAI-ARIA Listbox 패턴을 정확히 따릅니다:
- Lines 129-131:
aria-haspopup,aria-expanded,aria-label설정- Lines 146-148: listbox role과 ARIA 속성 설정
- Lines 162-163: 각 옵션의 선택/비활성 상태 표시
- Line 165:
onPointerDown의preventDefault로 포커스 이동 방지접근성과 사용자 경험이 모두 고려된 구현입니다.
| button_previous: cn( | ||
| buttonVariants({ variant: buttonVariant }), | ||
| 'size-(--cell-size) aria-disabled:opacity-50 p-0 select-none', | ||
| d.button_previous, | ||
| ), | ||
| button_next: cn( | ||
| buttonVariants({ variant: buttonVariant }), | ||
| 'size-(--cell-size) aria-disabled:opacity-50 p-0 select-none', | ||
| d.button_next, | ||
| ), | ||
| month_caption: cn( | ||
| 'flex items-center justify-center h-(--cell-size) w-full px-(--cell-size)', |
There was a problem hiding this comment.
🧩 Analysis chain
CSS 변수 문법 확인이 필요합니다
Lines 28, 33, 37에서 사용된 size-(--cell-size), h-(--cell-size), px-(--cell-size) 문법은 Tailwind의 표준 임의 값 문법이 아닙니다. 일반적으로는 size-[var(--cell-size)] 형태를 사용합니다.
이 문법이 프로젝트의 Tailwind 설정에서 커스텀으로 지원되는지 확인이 필요합니다.
다음 스크립트로 Tailwind 설정에서 이 패턴이 정의되어 있는지 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Tailwind 설정 파일에서 커스텀 패턴 확인
rg -n "size-\(|matchUtilities" --type-add 'config:*.{js,ts,cjs,mjs}' --type config
# 해당 문법이 실제로 컴파일되는지 CSS 출력 확인
fd -e css -x rg -l "var\(--cell-size\)" {}Length of output: 249
🏁 Script executed:
#!/bin/bash
# Find Tailwind configuration file
fd -e 'config' -name 'tailwind*' -type f
# Also search for any config files
fd -e 'js' -o -e 'ts' -o -e 'cjs' -o -e 'mjs' | grep -i 'tailwind\|postcss' | head -20Length of output: 347
🏁 Script executed:
#!/bin/bash
# Check package.json for Tailwind version and plugins
cat package.json | grep -A 5 -B 5 '"tailwind\|"postcss' || echo "Could not find tailwind/postcss in package.json"Length of output: 681
🏁 Script executed:
#!/bin/bash
# Search for where cn() or clsx is imported and used
rg -n "import.*\{.*cn\}|from.*clsx|from.*classnames" src/shared/components/datePicker/calendar.classes.ts -A 2 -B 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's a custom Tailwind plugin or configuration that handles parentheses syntax
rg -n "size-\(|h-\(|w-\(|px-\(|py-\(" --type ts --type js | head -20Length of output: 770
🏁 Script executed:
#!/bin/bash
# Check top of calendar.classes.ts for imports
head -n 15 src/shared/components/datePicker/calendar.classes.tsLength of output: 478
🏁 Script executed:
#!/bin/bash
# Search for tailwind.config at repository root
find . -maxdepth 2 -name "tailwind.config.*" -o -name "postcss.config.*"Length of output: 19
🏁 Script executed:
#!/bin/bash
# Search where cn is imported from in the codebase
rg -n "^import.*cn.*from" --type ts --type tsx | head -10Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if parentheses syntax is used with valid CSS output
rg -n "var\(--cell-size\)" src/ --type css --type tsx --type tsLength of output: 27
🏁 Script executed:
#!/bin/bash
# Read postcss configuration
cat postcss.config.jsLength of output: 98
🏁 Script executed:
#!/bin/bash
# Find shared/lib and check cn function
find src -path "*/shared/lib*" -type f | head -20Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for CSS variable var patterns without type filtering
rg "var\(--cell-size\)" src/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's a tailwind config at the root
ls -la | grep tailwindLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any build outputs or dist that might show compiled CSS
find . -path "*/dist/*" -o -path "*/.next/*" 2>/dev/null | grep -E "\.(css|js)$" | head -10Length of output: 780
🏁 Script executed:
#!/bin/bash
# Check cn function in shared/lib
cat src/shared/lib/index.tsLength of output: 24
🏁 Script executed:
#!/bin/bash
# Also check utils
cat src/shared/lib/utils.tsLength of output: 1458
🏁 Script executed:
#!/bin/bash
# Search for any CSS files in the project
find src -name "*.css" | head -20Length of output: 22
🏁 Script executed:
#!/bin/bash
# Check if there's a globals or styles directory
find src -type d -name "styles" -o -type d -name "css"Length of output: 10
🏁 Script executed:
#!/bin/bash
# Search more thoroughly for any tailwind config (including .cjs, .mjs variations)
find . -maxdepth 3 -name "tailwind*" -type f 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's any tailwind config in TypeScript or JavaScript at root
ls -la | grep -E "tailwind|next\.config"Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check next.config for any theme or tailwind extensions
find . -maxdepth 2 -name "next.config.*" -exec cat {} \;Length of output: 814
🏁 Script executed:
#!/bin/bash
# Look at the actual CSS file to see if there are CSS variable definitions
head -n 50 src/styles/globals.cssLength of output: 1265
🏁 Script executed:
#!/bin/bash
# Search for --cell-size variable definition
rg "\-\-cell-size" src/Length of output: 1000
🏁 Script executed:
#!/bin/bash
# Look for CSS/theme definitions with cell-size
rg "cell-size" . --type cssLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for Tailwind v4 theme or CSS variable extensions
tail -n 100 src/styles/globals.css | head -n 50Length of output: 2868
🏁 Script executed:
#!/bin/bash
# Check the entire globals.css for any --cell-size or related sizing variables
cat src/styles/globals.css | grep -i "cell\|size" | head -20Length of output: 2577
🌐 Web query:
Tailwind CSS v4 arbitrary values CSS variables syntax size-(--cell-size) valid
💡 Result:
Short answer: No — "size-(--cell-size)" is not a valid Tailwind v4 class by itself.
Details:
- Tailwind v4 changed arbitrary-value variable shorthand to parentheses (e.g., bg-(--brand-color) or w-(--my-var)). (tailwindcss.cn)
- However, Tailwind doesn't provide a generic "size-" utility. Use the appropriate utility (e.g., w-, h-, inline-size, block-size) with the variable: for example w-(--cell-size) or h-(--cell-size). If the utility is ambiguous you can add a type hint (e.g., text-(length:--my-var)). (gist.github.com)
If you want Tailwind to set an arbitrary custom property (CSS property name) you can use arbitrary properties: [block-size:var(--cell-size)] or the shorthand with parentheses for variables where supported. (cruip.com)
Citations:
- 1: https://www.tailwindcss.cn/docs/v4-beta?utm_source=openai
- 2: https://gist.github.com/t-mart/ee3684eff4064bab9cbe99890fc0e23c?utm_source=openai
- 3: https://cruip.com/how-to-use-calc-in-tailwind-css/?utm_source=openai
🌐 Web query:
Tailwind CSS v4 @tailwindcss/postcss CSS variable arbitrary values
💡 Result:
Short answer: Tailwind v4 moves to a CSS-first model (design tokens exposed as CSS variables) and changes how arbitrary values are parsed — the old square-bracket arbitrary syntax can break. Use the new v4 arbitrary syntax (parentheses) or reference the generated CSS variables directly. See the v4 announcement and migration notes for examples. [1][2]
Practical notes:
- v4 exposes theme values as CSS variables (via @theme), so prefer using those variables (e.g. var(--color-...)). [1]
- The arbitrary-value tokenization changed in v4 to avoid ambiguity; many guides show the new parentheses form (e.g. bg-(--brand-color) or bg-(var(--brand-color))) instead of w-[200px] / bg-[#ff0]. Test both bg-(var(--foo)) and bg-(--foo) in your project. [1][2]
- If you still need PostCSS integration, use the @tailwindcss/postcss workflow shown in the v4 docs (Tailwind’s new import/postcss handling). [1]
If you want, tell me which exact class you’re trying to convert (example input) and I’ll give the exact v4 class syntax to use.
Sources:
[1] Tailwind CSS v4 announcement (blog) — Tailwind CSS.
[2] "Getting ready for Tailwind v4.0" (migration notes / community write-up).
Lines 28, 33의 size-(--cell-size) 구문 수정 필요
Tailwind v4에서는 CSS 변수를 위한 괄호 문법 propertyName-(--var) 형태를 지원합니다. 그러나 size-는 Tailwind의 표준 유틸리티 클래스가 아닙니다.
Lines 28, 33의:
'size-(--cell-size) aria-disabled:opacity-50 p-0 select-none',
이 코드는 의도한 대로 작동하지 않습니다. size를 각각의 width/height 유틸리티로 분리해야 합니다:
'w-(--cell-size) h-(--cell-size) aria-disabled:opacity-50 p-0 select-none',
또는 arbitrary properties를 사용할 수 있습니다:
'[width:var(--cell-size)] [height:var(--cell-size)] aria-disabled:opacity-50 p-0 select-none',
🤖 Prompt for AI Agents
In src/shared/components/datePicker/calendar.classes.ts around lines 26 to 37,
the utility "size-(--cell-size)" is invalid in Tailwind v4; replace it on both
button_previous and button_next entries with proper width/height
utilities—either "w-(--cell-size) h-(--cell-size)" or the arbitrary property
form "[width:var(--cell-size)] [height:var(--cell-size)]"—and keep the rest of
the class string (aria-disabled:opacity-50 p-0 select-none) unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/shared/components/datePicker/calendar.tsx (2)
33-37: formatMonthDropdown의 로케일 불일치 가능성
toLocaleString('default', ...)를 사용하면 DayPicker에 전달된localeprop과 다른 로케일이 적용될 수 있습니다. 사용자가 명시적으로 다른 로케일을 전달한 경우 월 이름이 일관되지 않을 수 있습니다.DayPicker의 locale prop을 고려하도록 개선할 수 있습니다:
-formatMonthDropdown: (d) => - d.toLocaleString('default', { month: 'long' }), +formatMonthDropdown: (d, options) => + d.toLocaleString(options?.locale?.code || 'default', { month: 'long' }),
64-66: useEffect 의존성 안정성 확인 필요
modifiers객체는 매 렌더링마다 새로 생성될 가능성이 높아,modifiers.focused값이 변경되지 않았더라도 effect가 불필요하게 실행될 수 있습니다.보다 안정적인 의존성 배열로 개선할 수 있습니다:
+const focused = modifiers.focused; useEffect(() => { - if (modifiers.focused) ref.current?.focus(); -}, [modifiers.focused]); + if (focused) ref.current?.focus(); +}, [focused]);src/shared/components/datePicker/CustomDropdown.tsx (1)
133-174: 활성 옵션 자동 스크롤 추가 권장키보드로 드롭다운을 탐색할 때 활성 옵션(highlighted)이 뷰포트 밖에 있으면 사용자가 현재 위치를 파악하기 어렵습니다. 특히 옵션 목록이 긴 경우 UX가 저하될 수 있습니다.
활성 항목을 자동으로 스크롤하도록 개선할 수 있습니다:
+const activeItemRef = useRef<HTMLLIElement>(null); + +useEffect(() => { + if (open && activeItemRef.current) { + activeItemRef.current.scrollIntoView({ block: 'nearest' }); + } +}, [open, activeIndex]); {options.map((opt, idx) => { const selected = opt.value === value; const highlighted = idx === activeIndex; return ( <li + ref={highlighted ? activeItemRef : null} key={opt.value}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/shared/components/datePicker/CalendarDropdownAdapter.tsx(1 hunks)src/shared/components/datePicker/CustomDropdown.tsx(1 hunks)src/shared/components/datePicker/PopoverContent.client.tsx(1 hunks)src/shared/components/datePicker/calendar.tsx(1 hunks)src/shared/components/datePicker/popover.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/components/datePicker/popover.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
src/shared/components/datePicker/CustomDropdown.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/calendar.tsx (4)
src/shared/components/datePicker/calendar.classes.ts (1)
createCalendarClassNames(11-78)src/shared/lib/utils.ts (1)
cn(71-73)src/shared/components/datePicker/CalendarDropdownAdapter.tsx (1)
CalendarDropdownAdapter(4-23)src/shared/icons/components/icon.tsx (1)
IconProps(42-52)
src/shared/components/datePicker/PopoverContent.client.tsx (2)
src/shared/components/datePicker/popover.tsx (1)
PopoverContent(22-28)src/shared/lib/utils.ts (1)
cn(71-73)
src/shared/components/datePicker/CalendarDropdownAdapter.tsx (1)
src/shared/components/datePicker/CustomDropdown.tsx (1)
CustomDropdown(13-177)
🔇 Additional comments (1)
src/shared/components/datePicker/PopoverContent.client.tsx (1)
1-32: 구현이 잘 되어 있습니다Radix Popover.Content를 적절히 래핑하고, forwardRef를 통한 ref 전달, 기본 props 설정, 애니메이션 클래스 구성이 모두 올바르게 구현되었습니다. 'use client' 지시어와 Portal 사용도 적절합니다.
| return ( | ||
| <CustomDropdown | ||
| value={value as number} | ||
| options={options ?? []} | ||
| aria-label={ariaLabel} | ||
| onChange={(next) => { | ||
| const evt = { | ||
| target: { value: String(next) }, | ||
| } as unknown as React.ChangeEvent<HTMLSelectElement>; | ||
| onChange?.(evt); | ||
| }} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
타입 안전성 문제: value의 타입 변환
value as number 캐스팅은 DropdownProps의 value가 string | number | undefined일 수 있는데, 타입 검증 없이 number로 가정합니다. react-day-picker가 string 값을 전달하는 경우 런타임 오류나 예기치 않은 동작이 발생할 수 있습니다.
타입 가드를 추가하여 안전하게 처리하세요:
export function CalendarDropdownAdapter({
value,
onChange,
options,
'aria-label': ariaLabel,
}: DropdownProps) {
+ const numericValue = typeof value === 'number' ? value : undefined;
+
return (
<CustomDropdown
- value={value as number}
+ value={numericValue}
options={options ?? []}
aria-label={ariaLabel}
onChange={(next) => {
const evt = {
target: { value: String(next) },
} as unknown as React.ChangeEvent<HTMLSelectElement>;
onChange?.(evt);
}}
/>
);
}📝 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.
| return ( | |
| <CustomDropdown | |
| value={value as number} | |
| options={options ?? []} | |
| aria-label={ariaLabel} | |
| onChange={(next) => { | |
| const evt = { | |
| target: { value: String(next) }, | |
| } as unknown as React.ChangeEvent<HTMLSelectElement>; | |
| onChange?.(evt); | |
| }} | |
| /> | |
| ); | |
| export function CalendarDropdownAdapter({ | |
| value, | |
| onChange, | |
| options, | |
| 'aria-label': ariaLabel, | |
| }: DropdownProps) { | |
| const numericValue = typeof value === 'number' ? value : undefined; | |
| return ( | |
| <CustomDropdown | |
| value={numericValue} | |
| options={options ?? []} | |
| aria-label={ariaLabel} | |
| onChange={(next) => { | |
| const evt = { | |
| target: { value: String(next) }, | |
| } as unknown as React.ChangeEvent<HTMLSelectElement>; | |
| onChange?.(evt); | |
| }} | |
| /> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/shared/components/datePicker/CalendarDropdownAdapter.tsx around lines 10
to 22, the code force-casts value to number which is unsafe because
DropdownProps.value can be string | number | undefined; add a type guard to
detect if value is a string, number, or undefined and normalize it before
passing to CustomDropdown (e.g., parseInt or Number when value is a numeric
string, use a safe fallback like undefined or null when value is invalid, or
pass the original string when options are string-typed), and ensure the onChange
event conversion matches that normalized type so no unchecked casts remain.
| if (e.key === 'ArrowDown') { | ||
| e.preventDefault(); | ||
| let i = activeIndex + 1; | ||
| while (i < options.length && options[i].disabled) i++; | ||
| setActiveIndex(Math.min(i, options.length - 1)); | ||
| } else if (e.key === 'ArrowUp') { | ||
| e.preventDefault(); | ||
| let i = activeIndex - 1; | ||
| while (i >= 0 && options[i].disabled) i--; | ||
| setActiveIndex(Math.max(i, 0)); |
There was a problem hiding this comment.
키보드 내비게이션 버그: 비활성 옵션에 멈출 수 있음
ArrowDown/ArrowUp 로직에서 모든 남은 옵션이 disabled인 경우, while 루프가 배열 끝까지 진행한 뒤 Math.min/Math.max로 인해 비활성 옵션에 activeIndex가 설정될 수 있습니다.
재현 시나리오:
- activeIndex=0, options=[{disabled:false}, {disabled:true}, {disabled:true}]
- ArrowDown 입력 → i=1 (disabled) → i=2 (disabled) → i=3 (종료) → setActiveIndex(2)
- 결과: options[2]는 disabled인데 활성화됨
비활성 옵션을 건너뛸 수 있도록 수정하세요:
if (e.key === 'ArrowDown') {
e.preventDefault();
let i = activeIndex + 1;
while (i < options.length && options[i].disabled) i++;
- setActiveIndex(Math.min(i, options.length - 1));
+ if (i < options.length) {
+ setActiveIndex(i);
+ }
} else if (e.key === 'ArrowUp') {
e.preventDefault();
let i = activeIndex - 1;
while (i >= 0 && options[i].disabled) i--;
- setActiveIndex(Math.max(i, 0));
+ if (i >= 0) {
+ setActiveIndex(i);
+ }
}📝 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.
| if (e.key === 'ArrowDown') { | |
| e.preventDefault(); | |
| let i = activeIndex + 1; | |
| while (i < options.length && options[i].disabled) i++; | |
| setActiveIndex(Math.min(i, options.length - 1)); | |
| } else if (e.key === 'ArrowUp') { | |
| e.preventDefault(); | |
| let i = activeIndex - 1; | |
| while (i >= 0 && options[i].disabled) i--; | |
| setActiveIndex(Math.max(i, 0)); | |
| if (e.key === 'ArrowDown') { | |
| e.preventDefault(); | |
| let i = activeIndex + 1; | |
| while (i < options.length && options[i].disabled) i++; | |
| if (i < options.length) { | |
| setActiveIndex(i); | |
| } | |
| } else if (e.key === 'ArrowUp') { | |
| e.preventDefault(); | |
| let i = activeIndex - 1; | |
| while (i >= 0 && options[i].disabled) i--; | |
| if (i >= 0) { | |
| setActiveIndex(i); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/shared/components/datePicker/CustomDropdown.tsx around lines 94 to 103,
the ArrowDown/ArrowUp handlers can set activeIndex to a disabled option when the
while loop runs past the array bounds; change the logic so you scan for the next
non-disabled index and only call setActiveIndex if a valid index was found
within bounds, otherwise leave activeIndex unchanged (i.e., compute nextIdx by
advancing/retreating while skipping disabled, then if nextIdx is within [0,
options.length-1] and options[nextIdx] is not disabled call
setActiveIndex(nextIdx); do the same mirrored behavior for ArrowUp).
…date-picker-components
KongMezu
left a comment
KongMezu
left a comment
There was a problem hiding this comment.
Date 리뷰 반영까지 수고 많으셨습니다! approve 하도록 하겠습니다!
2 participants
🔥 작업 내용
🤔 추후 작업 사항
🔗 이슈
PR Point (To Reviewer)
안좋은 이슈: 폰트와 텍스트 컬러가 동시에 적용이 안된다
원인은 twMerge(=tailwind-merge) 때문이었어요.
cn 유틸에서 extendTailwindMerge({ extend: { theme: { text: [...] } } }) 로 text-* 타이포 클래스들(text-label-lg 등)을 등록해 둔 상태였습니다 tailwind-merge 에서 text-*는 “같은 그룹”으로 취급됩니다(색상 + 폰트사이즈 전부).
그래서 text-pink-200(색상)과 text-label-lg(타이포 크기)를 같이 넘기면, 뒤에 오는 것만 남고 앞의 것이 지워져요.
실제로 렌더된 클래스에서 둘중 하나의 스타일이 사라진 이유가 이거예요.
즉, “색상”과 “타이포 크기”가 서로 충돌 그룹으로 병합되고 있었습니다.
해결방법은 cn 유틸에서 타이포의 접두어를 제거했습니다
이렇게 되면 알려진 그룹으로 인식하지 못하는 ‘미지 클래스’로 간주합니다. 그러면 병합 대상에서 빠지므로 문제가 해결되는 것이죠.
하지만 이 방식이 근본적인 해결책은 아니라고 생각이 들어요 이 문제 대해선 추후 얘기를 더 해보았음합니다.
깨알 설명: twMerge 함수 커스텀
tailwind-merge와 clsx를 포함한 cn utils 함수란
export function cn(...inputs: ClassValue[]) {
return twMerge(clsx(inputs));
}
clsx는 두 개 이상의 className을 합쳐주기 위함이고, tailwind-merge는 기존 클래스와 충돌하게 되는 클래스를 override해줘요. 그러한 목적을 동시에 가진 것이 바로 이 cn 유틸 함수인 것이죠.
하지만 단순히 저렇게 쓰게 되면 약간의 문제가 있어요. 단순 tailwind가 제공하는 클래스가 아니라 커스텀한, 즉 우리 팀에서는 ds에서 추가한 color, typo 등의 토큰(유틸리티 클래스)을 사용하게 되면 문제가 생기게 돼요.
따라서 커스텀한 유틸리티 클래스라는 것을 twMerge함수에게 알려줘야 제대로 작동하는 것이죠. 그래서 tailwind-merge라이브러리에서는 이를 위한 목적의 extendTailwindMerge를 제공해줘요. 그래서 extendTailwindMerge 내부로 커스텀 클래스들을 전달해주고 있는 것이죠.
사용 방법
📸 피그마 스크린샷 or 기능 GIF
Summary by CodeRabbit
새로운 기능
접근성/사용성
스타일/토큰