Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough세션 이미지 캐러셀 컴포넌트에서 최대 이미지 개수(5개) 제한을 추가하고, 이미지 순서 계산 로직을 개선하며, 스키마 필드를 선택적으로 변경하고 커스텀 스크롤바 유틸리티를 추가했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/homepage/src/app/(with-header)/mypage/admin/sessions/_hooks/useSessionImageCarousel.tsx (1)
64-67:orderfallback 처리 및 인덱스 설정 확인 필요Line 64의
newImage.order ?? nextOrderfallback은 백엔드가order를 반환하지 않을 경우를 잘 처리합니다.Line 67의
setCurrentIndex(images.length)는 클로저 특성상 업데이트 전images배열 길이를 참조하므로, 새로 추가된 이미지의 인덱스를 올바르게 가리킵니다. (예: 기존 3개 → 새 이미지 인덱스 3)다만, 이 동작이 의도적임을 명확히 하기 위해 주석을 추가하면 유지보수에 도움이 됩니다.
📝 선택적 주석 추가
setCurrentIndex(images.length); + // images.length는 클로저로 인해 업데이트 전 값을 참조하므로, + // 새로 추가된 이미지의 인덱스(0-based)와 일치합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/homepage/src/app/`(with-header)/mypage/admin/sessions/_hooks/useSessionImageCarousel.tsx around lines 64 - 67, The fallback newImage.order ?? nextOrder correctly handles missing order from the backend but the intent of using setCurrentIndex(images.length) (which relies on the pre-update images.length to point at the newly appended image) is not obvious; add a short clarifying comment next to the setCurrentIndex(images.length) call explaining that images.length is intentionally used before state update because the new image is appended to the end and its index equals the previous length, and mention newImage.order and nextOrder to show the fallback behavior is deliberate.apps/homepage/src/app/(with-header)/mypage/admin/sessions/_components/carousel/SessionImageCarouselEdit.tsx (1)
141-145: 접근성 개선 제안:disabled속성 추가현재 스타일로만 비활성화 상태를 표현하고 있습니다. 스크린 리더 사용자를 위해
disabled속성을 추가하면 접근성이 향상됩니다.♿ 접근성 개선 제안
<button type='button' + disabled={!canAddMore} onClick={() => { if (!canAddMore) { alert(`이미지는 최대 ${MAX_IMAGES}장까지 추가할 수 있습니다.`); return; } fileInputRef.current?.click(); }} className={`flex h-20 w-20 shrink-0 flex-col items-center justify-center gap-1 rounded-lg border border-dashed ${ canAddMore ? 'cursor-pointer border-neutral-300 bg-neutral-50' - : 'cursor-not-allowed border-neutral-300 bg-neutral-50 opacity-50' + : 'border-neutral-300 bg-neutral-50 opacity-50 disabled:cursor-not-allowed' }`} aria-label='이미지 추가'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/homepage/src/app/`(with-header)/mypage/admin/sessions/_components/carousel/SessionImageCarouselEdit.tsx around lines 141 - 145, The element using the conditional className in SessionImageCarouselEdit is only visually disabled; update the interactive element so it uses a real disabled state: if it’s a <button> set disabled={!canAddMore} (and keep the existing className logic), otherwise replace the div with a <button> or add aria-disabled={String(!canAddMore)} plus role="button" and tabIndex={-1} when !canAddMore; reference the canAddMore variable and the JSX block with the conditional className to locate where to apply disabled/aria-disabled.apps/homepage/src/styles/globals.css (1)
60-81: LGTM! 커스텀 스크롤바 스타일링 잘 추가되었습니다.Tailwind
@layer utilities를 활용한 좋은 패턴입니다. 다만, 하드코딩된 색상값(#737373,#e5e5e5) 대신 Tailwind 테마 색상(neutral-500,neutral-200)을 사용하면 디자인 시스템 일관성이 더 좋아질 수 있습니다.♻️ 선택적 개선 제안
`@layer` utilities { .custom-scrollbar { scrollbar-width: thin; - scrollbar-color: `#737373` `#e5e5e5`; + scrollbar-color: theme('colors.neutral.500') theme('colors.neutral.200'); } .custom-scrollbar::-webkit-scrollbar-track { - background: `#e5e5e5`; + background: theme('colors.neutral.200'); border-radius: 10px; } .custom-scrollbar::-webkit-scrollbar-thumb { - background-color: `#737373`; + background-color: theme('colors.neutral.500'); border-radius: 10px; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/homepage/src/styles/globals.css` around lines 60 - 81, Replace the hardcoded hex colors in the .custom-scrollbar rules with Tailwind theme tokens so the styles follow the design system: change occurrences of `#737373` to theme('colors.neutral.500') and `#e5e5e5` to theme('colors.neutral.200') in the .custom-scrollbar, .custom-scrollbar::-webkit-scrollbar-track, and .custom-scrollbar::-webkit-scrollbar-thumb selectors (use the Tailwind theme() helper in your CSS).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/homepage/src/app/`(with-header)/mypage/admin/sessions/_components/carousel/SessionImageCarouselEdit.tsx:
- Around line 141-145: The element using the conditional className in
SessionImageCarouselEdit is only visually disabled; update the interactive
element so it uses a real disabled state: if it’s a <button> set
disabled={!canAddMore} (and keep the existing className logic), otherwise
replace the div with a <button> or add aria-disabled={String(!canAddMore)} plus
role="button" and tabIndex={-1} when !canAddMore; reference the canAddMore
variable and the JSX block with the conditional className to locate where to
apply disabled/aria-disabled.
In
`@apps/homepage/src/app/`(with-header)/mypage/admin/sessions/_hooks/useSessionImageCarousel.tsx:
- Around line 64-67: The fallback newImage.order ?? nextOrder correctly handles
missing order from the backend but the intent of using
setCurrentIndex(images.length) (which relies on the pre-update images.length to
point at the newly appended image) is not obvious; add a short clarifying
comment next to the setCurrentIndex(images.length) call explaining that
images.length is intentionally used before state update because the new image is
appended to the end and its index equals the previous length, and mention
newImage.order and nextOrder to show the fallback behavior is deliberate.
In `@apps/homepage/src/styles/globals.css`:
- Around line 60-81: Replace the hardcoded hex colors in the .custom-scrollbar
rules with Tailwind theme tokens so the styles follow the design system: change
occurrences of `#737373` to theme('colors.neutral.500') and `#e5e5e5` to
theme('colors.neutral.200') in the .custom-scrollbar,
.custom-scrollbar::-webkit-scrollbar-track, and
.custom-scrollbar::-webkit-scrollbar-thumb selectors (use the Tailwind theme()
helper in your CSS).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/homepage/src/app/(with-header)/mypage/admin/sessions/_components/carousel/SessionImageCarouselEdit.tsxapps/homepage/src/app/(with-header)/mypage/admin/sessions/_hooks/useSessionImageCarousel.tsxapps/homepage/src/schemas/admin/admin-sessions.schema.tsapps/homepage/src/styles/globals.css
ISSUE 🔗
close #227
What is this PR? 🔍
이미지 업로드 및 정렬 로직
order속성을 할당하는 로직이 개선되었습니다. 이제 기존 이미지 중 가장 높은 순서 값을 기준으로 새 번호를 할당하여, 이미지가 삭제되거나 재정렬된 후에도 올바른 위치에 추가되도록 보장합니다.order와s3Key필드를 선택 사항(optional)으로 허용합니다.order값이 제공되지 않을 경우 기본값0으로 설정하여 백엔드 통신의 견고함을 높였습니다.사용자 경험(UX) 개선 사항
MAX_IMAGES)에 도달하면 알림창을 표시하고 버튼을 시각적으로 비활성화하며, 비활성 상태를 반영하도록 버튼 스타일이 업데이트됩니다.Screenshot 📷
2026-03-02.5.31.55.mov
Test Checklist ✔