Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/shared/ui/ImageGallery/ImageGallery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function ImageGalleryContent({ className }: { className?: string }) {
</>
)}
</Flex>
<Flex justifyContent="center" className="mt-2">
<Flex justifyContent="center" className="mt-3">
<ImageGalleryDots />
</Flex>
</Flex>
Expand All @@ -62,7 +62,7 @@ function ImageGalleryContent({ className }: { className?: string }) {
function ImageGalleryDots() {
const { images, current, goToIndex } = useImageGallery();
return (
<Flex alignItems="center" className="gap-2">
<Flex alignItems="center" className="gap-2 md:gap-2.5">
{images.map((_, index) => {
const isActive = index === current;
return (
Expand All @@ -71,7 +71,10 @@ function ImageGalleryDots() {
type="button"
aria-label={`Image ${index + 1}`}
onClick={() => goToIndex(index)}
className={cn('h-2 w-2 rounded-full', isActive ? 'bg-primary-300' : 'bg-gray-200')}
className={cn(
'h-2 w-2 cursor-pointer rounded-full md:h-2.5 md:w-2.5',
isActive ? 'bg-primary-300' : 'bg-gray-200'
)}
/>
);
})}
Expand All @@ -93,7 +96,7 @@ function ImageGalleryArrow({ direction }: ImageGalleryArrowProps) {
type="button"
onClick={onClick}
className={cn(
'absolute top-1/2 z-10 -translate-y-1/2 rounded-full bg-white/75 p-1 shadow-sm transition-opacity hover:bg-white md:p-1.5',
'absolute top-1/2 z-10 -translate-y-1/2 cursor-pointer rounded-full bg-white/75 p-1 shadow-sm transition-opacity hover:bg-white md:p-1.5',
isPrev ? 'left-4' : 'right-4',
isHidden && 'hidden'
)}
Expand Down
41 changes: 33 additions & 8 deletions src/shared/ui/MediaUpload/MediaUpload.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useState } from 'react';
import type { Meta, StoryObj } from '@storybook/react';

import { MediaUpload, Props } from './MediaUpload';
Expand All @@ -11,6 +12,26 @@ const meta = {
export default meta;
type Story = StoryObj<Props>;

function MediaUploadStoryWrapper(args: Props & { serverResponseUrl?: string[] }) {
const { serverResponseUrl } = 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
{...args}
previewUrls={previewUrls}
previewFiles={previewFiles}
onFileChange={handleFileChange}
/>
);
}
Comment on lines +15 to +32
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.


export const Basic: Story = {
argTypes: {
label: { control: { type: 'text' } },
Expand All @@ -19,7 +40,7 @@ export const Basic: Story = {
maxSize: { control: { type: 'number', min: 1, step: 1 } },
acceptedFormats: { control: 'text' },
},
render: (args) => <MediaUpload {...args} />,
render: (args) => <MediaUploadStoryWrapper {...args} />,
};

export const UploadFormats: Story = {
Expand All @@ -29,26 +50,30 @@ export const UploadFormats: Story = {
maxSize: 3,
acceptedFormats: ['image/*', 'video/*'],
},
render: (args) => <MediaUploadStoryWrapper {...args} />,
};

export const WithTopAffix: Story = {
args: {
topAffix: '제목처럼 추가할 수 있어요',
},
render: (args) => <MediaUploadStoryWrapper {...args} />,
};

export const MultipleMode: Story = {
args: {
multiple: true,
},
render: (args) => <MediaUploadStoryWrapper {...args} />,
};

export const EditMode: Story = {
args: {
multiple: true,
initialPreviewUrls: [
'https://images.unsplash.com/photo-1500530855697-b586d89ba3ee?q=80&w=1200&auto=format&fit=crop',
],
initialFiles: [new File([], 'test.jpg')],
},
render: (args) => (
<MediaUploadStoryWrapper
{...args}
serverResponseUrl={[
'https://images.unsplash.com/photo-1500530855697-b586d89ba3ee?q=80&w=1200&auto=format&fit=crop',
]}
/>
),
};
50 changes: 22 additions & 28 deletions src/shared/ui/MediaUpload/MediaUpload.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ComponentProps, useId, useState } from 'react';
import { ComponentProps, useId } from 'react';

import { MediaPreview } from './MediaUploadPreview';

Expand Down Expand Up @@ -46,44 +46,43 @@ export type Props = {
*/
multiple?: boolean;
/**
* initial preview urls for edit mode (e.g., existing uploaded image urls)
* Array of preview URLs to display existing media
*/
initialPreviewUrls?: string[];
previewUrls?: string[];
/**
* initial files for edit mode (if available).
* Array of preview File objects for new uploads
*/
initialFiles?: File[];
previewFiles?: File[] | null;
/**
* Callback function called when files are selected, removed, or reset
*/
onFileChange?: (files: File[] | null, previewUrls: string[]) => void;
} & Omit<ComponentProps<'input'>, 'id'>;

const GB = 1024 * 1024 * 1024;

export function MediaUpload({
topAffix,
onFileUpload,
id,
label = '파일을 업로드해주세요. (jpg, jpeg, png)',
description = '* 파일은 5GB까지 업로드 가능합니다.',
maxSize = 5,
acceptedFormats = ['image/*'],
multiple = false,
initialPreviewUrls,
initialFiles,
previewFiles = [],
previewUrls = [],
onFileChange,
...props
}: Props) {
const generatedId = useId();
const inputId = id || generatedId;

const [selectedFiles, setSelectedFiles] = useState<File[]>(initialFiles || []);
const [previewUrls, setPreviewUrls] = useState<string[]>(initialPreviewUrls || []);
const isSelected = selectedFiles.length > 0;
const isSelected = previewUrls.length > 0;

const handleReset = () => {
previewUrls.forEach((url) => {
if (url.startsWith('blob:')) URL.revokeObjectURL(url);
});
setSelectedFiles([]);
setPreviewUrls([]);
onFileUpload?.(null);
onFileChange?.(null, []);
};

const handleFileChange = (event: React.ChangeEvent<HTMLInputElement>) => {
Expand All @@ -97,27 +96,22 @@ export function MediaUpload({
}

const validatedFiles = multiple ? files : [files[0]];
setSelectedFiles(validatedFiles);
const urls = validatedFiles.map((file) => URL.createObjectURL(file));
setPreviewUrls(urls);
onFileUpload?.(validatedFiles);
onFileChange?.(validatedFiles, urls);
};

const handleRemoveFile = (index: number) => {
const urlToRemove = previewUrls[index];
if (urlToRemove?.startsWith('blob:')) {
URL.revokeObjectURL(urlToRemove);
}
const newFiles = selectedFiles.filter((_, i) => i !== index);
if (urlToRemove?.startsWith('blob:')) URL.revokeObjectURL(urlToRemove);

const newFiles = previewFiles?.filter((_, i) => i !== index) ?? null;
const newUrls = previewUrls.filter((_, i) => i !== index);

setSelectedFiles(newFiles);
setPreviewUrls(newUrls);
onFileUpload?.(newFiles.length > 0 ? newFiles : null);
onFileChange?.(newFiles, newUrls);
};

return (
<div className="max-h-[500px] w-full">
<div className="max-h-[500px] w-full overflow-auto">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
<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.

<Flex justifyContent="between">
<Body1 className="text-gray-400">{topAffix}</Body1>
<RefreshButton handleReset={handleReset} isSelected={isSelected} />
Expand All @@ -126,7 +120,7 @@ export function MediaUpload({
<UploadBox id={inputId} label={label} description={description} />
) : (
<MediaPreview
files={selectedFiles}
files={previewFiles}
previewUrls={previewUrls}
onRemoveFile={handleRemoveFile}
multiple={multiple}
Expand Down Expand Up @@ -178,7 +172,7 @@ function RefreshButton({ handleReset, isSelected }: RefreshButtonProp) {
<Flex
as="button"
onClick={(e) => {
e.stopPropagation();
e.preventDefault();
if (isSelected) handleReset();
}}
alignItems="center"
Expand Down
35 changes: 28 additions & 7 deletions src/shared/ui/MediaUpload/MediaUploadPreview.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
import { useState, useEffect } from 'react';

import { Flex } from '../Flex';
import { Icon } from '../Icon';

type Props = {
files: File[];
files: File[] | null;
previewUrls: string[];
onRemoveFile: (index: number) => void;
multiple: boolean;
};

export function MediaPreview({ files, previewUrls, onRemoveFile, multiple }: Props) {
if (!multiple) {
return <MediaPreviewItem file={files[0]} previewUrl={previewUrls[0]} />;
return <MediaPreviewItem file={files?.[0]} previewUrl={previewUrls[0]} />;
}
return (
<div className="grid grid-cols-2 gap-2 md:grid-cols-3 md:gap-4">
{files?.map((file, index) => (
{previewUrls?.map((previewUrl, index) => (
<div key={index} className="relative aspect-square">
<MediaPreviewItem file={file} previewUrl={previewUrls[index]} />
<MediaPreviewItem file={files?.[index]} previewUrl={previewUrl} />
<button
type="button"
onClick={() => onRemoveFile(index)}
Expand All @@ -31,17 +33,27 @@ export function MediaPreview({ files, previewUrls, onRemoveFile, multiple }: Pro
}

type MediaPreviewItemProps = {
file: File;
file?: File;
previewUrl: string;
};
function MediaPreviewItem({ file, previewUrl }: MediaPreviewItemProps) {
const [isVideo, setIsVideo] = useState<boolean>(false);

useEffect(() => {
if (file) return setIsVideo(file.type.startsWith('video/'));
getMimeType(previewUrl).then((type) => {
if (type) setIsVideo(type.startsWith('video/'));
else setIsVideo(false);
});
}, [file, previewUrl]);

Comment on lines +42 to +49
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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

return (
<Flex
justifyContent="center"
alignItems="center"
className="relative h-full w-full rounded-xl border border-gray-200 bg-gray-50"
>
{file.type.startsWith('video/') ? (
{isVideo ? (
<video
src={previewUrl}
controls
Expand All @@ -50,10 +62,19 @@ function MediaPreviewItem({ file, previewUrl }: MediaPreviewItemProps) {
) : (
<img
src={previewUrl}
alt={`미리보기 ${file.name}`}
alt={file ? `미리보기 ${file.name}` : '미리보기 이미지'}
className="h-full max-h-[500px] w-full max-w-[500px] object-contain"
/>
)}
</Flex>
);
}

async function getMimeType(url: string): Promise<string | null> {
try {
const res = await fetch(url, { method: 'HEAD' });
return res.headers.get('Content-Type');
} catch {
return null;
}
}