Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough세 개의 새로운 파일이 추가되었습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🏷️ Labeler has automatically applied labels based on your PR title, branch name, or commit message. |
![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 (1)
src/pages/mypage/events/[id].tsx (1)
17-78:PostCardSavePage와 코드 중복 고려
EventSavePage와PostCardSavePage는 레이아웃 구조가 매우 유사합니다. 두 페이지 모두 Header, 이미지 섹션, EventCard를 사용하는 패턴을 공유합니다. API 연결 작업 시 공통 레이아웃 컴포넌트로 리팩토링하는 것을 고려해보세요.예시 구조:
// components/SavedItemLayout.tsx interface SavedItemLayoutProps { title: string; children: React.ReactNode; imageSection: React.ReactNode; additionalContent?: React.ReactNode; } export const SavedItemLayout = ({ title, children, imageSection, additionalContent }: SavedItemLayoutProps) => { const router = useRouter(); return ( <div className={cn('relative w-full min-h-[100vh] overflow-auto')}> <Header title={title} onClick={() => router.back()} className={cn('fixed top-0 left-0 right-0 z-50')} /> <main className={cn('flex flex-col items-center justify-start', 'px-[2.4rem]')}> {additionalContent} {imageSection} {children} </main> </div> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/events/components/DateTag.tsx(1 hunks)src/pages/mypage/events/[id].tsx(1 hunks)src/pages/mypage/postcard/[id].tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/pages/mypage/postcard/[id].tsx (2)
src/shared/constants/events/eventsData.ts (1)
eventData(11-75)src/shared/lib/utils.ts (1)
cn(71-73)
src/pages/mypage/events/[id].tsx (3)
src/shared/constants/events/eventsData.ts (1)
eventData(11-75)src/shared/lib/utils.ts (1)
cn(71-73)src/pages/events/components/DateTag.tsx (1)
DateTag(24-41)
src/pages/events/components/DateTag.tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (4)
src/pages/mypage/postcard/[id].tsx (2)
1-6: 임포트가 올바르게 구성되어 있습니다.필요한 컴포넌트와 유틸리티가 적절히 임포트되어 있습니다.
16-67: 레이아웃 구조가 깨끗하게 구현되었습니다.Header, 이미지 영역, EventCard가 잘 구성되어 있으며, PR 목표에 명시된 대로 카드 플립 컴포넌트를 위한 자리가 준비되어 있습니다. 이미지 placeholder와 실제 이미지의 높이도 일치합니다.
src/pages/mypage/events/[id].tsx (2)
1-6: 임포트가 올바르게 구성되어 있습니다.새로 추가된
DateTag컴포넌트를 포함하여 모든 필요한 컴포넌트가 적절히 임포트되어 있습니다.
31-34: DateTag 컴포넌트가 올바르게 사용되었습니다.행사 기간을 표시하기 위해
DateTag컴포넌트가 적절히 활용되고 있습니다.
| @@ -0,0 +1,41 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
'use client' 지시문의 필요성 확인
이 컴포넌트는 클라이언트 측 상태나 이벤트 핸들러를 사용하지 않으므로 'use client' 지시문이 불필요할 수 있습니다. 부모 컴포넌트에서 서버 컴포넌트로 사용할 계획이 없다면 이 지시문을 제거하는 것이 좋습니다.
다음 스크립트로 이 컴포넌트를 사용하는 부모 컴포넌트들을 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: DateTag 컴포넌트를 import하는 파일들을 찾아 서버/클라이언트 컴포넌트 여부 확인
rg -n "from.*DateTag" --type=tsx --type=ts -A 2 -B 2Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Find files importing DateTag using glob patterns
rg -n "from.*DateTag|import.*DateTag" -g "*.ts" -g "*.tsx" -A 2 -B 2
Length of output: 367
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: View DateTag.tsx complete implementation
cat src/pages/events/components/DateTag.tsx
Length of output: 861
🏁 Script executed:
#!/bin/bash
# Description: Check if [id].tsx has 'use client' directive
head -5 src/pages/mypage/events/[id].tsxLength of output: 259
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Check if [id].tsx has 'use client' anywhere in the file
rg -n "use client" src/pages/mypage/events/[id].tsx
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Description: Check file size and get full content of [id].tsx
wc -l src/pages/mypage/events/[id].tsx && cat src/pages/mypage/events/[id].tsxLength of output: 2252
'use client' 지시문 제거 필수
DateTag 컴포넌트는 클라이언트 상태나 이벤트 핸들러를 사용하지 않는 순수 프레젠테이션 컴포넌트입니다. 부모 컴포넌트인 src/pages/mypage/events/[id].tsx는 서버 컴포넌트이며 'use client' 지시문이 없습니다. 따라서 DateTag.tsx의 'use client' 지시문을 제거하세요.
'use client'; // ← 이 줄 제거
🤖 Prompt for AI Agents
In src/pages/events/components/DateTag.tsx around line 1, remove the top-level
'use client' directive because DateTag is a pure presentational component that
does not use client state or event handlers and its parent is a server
component; simply delete the "'use client';" line so the component remains a
server-compatible module.
| interface DateTagProps extends VariantProps<typeof dateTagStyle> { | ||
| startDate?: string; | ||
| endDate?: string; | ||
| className?: string; | ||
| } | ||
|
|
||
| const dateTagStyle = cva( | ||
| ` | ||
| inline-flex justify-center items-center whitespace-nowrap | ||
| w-full h-[4.5rem] | ||
| px-[1.3rem] py-[1.2rem] | ||
| rounded-[2rem] | ||
| border border-pink-100 | ||
| bg-pink-50 | ||
| text-pink-300 text-label-lg | ||
| `, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
불필요한 cva 및 VariantProps 사용 제거
dateTagStyle에 variant가 정의되어 있지 않으므로 class-variance-authority를 사용할 필요가 없습니다. 단순 className 문자열로 충분합니다.
다음과 같이 리팩토링하세요:
-import { cva, type VariantProps } from 'class-variance-authority';
import { cn } from '@/shared/lib';
-interface DateTagProps extends VariantProps<typeof dateTagStyle> {
+interface DateTagProps {
startDate?: string;
endDate?: string;
className?: string;
}
-const dateTagStyle = cva(
- `
+const dateTagStyle = `
inline-flex justify-center items-center whitespace-nowrap
w-full h-[4.5rem]
px-[1.3rem] py-[1.2rem]
rounded-[2rem]
border border-pink-100
bg-pink-50
text-pink-300 text-label-lg
- `,
-);
+`;그리고 Line 37에서:
- <div className={cn(dateTagStyle(), className)}>
+ <div className={cn(dateTagStyle, className)}>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/events/components/DateTag.tsx around lines 6 to 22, remove use of
class-variance-authority: change the DateTagProps interface to drop "extends
VariantProps<typeof dateTagStyle>" so it only defines startDate?: string;
endDate?: string; className?: string; replace the dateTagStyle cva(...)
declaration with a simple string const (e.g. const dateTagStyle = "inline-flex
justify-center items-center ... text-label-lg";) and remove any unused
cva/VariantProps imports; then update the usage at line 37 to treat dateTagStyle
as a string (concatenate with props.className) rather than passing variant
props.
| const EventSavePage = () => { | ||
| const router = useRouter(); | ||
| const { id } = router.query; | ||
|
|
||
| const event = eventData.find((e) => e.id === Number(id)); | ||
| if (!event) return null; |
There was a problem hiding this comment.
router.isReady 확인 필요
PostCardSavePage와 동일한 이슈입니다. Next.js Pages Router에서 router.query는 초기 렌더링 시 빈 객체이므로 router.isReady를 확인해야 합니다.
다음과 같이 수정하세요:
const EventSavePage = () => {
const router = useRouter();
const { id } = router.query;
+ if (!router.isReady) {
+ return null; // 또는 로딩 스피너
+ }
+
const event = eventData.find((e) => e.id === Number(id));
if (!event) return null;📝 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 EventSavePage = () => { | |
| const router = useRouter(); | |
| const { id } = router.query; | |
| const event = eventData.find((e) => e.id === Number(id)); | |
| if (!event) return null; | |
| const EventSavePage = () => { | |
| const router = useRouter(); | |
| const { id } = router.query; | |
| if (!router.isReady) { | |
| return null; // 또는 로딩 스피너 | |
| } | |
| const event = eventData.find((e) => e.id === Number(id)); | |
| if (!event) return null; |
🤖 Prompt for AI Agents
In src/pages/mypage/events/[id].tsx around lines 8 to 13, the code reads
router.query and uses id before Next.js router is ready; guard against this by
checking router.isReady before accessing router.query and before converting id
to Number — return null or a loading state when !router.isReady, then once ready
pull id from router.query, validate it, and only then find the event in
eventData.
| {imageSrc ? ( | ||
| <Image | ||
| src={imageSrc} | ||
| alt={`${name} 이미지`} | ||
| width={354} | ||
| height={430} | ||
| className={cn('w-full h-auto object-cover rounded-[2rem]')} | ||
| /> | ||
| ) : ( | ||
| <div | ||
| className={cn('w-full h-[43.6rem] bg-gray-200 rounded-[2rem]')} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
이미지와 placeholder 높이 불일치
Image 컴포넌트의 height={430} (43rem)과 placeholder의 h-[43.6rem] (43.6rem)이 일치하지 않습니다. 이는 이미지 로딩 시 레이아웃 시프트를 유발할 수 있습니다.
다음 중 하나로 수정하세요:
옵션 1: placeholder 높이를 image에 맞추기
) : (
<div
- className={cn('w-full h-[43.6rem] bg-gray-200 rounded-[2rem]')}
+ className={cn('w-full h-[43rem] bg-gray-200 rounded-[2rem]')}
/>옵션 2: image 높이를 placeholder에 맞추기
<Image
src={imageSrc}
alt={`${name} 이미지`}
width={354}
- height={430}
+ height={436}
className={cn('w-full h-auto object-cover rounded-[2rem]')}
/>📝 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.
| {imageSrc ? ( | |
| <Image | |
| src={imageSrc} | |
| alt={`${name} 이미지`} | |
| width={354} | |
| height={430} | |
| className={cn('w-full h-auto object-cover rounded-[2rem]')} | |
| /> | |
| ) : ( | |
| <div | |
| className={cn('w-full h-[43.6rem] bg-gray-200 rounded-[2rem]')} | |
| /> | |
| )} | |
| {imageSrc ? ( | |
| <Image | |
| src={imageSrc} | |
| alt={`${name} 이미지`} | |
| width={354} | |
| height={430} | |
| className={cn('w-full h-auto object-cover rounded-[2rem]')} | |
| /> | |
| ) : ( | |
| <div | |
| className={cn('w-full h-[43rem] bg-gray-200 rounded-[2rem]')} | |
| /> | |
| )} |
| {imageSrc ? ( | |
| <Image | |
| src={imageSrc} | |
| alt={`${name} 이미지`} | |
| width={354} | |
| height={430} | |
| className={cn('w-full h-auto object-cover rounded-[2rem]')} | |
| /> | |
| ) : ( | |
| <div | |
| className={cn('w-full h-[43.6rem] bg-gray-200 rounded-[2rem]')} | |
| /> | |
| )} | |
| {imageSrc ? ( | |
| <Image | |
| src={imageSrc} | |
| alt={`${name} 이미지`} | |
| width={354} | |
| height={436} | |
| className={cn('w-full h-auto object-cover rounded-[2rem]')} | |
| /> | |
| ) : ( | |
| <div | |
| className={cn('w-full h-[43.6rem] bg-gray-200 rounded-[2rem]')} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In src/pages/mypage/events/[id].tsx around lines 43 to 55, the Image component
uses height={430} (43rem) while the placeholder div uses class h-[43.6rem],
causing a mismatch and potential layout shift; fix by making the two heights
identical — either set the placeholder class to h-[430px] (or
h-[43rem]/equivalent) to match height={430}, or change the Image height prop to
441 (or the exact pixel value equivalent of 43.6rem) to match h-[43.6rem];
update the value in the component so both use the same height unit and numeric
value.
| const router = useRouter(); | ||
| const { id } = router.query; | ||
|
|
||
| const event = eventData.find((e) => e.id === Number(id)); | ||
| if (!event) return null; |
There was a problem hiding this comment.
router.isReady 확인 필요
Next.js Pages Router에서 router.query는 초기 렌더링 시 빈 객체이므로, id가 undefined가 될 수 있습니다. 이로 인해 eventData.find()가 올바르게 작동하지 않거나 예상치 못한 동작이 발생할 수 있습니다.
다음과 같이 수정하여 라우터가 준비될 때까지 로딩 상태를 표시하세요:
const PostCardSavePage = () => {
const router = useRouter();
const { id } = router.query;
+ if (!router.isReady) {
+ return null; // 또는 로딩 스피너
+ }
+
const event = eventData.find((e) => e.id === Number(id));
if (!event) return null;📝 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 router = useRouter(); | |
| const { id } = router.query; | |
| const event = eventData.find((e) => e.id === Number(id)); | |
| if (!event) return null; | |
| const router = useRouter(); | |
| const { id } = router.query; | |
| if (!router.isReady) { | |
| return null; // 또는 로딩 스피너 | |
| } | |
| const event = eventData.find((e) => e.id === Number(id)); | |
| if (!event) return null; |
🤖 Prompt for AI Agents
In src/pages/mypage/postcard/[id].tsx around lines 8 to 12, router.query can be
empty on initial render so using id immediately causes incorrect lookup; guard
on router.isReady before accessing router.query (or return a loading state) and
only call eventData.find once router.isReady is true, rendering a loading
indicator while waiting and then handling the not-found case after id is
available.
2 participants
🔥 작업 내용
🤔 추후 작업 사항
🔗 이슈
PR Point (To Reviewer)
행사 & 엽서 저장 페이지
기존 Node.tsx와 events/[id].tsx 의 레이아웃 에서 크게 바뀌지 않은 형태입니다. 카드 플립은 정민님이 따로 컴포넌트로 분리해주신다고 해 가져올 예정입니다.
📸 피그마 스크린샷 or 기능 GIF
Summary by CodeRabbit
릴리스 노트