Conversation
WalkthroughImageGallery와 MediaUpload 컴포넌트를 새로 추가하고 각 컴포넌트용 Storybook 스토리를 도입했습니다. ImageGallery는 컨텍스트 기반 내비게이션(화살표/도트)과 공개 타입/컴포넌트를 제공하며, MediaUpload는 단일/다중 업로드, 용량 검증, 미리보기, 제거/리셋 및 관련 아이콘( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant MU as MediaUpload
participant Input as input[type=file]
participant Preview as MediaPreview
participant CB as onFileUpload (prop)
User->>MU: 컴포넌트 렌더
MU->>Input: accept/multiple 바인딩
User->>Input: 파일 선택
Input-->>MU: FileList
MU->>MU: 용량검증, 선택(single/multiple)
MU->>MU: object URL 생성, state 업데이트
MU-->>CB: 유효 파일 배열 또는 null
MU->>Preview: files, previewUrls 전달
alt 파일 제거
User->>Preview: 제거 클릭(index)
Preview-->>MU: onRemoveFile(index)
MU->>MU: 파일/프리뷰 삭제, state 업데이트
MU-->>CB: 남은 파일 배열 또는 null
end
alt 리셋
User->>MU: Reset 클릭
MU->>MU: state 초기화
MU-->>CB: null
end
sequenceDiagram
autonumber
actor User
participant IG as ImageGallery
participant Ctx as ImageGalleryProvider
participant Content as ImageGalleryContent
participant UI as Arrows/Dots
User->>IG: images 전달
IG->>Ctx: Provider 초기화(current=0, total)
Ctx->>Content: 상태 전달(current, first/last)
User->>UI: Next/Prev 클릭
UI->>Ctx: goNext()/goPrev()
Ctx->>Ctx: current 업데이트(경계 클램프)
Ctx-->>Content: 업데이트 반영
User->>UI: 도트 클릭(i)
UI->>Ctx: goToIndex(i)
Ctx-->>Content: 업데이트 반영
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ 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 |
|
Update: 2025년 10월 12일 13시 52분 58초 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
src/shared/ui/assets/refresh.svgis excluded by!**/*.svgsrc/shared/ui/assets/upload.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
src/shared/ui/ImageGallery/ImageGallery.stories.tsx(1 hunks)src/shared/ui/ImageGallery/ImageGallery.tsx(1 hunks)src/shared/ui/ImageGallery/ImageGalleryContext.tsx(1 hunks)src/shared/ui/ImageGallery/index.ts(1 hunks)src/shared/ui/MediaUpload/MediaUpload.stories.tsx(1 hunks)src/shared/ui/MediaUpload/MediaUpload.tsx(1 hunks)src/shared/ui/MediaUpload/MediaUploadPreview.tsx(1 hunks)src/shared/ui/MediaUpload/index.ts(1 hunks)src/shared/ui/assets/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
Props(9-48)
src/shared/ui/ImageGallery/ImageGallery.stories.tsx (1)
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
ImageGallery(24-30)
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
src/shared/ui/ImageGallery/ImageGalleryContext.tsx (2)
ImageGalleryProvider(37-61)useImageGallery(27-29)
src/shared/ui/MediaUpload/MediaUpload.stories.tsx (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (2)
MediaUpload(52-130)Props(9-48)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
MediaPreview(10-29)
🪛 Biome (2.1.2)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx
[error] 39-39: Provide a track for captions when using audio or video elements.
Captions support users with hearing-impairments. They should be a transcription or translation of the dialogue, sound effects, musical cues, and other relevant audio information.
(lint/a11y/useMediaCaption)
[error] 19-22: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (7)
src/shared/ui/ImageGallery/index.ts (1)
1-1: LGTM!컴포넌트를 깔끔하게 re-export하고 있습니다.
src/shared/ui/ImageGallery/ImageGalleryContext.tsx (1)
41-46: LGTM!
goPrev,goNext,goToIndex함수들이 경계값을 올바르게 처리하고 있으며,useCallback의 의존성 배열도 정확합니다.src/shared/ui/ImageGallery/ImageGallery.tsx (1)
24-30: 빈 배열 처리 추가 필요
images가 비어 있으면ImageGalleryContent에서images[current]가undefined를 반환해 런타임 에러가 발생할 수 있습니다. 컴포넌트 최상단에if (images.length === 0) return null;또는 적절한 빈 상태 UI 렌더링을 추가하세요.src/shared/ui/assets/index.tsx (1)
16-19: LGTM! 아이콘 추가가 올바르게 구현되었습니다.새로운
Refresh와Upload아이콘이 기존 패턴을 일관되게 따르고 있으며, MediaUpload 컴포넌트에서 필요로 하는 아이콘들을 적절히 제공하고 있습니다.Also applies to: 41-42
src/shared/ui/MediaUpload/index.ts (1)
1-1: LGTM! 표준 re-export 패턴을 따르고 있습니다.공개 API 표면을 명확하게 정의하는 올바른 barrel export입니다.
src/shared/ui/MediaUpload/MediaUpload.stories.tsx (1)
1-44: LGTM! Storybook 스토리가 잘 구성되어 있습니다.다양한 사용 사례(Basic, UploadFormats, WithTopAffix, MultipleMode)를 적절히 커버하고 있으며, argTypes를 통한 인터랙티브 컨트롤도 잘 설정되어 있습니다.
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
132-172: LGTM! 서브 컴포넌트들이 잘 구현되어 있습니다.
UploadBox와RefreshButton컴포넌트는 명확한 책임을 가지고 있으며, 접근성을 고려한 구현(label의 htmlFor, button의 aria-label)이 잘 되어 있습니다.
| <button | ||
| key={index} | ||
| type="button" | ||
| aria-label={`이미지 ${index + 1}`} | ||
| onClick={() => goToIndex(index)} | ||
| className={cn('h-2 w-2 rounded-full', isActive ? 'bg-primary-300' : 'bg-gray-200')} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
접근성: 점(dot) 버튼에 현재 상태 표시가 필요합니다.
현재 활성화된 이미지를 나타내는 dot에 aria-current="true" 속성이 없어, 스크린 리더 사용자가 현재 위치를 알 수 없습니다.
다음과 같이 수정하세요:
<button
key={index}
type="button"
aria-label={`이미지 ${index + 1}`}
+ aria-current={isActive ? 'true' : 'false'}
onClick={() => goToIndex(index)}📝 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.
| <button | |
| key={index} | |
| type="button" | |
| aria-label={`이미지 ${index + 1}`} | |
| onClick={() => goToIndex(index)} | |
| className={cn('h-2 w-2 rounded-full', isActive ? 'bg-primary-300' : 'bg-gray-200')} | |
| /> | |
| ); | |
| <button | |
| key={index} | |
| type="button" | |
| aria-label={`이미지 ${index + 1}`} | |
| aria-current={isActive ? 'true' : 'false'} | |
| onClick={() => goToIndex(index)} | |
| className={cn('h-2 w-2 rounded-full', isActive ? 'bg-primary-300' : 'bg-gray-200')} | |
| /> | |
| ); |
🤖 Prompt for AI Agents
In src/shared/ui/ImageGallery/ImageGallery.tsx around lines 64 to 71, the dot
button lacks an accessibility indicator for the currently active image; update
the button to include an aria-current attribute when isActive is true (e.g.,
aria-current="true" or aria-current={isActive ? "true" : undefined}) so screen
readers can announce the current slide, leaving markup and styling otherwise
unchanged.
| export function ImageGalleryProvider({ images, altPrefix, children }: ProviderProps) { | ||
| const [current, setCurrent] = useState(0); | ||
| const total = images.length; | ||
|
|
||
| const goPrev = useCallback(() => setCurrent((c) => Math.max(0, c - 1)), []); | ||
| const goNext = useCallback(() => setCurrent((c) => Math.min(total - 1, c + 1)), [total]); | ||
| const goToIndex = useCallback( | ||
| (index: number) => setCurrent(Math.max(0, Math.min(total - 1, index))), | ||
| [total] | ||
| ); | ||
|
|
||
| const value: ImageGalleryContextType = { | ||
| images, | ||
| current, | ||
| altPrefix, | ||
| total, | ||
| firstImage: current === 0, | ||
| lastImage: current === total - 1, | ||
| goPrev, | ||
| goNext, | ||
| goToIndex, | ||
| }; | ||
|
|
||
| return <ImageGalleryContext.Provider value={value}>{children}</ImageGalleryContext.Provider>; | ||
| } |
There was a problem hiding this comment.
빈 배열에 대한 엣지 케이스 처리가 필요합니다.
images 배열이 비어있을 때(total === 0), Line 54의 lastImage: current === total - 1은 current === -1이 되어 항상 false를 반환합니다. 또한 빈 배열일 때 firstImage와 lastImage 모두 true가 되어야 의미상 맞습니다.
다음과 같이 수정하는 것을 권장합니다:
const value: ImageGalleryContextType = {
images,
current,
altPrefix,
total,
- firstImage: current === 0,
- lastImage: current === total - 1,
+ firstImage: total === 0 || current === 0,
+ lastImage: total === 0 || current === total - 1,
goPrev,
goNext,
goToIndex,
};또는 ImageGallery 컴포넌트 레벨에서 빈 배열을 early return으로 처리하는 것이 더 나을 수 있습니다 (ImageGallery.tsx 참조).
📝 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 ImageGalleryProvider({ images, altPrefix, children }: ProviderProps) { | |
| const [current, setCurrent] = useState(0); | |
| const total = images.length; | |
| const goPrev = useCallback(() => setCurrent((c) => Math.max(0, c - 1)), []); | |
| const goNext = useCallback(() => setCurrent((c) => Math.min(total - 1, c + 1)), [total]); | |
| const goToIndex = useCallback( | |
| (index: number) => setCurrent(Math.max(0, Math.min(total - 1, index))), | |
| [total] | |
| ); | |
| const value: ImageGalleryContextType = { | |
| images, | |
| current, | |
| altPrefix, | |
| total, | |
| firstImage: current === 0, | |
| lastImage: current === total - 1, | |
| goPrev, | |
| goNext, | |
| goToIndex, | |
| }; | |
| return <ImageGalleryContext.Provider value={value}>{children}</ImageGalleryContext.Provider>; | |
| } | |
| export function ImageGalleryProvider({ images, altPrefix, children }: ProviderProps) { | |
| const [current, setCurrent] = useState(0); | |
| const total = images.length; | |
| const goPrev = useCallback(() => setCurrent((c) => Math.max(0, c - 1)), []); | |
| const goNext = useCallback(() => setCurrent((c) => Math.min(total - 1, c + 1)), [total]); | |
| const goToIndex = useCallback( | |
| (index: number) => setCurrent(Math.max(0, Math.min(total - 1, index))), | |
| [total] | |
| ); | |
| const value: ImageGalleryContextType = { | |
| images, | |
| current, | |
| altPrefix, | |
| total, | |
| firstImage: total === 0 || current === 0, | |
| lastImage: total === 0 || current === total - 1, | |
| goPrev, | |
| goNext, | |
| goToIndex, | |
| }; | |
| return <ImageGalleryContext.Provider value={value}>{children}</ImageGalleryContext.Provider>; | |
| } |
🤖 Prompt for AI Agents
In src/shared/ui/ImageGallery/ImageGalleryContext.tsx around lines 37 to 61,
handle the empty images array edge case: when total === 0 treat the gallery as
both first and last image and ensure navigation is a no-op. Update computed
flags to something like firstImage = total === 0 || current === 0 and lastImage
= total === 0 || current === total - 1, and ensure goPrev/goNext/goToIndex
either early-return or clamp using total > 0 so they don’t produce invalid
indices when images is empty.
|
|
||
| type Props = { | ||
| /** | ||
| * 이미지 URL 목록 |
There was a problem hiding this comment.
통일감 있게 영어로 써주시면 좋을 것 같아요~!
| <div className={cn('relative h-[500px] w-[500px] overflow-hidden rounded-lg', className)}> | ||
| <img | ||
| src={images[current]} | ||
| alt={`${altPrefix}${current + 1}`} |
There was a problem hiding this comment.
alt에는 이미지 네이밍을 넣어주면 어떨까요??
There was a problem hiding this comment.
이미지 네이밍은 어떤 것을 의미하는 것일까요?
개발자가 넣어주는 것인지, 사용자가 입력한 파일명을 의미하신 것인지 헷갈려서요!
There was a problem hiding this comment.
사용자가 업로드한 파일의 파일명 말한거였어요!
There was a problem hiding this comment.
fix: altPrefix -> filename 대체
파일명 업로드 가능하게 컴포넌트단에서 수정해두었는데, 현재는 서버에서 파일명없이 url만 응답받고 있어서 서비스 적용을 위해서는 응답형식 변경이 필요해보여 문의드려두었습니다! 요것은 물론 이미 아시겠지만 업참바로 공유드립니다ㅎㅎ 🙇
There was a problem hiding this comment.
빠른 공유 좋으네요~!
이미지 갤러리가 실제 get 조회할 때 사용된다는 가정하에 생각해보면, api 응답에 파일명도 같이 내려오니 해당 네이밍을 그대로 사용하면 될 것 같은데용.( 이렇게 된다면 서버에서 구조를 바꾸지 않아도 될 것 같아서요!) 요 의견은 어떠신가요??
| src={images[current]} | ||
| alt={`${altPrefix}${current + 1}`} | ||
| className="h-full w-full object-scale-down" | ||
| loading="lazy" |
There was a problem hiding this comment.
첫 이미지는 lazy 속성보다는 기본 속성이 eager를 사용해 즉각적으로 다운로드 받고, 나머지 이미지는 lazy를 적용하는거 어떠신가요?!
There was a problem hiding this comment.
넘 좋아용 감사합니다 fix: 첫번쨰 이미지만 image loading eager 적용
| return ( | ||
| <Flex dir="col" alignItems="center"> | ||
| <div className={cn('relative h-[500px] w-[500px] overflow-hidden rounded-lg', className)}> | ||
| <img |
There was a problem hiding this comment.
width, height 속성이 디폴트로 있으면 좋을 것 같아요!
There was a problem hiding this comment.
| const oversizedFiles = files.filter((file) => file.size / GB > maxSize); | ||
| if (oversizedFiles.length > 0) { | ||
| return alert(`${maxSize}GB 이하의 파일로 등록해주세요.`); |
There was a problem hiding this comment.
요 alert는 사용하는 측에서 제어하는게 좋은 것 같아요!
There was a problem hiding this comment.
| handleReset: () => void; | ||
| isSelected: boolean; | ||
| }; | ||
| function RefreshButton({ handleReset, isSelected }: RefreshButtonProp) { |
There was a problem hiding this comment.
저는 요 버튼이 박스 영역 밖에 있어서 초기화버튼을 누를때 레이아웃 쉬프트가 발생하는 것처럼 느껴지더라구요! 개인적으로는 해당 버튼이 박스 내부에 존재해서, 일정한 heigth을 유지하는게 좋을 것 같은데 어떻게 생각하시나용?
There was a problem hiding this comment.
제가 피그마를 다시 살펴봤는데, 항상 노출되는 요소이더라고요. 아래 두가지 경우를 함께 고려하다보니, 항상 노출하면서 disabled 처리를 적용하는게 디자인과 UX를 같이 챙길 수 있는 방법이라고 판단되어서 사진과 같이 수정진행했습니다!
fix: refresh button 노출방식 수정
<고려한 부분>
- 다중 이미지 업로드를 지원하는 경우 각 이미지에 x버튼이 위치
- 좌측에 문구가 들어가는 케이스의 레이아웃 조화
<수정 후>
| return ( | ||
| <div className="relative rounded-xl border border-gray-200"> | ||
| {file.type.startsWith('video/') ? ( | ||
| <video src={previewUrl} controls className="h-[200px] w-full object-scale-down" /> |
There was a problem hiding this comment.
이미지 갤러리에서는 원본 크기가 그대로 표시되지만, 업로드 시에는 height가 고정되어 있어서 실제 이미지 크기를 예측하기 어렵다고 느꼈어요!
현재처럼 200px로 고정하기보다는 sm, md, lg, xl 같은 사이즈 단위로 구분하거나, 커스텀 설정이 가능하도록 해보는 건 어떨까요??
There was a problem hiding this comment.
| const { images, current, total, altPrefix } = useImageGallery(); | ||
| return ( | ||
| <Flex dir="col" alignItems="center"> | ||
| <div className={cn('relative h-[500px] w-[500px] overflow-hidden rounded-lg', className)}> |
There was a problem hiding this comment.
값이 고정되어 있어 반응형 지원이 안되는 것 같아요! 확인 부탁 드려용
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
src/shared/ui/ImageGallery/ImageGalleryContext.tsx (1)
53-54: 빈 배열 엣지 케이스 미처리 확인이전 리뷰에서 지적된
images배열이 비어있을 때(total === 0)의 엣지 케이스가 아직 처리되지 않았습니다.현재:
lastImage: current === total - 1→0 === -1→ 항상falsefirstImage: current === 0→ 항상true이는 논리적으로 불일치하며, 빈 갤러리의 경우 두 플래그가 모두
true이어야 합니다.이전 리뷰 코멘트를 참조하여 다음과 같이 수정하세요:
const value: ImageGalleryContextType = { images, current, altPrefix, total, - firstImage: current === 0, - lastImage: current === total - 1, + firstImage: total === 0 || current === 0, + lastImage: total === 0 || current === total - 1, goPrev, goNext, goToIndex, };src/shared/ui/ImageGallery/ImageGallery.tsx (2)
69-69: aria-label 언어 일관성 확보
aria-label이 영어("Image")로 작성되어 있습니다. 다른 UI 텍스트와의 일관성을 위해 한국어 또는 다국어 지원을 고려하세요.Based on learnings (keemsebin의 통일성 관련 코멘트 참조)
- aria-label={`Image ${index + 1}`} + aria-label={`이미지 ${index + 1}`}또는 다국어 지원이 필요하다면 i18n 시스템을 활용하세요.
39-44: 이미지에 명시적 width/height 속성 추가 권장
<img>요소에width와height속성이 없어 브라우저가 이미지 로드 전 레이아웃 공간을 예약하지 못합니다. 이로 인해 CLS(Cumulative Layout Shift)가 발생할 수 있습니다.Based on learnings (keemsebin의 코멘트 참조)
다음과 같이 수정하는 것을 권장합니다:
<img src={images[current]} className="h-full w-full object-scale-down" loading={loading} alt={alt} + width={500} + height={500} />참고: 반응형 크기 조정이 필요한 경우, 상위 컨테이너의 하드코딩된 크기를 제거하고 (이전 리뷰 코멘트 참조) 이미지의 실제 종횡비를 사용하거나
aspect-ratioCSS를 활용하세요.src/shared/ui/MediaUpload/MediaUpload.tsx (5)
12-12: JSDoc 오타를 수정하세요.
@defualt를@default로 수정해야 합니다.다음 diff를 적용하세요:
/** * id of the file input. - * @defualt random string value + * @default random string value */
66-68: 메모리 누수: 컴포넌트 언마운트 시 Object URL을 정리하지 않고 있습니다.컴포넌트가 언마운트될 때 생성된 모든 object URL을 해제하지 않으면 메모리 누수가 발생합니다.
useEffect를 추가하여 cleanup을 구현하세요:+import { ComponentProps, useEffect, useId, useState } from 'react'; -import { ComponentProps, useId, useState } from 'react'; // ... 기존 코드 ... const [selectedFiles, setSelectedFiles] = useState<File[]>([]); const [previewUrls, setPreviewUrls] = useState<string[]>([]); const isSelected = selectedFiles.length > 0; + // 컴포넌트 언마운트 시 모든 object URLs 정리 + useEffect(() => { + return () => { + previewUrls.forEach((url) => URL.revokeObjectURL(url)); + }; + }, [previewUrls]); + const handleReset = () => {
70-74: 메모리 누수: Object URL을 해제하지 않고 있습니다.
handleReset에서previewUrls를 초기화하기 전에URL.revokeObjectURL()을 호출하여 메모리 누수를 방지해야 합니다.다음 diff를 적용하세요:
const handleReset = () => { + previewUrls.forEach((url) => URL.revokeObjectURL(url)); setSelectedFiles([]); setPreviewUrls([]); onFileUpload?.(null); };
76-91: 메모리 누수: 기존 Object URL을 해제하지 않고 새 파일을 선택하고 있습니다.새 파일을 선택할 때 기존
previewUrls의 object URL들을 해제하지 않으면 메모리 누수가 발생합니다.다음 diff를 적용하여 기존 URL들을 먼저 해제하세요:
const handleFileChange = (event: React.ChangeEvent<HTMLInputElement>) => { const files = Array.from(event.target.files || []); if (files.length === 0) { return handleReset(); } const oversizedFiles = files.filter((file) => file.size / GB > maxSize); if (oversizedFiles.length > 0) { const message = `${maxSize}GB 이하의 파일로 등록해주세요.`; if (onError) { onError(message); } else { alert(message); } return; } + // 기존 object URLs 해제 + previewUrls.forEach((url) => URL.revokeObjectURL(url)); + const validatedFiles = multiple ? files : [files[0]]; setSelectedFiles(validatedFiles); const urls = validatedFiles.map((file) => URL.createObjectURL(file)); setPreviewUrls(urls); onFileUpload?.(validatedFiles); };
93-100: 메모리 누수: 제거된 파일의 Object URL을 해제하지 않고 있습니다.파일을 제거할 때 해당 object URL을
URL.revokeObjectURL()로 해제하지 않으면 메모리 누수가 발생합니다.다음 diff를 적용하세요:
const handleRemoveFile = (index: number) => { + // 제거되는 URL 해제 + URL.revokeObjectURL(previewUrls[index]); + const newFiles = selectedFiles.filter((_, i) => i !== index); const newUrls = previewUrls.filter((_, i) => i !== index); setSelectedFiles(newFiles); setPreviewUrls(newUrls); onFileUpload?.(newFiles.length > 0 ? newFiles : null); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/shared/ui/ImageGallery/ImageGallery.tsx(1 hunks)src/shared/ui/ImageGallery/ImageGalleryContext.tsx(1 hunks)src/shared/ui/MediaUpload/MediaUpload.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
src/shared/ui/ImageGallery/ImageGalleryContext.tsx (2)
ImageGalleryProvider(37-61)useImageGallery(27-29)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
MediaPreview(10-29)
🪛 GitHub Actions: Check Pull Request
src/shared/ui/ImageGallery/ImageGalleryContext.tsx
[error] 27-27: ESLint: react-refresh/only-export-components - Fast refresh only works when a file exports only components. Too many warnings; ESLint exited with code 1 during 'npm run lint'.
⏰ 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: preview-storybook
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (2)
18-18: 인덱스를 key로 사용하는 것은 여전히 위험합니다.파일을 동적으로 추가하거나 제거할 때 인덱스 기반 key는 React의 reconciliation 문제를 일으킬 수 있습니다. 이전 리뷰에서 제안된 대로 파일 이름과 타임스탬프 조합을 사용하세요.
다음 diff를 적용하세요:
- {files?.map((file, index) => ( - <div key={index} className="relative aspect-square"> + {files?.map((file, index) => ( + <div key={`${file.name}-${file.lastModified}-${index}`} className="relative aspect-square">
45-49: 접근성: video 요소에 자막 트랙을 추가하세요.청각 장애가 있는 사용자를 위해 video 요소에는 captions 트랙이 필요합니다. 이는 이전 리뷰와 정적 분석 도구에서도 지적된 사항입니다.
video 요소에
<track>요소를 추가하세요:{file.type.startsWith('video/') ? ( - <video - src={previewUrl} - controls - className="h-full max-h-[500px] w-full max-w-[500px] object-contain" - /> + <video + src={previewUrl} + controls + className="h-full max-h-[500px] w-full max-w-[500px] object-contain" + > + <track kind="captions" /> + </video>참고: 실제 자막 파일(
.vtt등)을 제공할 수 없는 경우, 사용자에게 자막 파일 업로드 옵션을 제공하거나 컴포넌트 문서에 접근성 요구사항을 명시하는 것을 권장합니다.src/shared/ui/MediaUpload/MediaUpload.tsx (4)
82-85: Critical: 이벤트 핸들러에서 Error를 throw하면 앱이 크래시될 수 있습니다.이전 리뷰에서 지적된 대로, 이벤트 핸들러에서
throw new Error()를 사용하면 에러 바운더리가 없는 경우 애플리케이션이 크래시될 수 있습니다.Props에 선택적 오류 콜백을 추가하세요:
export type Props = { // ... 기존 props + /** + * callback when file validation fails + */ + onError?: (message: string) => void; // ... } & Omit<ComponentProps<'input'>, 'id'>;그리고 handleFileChange에서 사용:
if (oversizedFiles.length > 0) { - throw new Error(`${maxSize}GB 이하의 파일로 등록해주세요.`); + const message = `${maxSize}GB 이하의 파일로 등록해주세요.`; + if (onError) { + onError(message); + } else { + console.error(message); + } + return; }
66-68: 메모리 누수: 컴포넌트 언마운트 시 Object URL을 정리하지 않고 있습니다.이전 리뷰에서 지적된 대로, 컴포넌트가 언마운트될 때 생성된 모든 object URL을 해제하지 않으면 메모리 누수가 발생합니다.
useEffect를 추가하여 cleanup을 구현하세요:-import { ComponentProps, useId, useState } from 'react'; +import { ComponentProps, useEffect, useId, useState } from 'react'; // ... 기존 코드 ... const [selectedFiles, setSelectedFiles] = useState<File[]>([]); const [previewUrls, setPreviewUrls] = useState<string[]>([]); const isSelected = selectedFiles.length > 0; + // 컴포넌트 언마운트 시 object URLs 정리 + useEffect(() => { + return () => { + previewUrls.forEach((url) => URL.revokeObjectURL(url)); + }; + }, [previewUrls]); + const handleReset = () => {
77-92: 메모리 누수: 기존 Object URL을 해제하지 않고 새 파일을 선택하고 있습니다.이전 리뷰에서 지적된 대로, 새 파일을 선택할 때 기존
previewUrls의 object URL들을 해제하지 않으면 메모리 누수가 발생합니다.다음 diff를 적용하여 기존 URL들을 먼저 해제하세요:
const handleFileChange = (event: React.ChangeEvent<HTMLInputElement>) => { const files = Array.from(event.target.files || []); if (files.length === 0) { return handleReset(); } const oversizedFiles = files.filter((file) => file.size / GB > maxSize); if (oversizedFiles.length > 0) { throw new Error(`${maxSize}GB 이하의 파일로 등록해주세요.`); } + // 기존 object URLs 해제 + previewUrls.forEach((url) => URL.revokeObjectURL(url)); + const validatedFiles = multiple ? files : [files[0]]; setSelectedFiles(validatedFiles); const urls = validatedFiles.map((file) => URL.createObjectURL(file)); setPreviewUrls(urls); onFileUpload?.(validatedFiles); };
161-174: 선택사항: 레이아웃 시프트를 방지하기 위해 RefreshButton 위치를 조정하는 것을 고려하세요.이전 리뷰에서 제안된 대로, RefreshButton이 파일 선택 여부에 따라 활성/비활성 상태가 변경되면서 시각적 피드백이 부족할 수 있습니다. 버튼을 항상 렌더링하되
visibility나opacity를 사용하여 레이아웃을 보존하는 것을 고려해보세요.예시:
return ( <Flex as="button" onClick={isSelected ? handleReset : undefined} alignItems="center" - className={`text-primary-300 justify-end ${isSelected ? 'cursor-pointer' : 'cursor-not-allowed opacity-50'}`} + className={`text-primary-300 justify-end ${isSelected ? 'cursor-pointer' : 'invisible'}`} aria-label="초기화" > <Icon name="refresh" size={24} color="primary" /> <Body1>초기화</Body1> </Flex> );src/shared/ui/ImageGallery/ImageGalleryContext.tsx (1)
52-53: 빈 배열에 대한 엣지 케이스 처리 필요
total === 0일 때lastImage: current === total - 1은0 === -1이 되어 항상false를 반환합니다. 의미상 빈 갤러리는 첫 번째이자 마지막 이미지로 간주되어야 합니다.다음과 같이 수정하세요:
- firstImage: current === 0, - lastImage: current === total - 1, + firstImage: total === 0 || current === 0, + lastImage: total === 0 || current === total - 1,src/shared/ui/ImageGallery/ImageGallery.tsx (3)
21-27: 빈 배열 검증 추가 필요
images배열이 비어있을 때에 대한 처리가 없습니다. 빈 배열이 전달되면 빈 컨테이너가 렌더링되고 내비게이션이 잘못 작동할 수 있습니다.다음과 같이 early return을 추가하세요:
export function ImageGallery({ images, className }: Props) { + if (!images || images.length === 0) { + return null; + } return ( <ImageGalleryProvider images={images}>
29-60: 접근성: 키보드 내비게이션 지원 필요키보드 사용자를 위한 방향키 내비게이션이 구현되어 있지 않습니다. 이는 WCAG 2.1 기준 위반입니다.
다음과 같이 키보드 이벤트 핸들러를 추가하세요:
function ImageGalleryContent({ className }: { className?: string }) { - const { images, current, total, firstImage } = useImageGallery(); + const { images, current, total, firstImage, goPrev, goNext } = useImageGallery(); const loading = firstImage ? 'eager' : 'lazy'; + + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'ArrowLeft') { + e.preventDefault(); + goPrev(); + } else if (e.key === 'ArrowRight') { + e.preventDefault(); + goNext(); + } + }; return ( - <Flex dir="col" alignItems="center" className={cn('w-full max-w-[500px]', className)}> + <Flex + dir="col" + alignItems="center" + className={cn('w-full max-w-[500px]', className)} + tabIndex={0} + onKeyDown={handleKeyDown} + role="region" + aria-label="Image gallery" + >
69-76: 접근성: aria-current 속성 추가 필요현재 활성화된 이미지를 나타내는 dot 버튼에
aria-current속성이 없어, 스크린 리더 사용자가 현재 위치를 알 수 없습니다.다음과 같이 수정하세요:
<button key={index} type="button" aria-label={`Image ${index + 1}`} + aria-current={isActive ? 'true' : undefined} onClick={() => goToIndex(index)}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/shared/ui/ImageGallery/ImageGallery.stories.tsx(1 hunks)src/shared/ui/ImageGallery/ImageGallery.tsx(1 hunks)src/shared/ui/ImageGallery/ImageGalleryContext.tsx(1 hunks)src/shared/ui/MediaUpload/MediaUpload.tsx(1 hunks)src/shared/ui/MediaUpload/MediaUploadPreview.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
MediaPreview(11-31)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
Props(9-48)
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
src/shared/ui/ImageGallery/ImageGalleryContext.tsx (2)
ImageGalleryProvider(37-60)useImageGallery(28-30)
src/shared/ui/ImageGallery/ImageGallery.stories.tsx (1)
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
ImageGallery(21-27)
src/shared/ui/ImageGallery/ImageGalleryContext.tsx (1)
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
ImageGalleryItem(8-8)
🪛 Biome (2.1.2)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx
[error] 45-49: Provide a track for captions when using audio or video elements.
Captions support users with hearing-impairments. They should be a transcription or translation of the dialogue, sound effects, musical cues, and other relevant audio information.
(lint/a11y/useMediaCaption)
⏰ 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: preview-storybook
🔇 Additional comments (4)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
48-48: 미리보기 크기 개선이 적용되었습니다.이전 리뷰에서 제안된 대로 고정된
h-[200px]대신max-h-[500px]와object-contain을 사용하여 원본 비율을 유지하면서 최대 크기를 제한하는 방식으로 개선되었습니다. 좋은 변경사항입니다.Also applies to: 54-54
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
71-71: 메모리 누수 방지가 부분적으로 개선되었습니다.
handleReset과handleRemoveFile에서URL.revokeObjectURL()을 호출하여 이전 리뷰에서 지적된 메모리 누수 문제가 해결되었습니다. 좋은 개선입니다.Also applies to: 95-95
src/shared/ui/ImageGallery/ImageGallery.tsx (1)
82-105: 화살표 버튼 구현 잘 되었습니다
aria-label이 올바르게 방향을 나타내고 있으며, 버튼의 표시/숨김 로직도 적절합니다.src/shared/ui/ImageGallery/ImageGallery.stories.tsx (1)
1-49: 스토리북 구현이 잘 되었습니다Storybook 설정과 샘플 데이터가 적절하게 구성되어 있으며, 컴포넌트를 효과적으로 시연하고 있습니다. 샘플 이미지들이
name속성을 포함하고 있어 접근성 테스트에도 유용합니다.
| <img | ||
| src={images[current].url} | ||
| loading={loading} | ||
| alt={images[current].name} | ||
| width={500} | ||
| height={500} | ||
| className="h-full w-full object-contain" | ||
| /> |
There was a problem hiding this comment.
alt 속성에 대한 폴백 처리 필요
Line 43에서 alt={images[current].name}을 사용하고 있는데, name이 optional이므로 undefined가 될 수 있습니다. 이 경우 이미지에 대한 설명이 없어 접근성이 저하됩니다.
다음과 같이 폴백을 추가하세요:
- alt={images[current].name}
+ alt={images[current].name || `Image ${current + 1}`}📝 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.
| <img | |
| src={images[current].url} | |
| loading={loading} | |
| alt={images[current].name} | |
| width={500} | |
| height={500} | |
| className="h-full w-full object-contain" | |
| /> | |
| <img | |
| src={images[current].url} | |
| loading={loading} | |
| alt={images[current].name || `Image ${current + 1}`} | |
| width={500} | |
| height={500} | |
| className="h-full w-full object-contain" | |
| /> |
🤖 Prompt for AI Agents
In src/shared/ui/ImageGallery/ImageGallery.tsx around lines 40 to 47, the img
alt prop uses images[current].name which can be undefined; update the alt to use
a safe fallback such as images[current]?.name ?? images[current]?.description ??
'' (or another meaningful fallback string) to ensure a defined alt value for
accessibility, using optional chaining to avoid runtime errors.
| const goPrev = useCallback(() => setCurrent((c) => Math.max(0, c - 1)), []); | ||
| const goNext = useCallback(() => setCurrent((c) => Math.min(total - 1, c + 1)), [total]); | ||
| const goToIndex = useCallback( | ||
| (index: number) => setCurrent(Math.max(0, Math.min(total - 1, index))), | ||
| [total] | ||
| ); |
There was a problem hiding this comment.
빈 배열일 때 내비게이션 함수에서 잘못된 인덱스 생성
images 배열이 비어있을 때(total === 0), goNext와 goToIndex가 음수 인덱스(-1)를 생성할 수 있습니다:
goNext:Math.min(total - 1, c + 1)→Math.min(-1, 1)→-1goToIndex:Math.min(total - 1, index)→Math.min(-1, index)→ 양수 인덱스에 대해-1
이는 current가 항상 [0, total-1] 범위에 있다는 가정을 깨뜨립니다.
다음과 같이 수정하세요:
- const goNext = useCallback(() => setCurrent((c) => Math.min(total - 1, c + 1)), [total]);
+ const goNext = useCallback(() => setCurrent((c) => (total > 0 ? Math.min(total - 1, c + 1) : 0)), [total]);
const goToIndex = useCallback(
- (index: number) => setCurrent(Math.max(0, Math.min(total - 1, index))),
+ (index: number) => setCurrent(total > 0 ? Math.max(0, Math.min(total - 1, index)) : 0),
[total]
);🤖 Prompt for AI Agents
In src/shared/ui/ImageGallery/ImageGalleryContext.tsx around lines 41 to 46, the
navigation helpers can set current to -1 when total === 0; update goPrev,
goNext, and goToIndex to guard against empty arrays by clamping against 0 and
total - 1 only when total > 0. Specifically, compute a safeMax = Math.max(0,
total - 1) (or early-return when total === 0) and then setCurrent using
Math.max(0, Math.min(safeMax, ...)) so current never becomes negative; keep the
same useCallback dependencies (include total where already present).
keemsebin
left a comment
keemsebin
left a comment
There was a problem hiding this comment.
수정 꼼꼼하게 해주셔서 감삼다~!
아무래도 띵동에 계속 계셔야겠서요.
2 participants
🔥 연관 이슈
🚀 작업 내용
2025-10-09.12.55.57.mov
2025-10-09.12.58.57.mov
🤔 고민했던 내용
Summary by CodeRabbit
신기능
문서