[REFACTOR] MediaUpload, ImageGallery 컴포넌트 사용성개선#82
Conversation
WalkthroughImageGallery의 프레젠테이션 조정, MediaUpload의 내부 상태 제거 후 prop·콜백 기반 제어로 API 변경, MediaUploadPreview에 미디어 타입 감지(파일 타입 또는 HEAD 요청) 추가 및 스토리에 외부 제어용 래퍼 도입이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StoryWrapper as MediaUploadStoryWrapper
participant MediaUpload
participant Parent as App/Store
Note over User,MediaUpload: 스토리 환경의 업로드 흐름
User->>MediaUpload: 파일 선택
MediaUpload->>StoryWrapper: onFileChange(files, previewUrls)
StoryWrapper->>StoryWrapper: previewFiles/previewUrls 갱신
StoryWrapper->>Parent: (선택적) 서버/상태 동기화
StoryWrapper->>MediaUpload: 최신 previewFiles/previewUrls 전달 (props)
sequenceDiagram
participant MediaPreviewItem
participant CDN as URLHost
Note over MediaPreviewItem,CDN: previewUrl의 미디어 타입 판정
alt file 제공됨
MediaPreviewItem->>MediaPreviewItem: file.type.startsWith('video/') 검사
else file 없음
MediaPreviewItem->>CDN: HEAD request (previewUrl)
CDN-->>MediaPreviewItem: Content-Type 헤더 응답
MediaPreviewItem->>MediaPreviewItem: isVideo 판정
end
MediaPreviewItem->>MediaPreviewItem: video 또는 img 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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월 18일 22시 48분 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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/shared/ui/ImageGallery/ImageGallery.tsx(4 hunks)src/shared/ui/MediaUpload/MediaUpload.tsx(1 hunks)
🔇 Additional comments (3)
src/shared/ui/ImageGallery/ImageGallery.tsx (3)
55-55: LGTM!여백 증가로 시각적 개선이 이루어졌습니다.
65-65: LGTM!반응형 간격 조정과 dot 크기 개선,
cursor-pointer추가로 사용성이 향상되었습니다.cn()함수를 사용한 조건부 스타일링도 적절합니다.Also applies to: 74-77
99-99: LGTM!arrow 버튼에
cursor-pointer를 명시적으로 추가하여 일관성이 개선되었습니다.
|
|
||
| return ( | ||
| <div className="max-h-[500px] w-full"> | ||
| <div className="max-h-[500px] w-full overflow-auto"> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
세로 스크롤만 필요하다면 overflow-y-auto 사용을 권장합니다.
overflow-auto는 가로/세로 양방향 스크롤을 활성화합니다. 의도한 것이 세로 스크롤만이라면 overflow-y-auto가 더 정확한 선택입니다.
다음 diff를 적용하여 세로 스크롤만 활성화하세요:
- <div className="max-h-[500px] w-full overflow-auto">
+ <div className="max-h-[500px] w-full overflow-y-auto">📝 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.
| <div className="max-h-[500px] w-full overflow-auto"> | |
| <div className="max-h-[500px] w-full overflow-y-auto"> |
🤖 Prompt for AI Agents
In src/shared/ui/MediaUpload/MediaUpload.tsx around line 120, the div currently
uses overflow-auto which enables both horizontal and vertical scrolling; replace
it with overflow-y-auto to allow only vertical scrolling (keep the existing
max-h-[500px] and w-full intact) so horizontal scroll is prevented while
preserving vertical overflow behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/shared/ui/MediaUpload/MediaUpload.stories.tsx (1)
34-41: Story args control 타입 정합성 제안acceptedFormats는 string[]입니다. Storybook control을 'text' 대신 'object'(또는 'array')로 두면 실사용 타입과 일치합니다. 기능엔 지장 없으나 오입력 방지에 도움 됩니다.
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (2)
57-61: 미디어 프리뷰 최적화(경량 메타데이터만 로드)프리뷰 용도라면 video에 preload="metadata"를 지정해 초기 로드를 줄일 수 있습니다.
- <video + <video src={previewUrl} controls + preload="metadata" className="h-full max-h-[500px] w-full max-w-[500px] object-contain" />
19-31: 멀티 프리뷰 삭제 버튼 접근성 OKtype="button", 시맨틱 버튼, 닫기 아이콘 제공 모두 적절합니다. aria-label 추가를 고려하면 더 좋아집니다(예: "미리보기 삭제").
src/shared/ui/MediaUpload/MediaUpload.tsx (3)
88-101: 이벤트 핸들러에서 예외 throw 금지 — UX 중단 및 오류 경계 미적용React 이벤트 핸들러에서 throw는 사용자 흐름을 끊고 에러 바운더리로도 포착되지 않습니다. 파일 입력을 초기화하고 조용히 무시하는 쪽이 안전합니다.
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 이하의 파일로 등록해주세요.`); + // 용량 초과: 입력 초기화 후 종료 + event.currentTarget.value = ''; + return; }추가로 검증 실패콜백(onError 등)을 도입하면 소비자가 메시지를 표시할 수 있습니다.
170-177: submit 방지 목적이면 type="button"으로 명시button 기본 type은 submit입니다. preventDefault 대신 type="button"을 지정해 폼 제출 부작용을 근본적으로 차단하세요.
return ( <Flex as="button" - onClick={(e) => { - e.preventDefault(); - if (isSelected) handleReset(); - }} + type="button" + onClick={() => { + if (isSelected) handleReset(); + }} alignItems="center"
29-33: 공개 API에서 사용되지 않는 onFileUpload 제거전체 코드베이스를 검색한 결과,
onFileUpload는 정의되기만 할 뿐 어떤 파일에서도 사용되지 않습니다. 컴포넌트 구현에서 이 prop을 destructure하지 않으며, 대신onFileChange콜백을 사용하고 있습니다. 사용되지 않는 공개 API를 유지하면 혼란을 초래하므로 Props 타입에서 제거해야 합니다.제거 대상: src/shared/ui/MediaUpload/MediaUpload.tsx, 29-33줄 (onFileUpload 속성 정의)
♻️ Duplicate comments (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
114-114: 세로 스크롤만 필요 시 overflow-y-auto 권장이전 리뷰와 동일 제안입니다. 가로 스크롤을 방지하려면 overflow-y-auto가 더 정확합니다.
- <div className="max-h-[500px] w-full overflow-auto"> + <div className="max-h-[500px] w-full overflow-y-auto">
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/shared/ui/MediaUpload/MediaUpload.stories.tsx(4 hunks)src/shared/ui/MediaUpload/MediaUpload.tsx(5 hunks)src/shared/ui/MediaUpload/MediaUploadPreview.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/shared/ui/MediaUpload/MediaUploadPreview.tsx (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
Props(9-60)
src/shared/ui/MediaUpload/MediaUpload.stories.tsx (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (2)
Props(9-60)MediaUpload(64-141)
🪛 GitHub Actions: Check Pull Request
src/shared/ui/MediaUpload/MediaUpload.stories.tsx
[warning] 16-16: Step 'npm run lint' reported 1 ESLint warning: 'Must use destructuring assignment' in MediaUpload.stories.tsx:16:60. ESLint found too many warnings (maximum: 0). Process completed with exit code 1.
🔇 Additional comments (1)
src/shared/ui/MediaUpload/MediaUpload.tsx (1)
122-128: 프리뷰 데이터 흐름 전환(LGTM)previewFiles/previewUrls + onFileChange로의 제어형 전환 좋습니다. 상위에서 상태 일원화되어 스토리/소비자 제어가 쉬워졌습니다.
| function MediaUploadStoryWrapper(args: Props & { serverResponseUrl?: string[] }) { | ||
| const [previewUrls, setPreviewUrls] = useState<string[]>(args.serverResponseUrl ?? []); | ||
| const [previewFiles, setPreviewFiles] = useState<File[]>([]); | ||
|
|
||
| const handleFileChange = (files: File[] | null, urls: string[]) => { | ||
| setPreviewFiles(files || []); | ||
| setPreviewUrls(urls); | ||
| }; | ||
|
|
||
| return ( | ||
| <MediaUpload | ||
| {...args} | ||
| previewUrls={previewUrls} | ||
| previewFiles={previewFiles} | ||
| onFileChange={handleFileChange} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
ESLint 경고로 파이프라인 실패 — 구조 분해 할당으로 수정하고 불필요한 prop 전파 차단
- args.serverResponseUrl 직접 접근으로 prefer-destructuring 경고 발생(최대 경고 0 설정으로 CI 실패).
- 또한 {...args}로 MediaUpload에 전달 시 정의되지 않은 serverResponseUrl prop이 하위로 전파될 수 있습니다.
아래처럼 구조 분해 + 나머지 전달로 해결하세요.
-function MediaUploadStoryWrapper(args: Props & { serverResponseUrl?: string[] }) {
- const [previewUrls, setPreviewUrls] = useState<string[]>(args.serverResponseUrl ?? []);
+function MediaUploadStoryWrapper(args: Props & { serverResponseUrl?: string[] }) {
+ const { serverResponseUrl = [], ...rest } = args;
+ const [previewUrls, setPreviewUrls] = useState<string[]>(serverResponseUrl);
const [previewFiles, setPreviewFiles] = useState<File[]>([]);
@@
- return (
- <MediaUpload
- {...args}
+ return (
+ <MediaUpload
+ {...rest}
previewUrls={previewUrls}
previewFiles={previewFiles}
onFileChange={handleFileChange}
/>
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function MediaUploadStoryWrapper(args: Props & { serverResponseUrl?: string[] }) { | |
| const [previewUrls, setPreviewUrls] = useState<string[]>(args.serverResponseUrl ?? []); | |
| const [previewFiles, setPreviewFiles] = useState<File[]>([]); | |
| const handleFileChange = (files: File[] | null, urls: string[]) => { | |
| setPreviewFiles(files || []); | |
| setPreviewUrls(urls); | |
| }; | |
| return ( | |
| <MediaUpload | |
| {...args} | |
| previewUrls={previewUrls} | |
| previewFiles={previewFiles} | |
| onFileChange={handleFileChange} | |
| /> | |
| ); | |
| } | |
| function MediaUploadStoryWrapper(args: Props & { serverResponseUrl?: string[] }) { | |
| const { serverResponseUrl = [], ...rest } = args; | |
| const [previewUrls, setPreviewUrls] = useState<string[]>(serverResponseUrl); | |
| const [previewFiles, setPreviewFiles] = useState<File[]>([]); | |
| const handleFileChange = (files: File[] | null, urls: string[]) => { | |
| setPreviewFiles(files || []); | |
| setPreviewUrls(urls); | |
| }; | |
| return ( | |
| <MediaUpload | |
| {...rest} | |
| previewUrls={previewUrls} | |
| previewFiles={previewFiles} | |
| onFileChange={handleFileChange} | |
| /> | |
| ); | |
| } |
🧰 Tools
🪛 GitHub Actions: Check Pull Request
[warning] 16-16: Step 'npm run lint' reported 1 ESLint warning: 'Must use destructuring assignment' in MediaUpload.stories.tsx:16:60. ESLint found too many warnings (maximum: 0). Process completed with exit code 1.
🤖 Prompt for AI Agents
In src/shared/ui/MediaUpload/MediaUpload.stories.tsx around lines 15 to 32, the
code reads args.serverResponseUrl directly and spreads {...args} into
MediaUpload which triggers an ESLint prefer-destructuring warning and may pass
an undefined serverResponseUrl prop down; fix by destructuring serverResponseUrl
from args (e.g. const { serverResponseUrl, ...rest } = args), initialize
previewUrls from serverResponseUrl, and spread only the remaining props into
<MediaUpload {...rest} ...> so the undefined prop is not forwarded.
| useEffect(() => { | ||
| if (file) return setIsVideo(file.type.startsWith('video/')); | ||
| getMimeType(previewUrl).then((type) => { | ||
| if (type) setIsVideo(type.startsWith('video/')); | ||
| else setIsVideo(false); | ||
| }); | ||
| }, [file, previewUrl]); | ||
|
|
There was a problem hiding this comment.
HEAD 요청에 중단/정합성 처리 없음 — 언마운트/빠른 전환 시 상태 경쟁 조건과 불필요 네트워크 발생
- fetch(HEAD) 결과가 늦게 도착하면 언마운트 후 setState가 실행될 수 있습니다.
- res.ok 체크가 없어 4xx/5xx도 신뢰할 수 없는 Content-Type을 반환할 수 있습니다.
AbortController로 취소를 지원하고 res.ok를 확인하세요.
- useEffect(() => {
- if (file) return setIsVideo(file.type.startsWith('video/'));
- getMimeType(previewUrl).then((type) => {
- if (type) setIsVideo(type.startsWith('video/'));
- else setIsVideo(false);
- });
- }, [file, previewUrl]);
+ useEffect(() => {
+ if (file) {
+ setIsVideo(file.type.startsWith('video/'));
+ return;
+ }
+ const ac = new AbortController();
+ let cancelled = false;
+ getMimeType(previewUrl, ac.signal).then((type) => {
+ if (!cancelled) setIsVideo(Boolean(type?.startsWith('video/')));
+ });
+ return () => {
+ cancelled = true;
+ ac.abort();
+ };
+ }, [file, previewUrl]);
@@
-async function getMimeType(url: string): Promise<string | null> {
+async function getMimeType(url: string, signal?: AbortSignal): Promise<string | null> {
try {
- const res = await fetch(url, { method: 'HEAD' });
- return res.headers.get('Content-Type');
+ const res = await fetch(url, { method: 'HEAD', signal });
+ if (!res.ok) return null;
+ return res.headers.get('Content-Type');
} catch {
return null;
}
}추가로, 외부 CDN CORS 정책으로 HEAD가 막힐 수 있어 캐시(Map<url, mime>)를 두어 재시도/중복요청을 줄이는 것도 권장합니다.
Also applies to: 73-80
2 participants
🔥 연관 이슈
🚀 작업 내용
Summary by CodeRabbit
Style
Refactor
Docs/Stories