-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feat] ImageManager TypeA 컴포넌트 추가 #94
base: develop
Are you sure you want to change the base?
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/web/src/components/common/ImageManager/TypeA/ImageUploader.tsxOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@repo/eslint-config' imported from /eslint.config.mjs apps/web/src/components/common/ImageManager/TypeA/TypeA.tsxOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@repo/eslint-config' imported from /eslint.config.mjs apps/web/src/components/common/ImageManager/TypeA/UploadedImages.css.tsOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@repo/eslint-config' imported from /eslint.config.mjs
개요Walkthrough이 풀 리퀘스트는 새로운 Changes
연결된 이슈에 대한 평가
가능한 관련 PR
제안된 레이블
제안된 리뷰어
토끼의 시 🐰
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (7)
apps/web/src/components/common/ImageManager/TypeB/TypeB.tsx (1)
3-5
: 컴포넌트 타입 정의가 필요합니다.향후 구현을 위해 Props 타입을 미리 정의하는 것이 좋습니다.
-export function TypeB() { +type TypeBProps = { + // TODO: Props 정의 추가 +}; + +export function TypeB({}: TypeBProps) { return <div>TypeB</div>; }apps/web/src/components/common/ImageManager/TypeA/ImageUploader.css.ts (1)
12-12
: 트랜지션 시간을 테마 변수로 관리하는 것이 좋습니다.일관성 있는 애니메이션을 위해 하드코딩된 트랜지션 시간을 테마 변수로 관리하는 것을 추천드립니다.
- transition: 'all 0.2s ease', + transition: `all ${vars.transitions.default} ease`,apps/web/src/components/common/ImageManager/ImageManager.tsx (2)
21-24
: Object.assign 대신 더 현대적인 문법을 사용하면 좋겠습니다.객체 리터럴을 직접 사용하는 것이 더 읽기 쉽고 현대적입니다.
-export const ImageManager: ImageManagerComposition = Object.assign({ - TypeA, - TypeB, -}); +export const ImageManager: ImageManagerComposition = { + TypeA, + TypeB, +};
11-20
: 문서에 Props 타입에 대한 설명을 추가하면 좋겠습니다.예제 코드는 잘 작성되어 있지만, Props의 각 속성에 대한 설명이 있으면 더 좋을 것 같습니다.
/** - * + * @param maxFileSize - 파일당 최대 크기 (MB 단위) + * @param maxFiles - 업로드 가능한 최대 파일 수 * @example * ```tsx * // Prompt 등에서 사용하게 되는 TypeAapps/web/src/components/common/ImageManager/TypeA/context.tsx (1)
14-20
: 커스텀 훅 구현에 대한 제안컨텍스트 사용 범위 제한이 잘 되어있지만, 에러 메시지에 더 자세한 사용 예시를 추가하면 좋을 것 같습니다.
에러 메시지를 다음과 같이 수정하는 것을 제안드립니다:
- throw new Error('ImageManager 컴포넌트 내부에서만 사용할 수 있습니다.'); + throw new Error( + 'ImageManager 컴포넌트 내부에서만 사용할 수 있습니다. ' + + '사용 예시: <ImageManager><YourComponent /></ImageManager>' + );apps/web/src/components/common/ImageManager/TypeA/UploadedImages.tsx (1)
3-3
: Import 경로 최적화 필요현재 Icon 컴포넌트의 import 경로가 너무 깊습니다. 경로 별칭(path alias)을 사용하여 더 간단하게 만들 수 있습니다.
-import { Icon } from '../../../../../../../packages/ui/src/components/Icon'; +import { Icon } from '@repo/ui/Icon';apps/web/src/app/page.tsx (1)
490-490
: 불필요한 props 제거 권장maxFileSize={10}와 maxFiles={5}는 컴포넌트의 기본값과 동일합니다. 명시적으로 지정할 필요가 없습니다.
- <ImageManager.TypeA maxFileSize={10} maxFiles={5} /> + <ImageManager.TypeA />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/web/src/app/layout.tsx
(1 hunks)apps/web/src/app/page.tsx
(2 hunks)apps/web/src/components/common/ImageManager/ImageManager.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/ImageUploader.css.ts
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/ImageUploader.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/TypeA.css.ts
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/TypeA.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/UploadedImages.css.ts
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/UploadedImages.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/context.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/types.ts
(1 hunks)apps/web/src/components/common/ImageManager/TypeB/TypeB.tsx
(1 hunks)apps/web/src/components/common/ImageManager/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/app/layout.tsx
- apps/web/src/components/common/ImageManager/index.ts
🔇 Additional comments (5)
apps/web/src/components/common/ImageManager/TypeA/types.ts (1)
1-5
: 타입 정의가 명확하고 잘 구조화되어 있습니다!이미지 파일 관리에 필요한 모든 속성이 포함되어 있습니다.
apps/web/src/components/common/ImageManager/TypeA/TypeA.css.ts (2)
5-20
: 스타일 레시피가 잘 구현되었습니다.중앙 정렬 여부에 따른 변형을 처리하는 방식이 깔끔하며, 테마 변수를 적절히 활용하고 있습니다.
22-25
: 이미지 컨텐츠 스타일이 간단명료합니다.flex 레이아웃을 사용한 정렬 방식이 적절합니다.
apps/web/src/components/common/ImageManager/TypeA/context.tsx (1)
4-8
: 컨텍스트 타입 정의가 명확합니다.이미지 관리에 필요한 핵심 기능들이 잘 정의되어 있습니다.
apps/web/src/components/common/ImageManager/TypeA/TypeA.tsx (1)
75-92
: 잘 구현된 메모리 정리 로직컴포넌트 언마운트 시와 이미지 제거 시의 메모리 정리가 잘 구현되어 있습니다.
apps/web/src/components/common/ImageManager/TypeA/UploadedImages.css.ts
Outdated
Show resolved
Hide resolved
return ( | ||
<label | ||
className={styles.uploader} | ||
onDragOver={(e) => e.preventDefault()} | ||
onDrop={handleDrop} | ||
> | ||
{children} | ||
<input | ||
type="file" | ||
accept="image/*" | ||
multiple | ||
onChange={handleChange} | ||
className={styles.input} | ||
/> | ||
</label> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
접근성 및 사용자 경험 개선이 필요합니다.
드래그 앤 드롭 영역에 대한 접근성 속성과 시각적 피드백이 부족합니다.
다음과 같은 개선을 제안드립니다:
<label
className={styles.uploader}
onDragOver={(e) => e.preventDefault()}
onDrop={handleDrop}
+ role="button"
+ tabIndex={0}
+ aria-label="이미지 업로드"
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ e.currentTarget.click();
+ }
+ }}
>
{children}
<input
type="file"
accept="image/*"
multiple
onChange={handleChange}
className={styles.input}
+ aria-hidden="true"
/>
</label>
📝 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.
return ( | |
<label | |
className={styles.uploader} | |
onDragOver={(e) => e.preventDefault()} | |
onDrop={handleDrop} | |
> | |
{children} | |
<input | |
type="file" | |
accept="image/*" | |
multiple | |
onChange={handleChange} | |
className={styles.input} | |
/> | |
</label> | |
); | |
return ( | |
<label | |
className={styles.uploader} | |
onDragOver={(e) => e.preventDefault()} | |
onDrop={handleDrop} | |
role="button" | |
tabIndex={0} | |
aria-label="이미지 업로드" | |
onKeyDown={(e) => { | |
if (e.key === 'Enter' || e.key === ' ') { | |
e.preventDefault(); | |
e.currentTarget.click(); | |
} | |
}} | |
> | |
{children} | |
<input | |
type="file" | |
accept="image/*" | |
multiple | |
onChange={handleChange} | |
className={styles.input} | |
aria-hidden="true" | |
/> | |
</label> | |
); |
const handleDrop = useCallback( | ||
(e: DragEvent) => { | ||
e.preventDefault(); | ||
onUpload(e.dataTransfer.files); | ||
}, | ||
[onUpload] | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파일 유효성 검사 로직이 필요합니다.
현재 구현에서 파일 크기 제한이나 이미지 타입 검증이 누락되어 있습니다.
다음과 같은 유효성 검사 로직 추가를 제안드립니다:
const handleDrop = useCallback(
(e: DragEvent) => {
e.preventDefault();
+ const files = Array.from(e.dataTransfer.files);
+ const isValidFiles = files.every(file => {
+ const isImage = file.type.startsWith('image/');
+ const isValidSize = file.size <= 5 * 1024 * 1024; // 5MB 제한
+ return isImage && isValidSize;
+ });
+
+ if (!isValidFiles) {
+ alert('유효하지 않은 파일이 포함되어 있습니다. 이미지 파일(최대 5MB)만 업로드 가능합니다.');
+ return;
+ }
onUpload(e.dataTransfer.files);
},
[onUpload]
);
📝 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 handleDrop = useCallback( | |
(e: DragEvent) => { | |
e.preventDefault(); | |
onUpload(e.dataTransfer.files); | |
}, | |
[onUpload] | |
); | |
const handleDrop = useCallback( | |
(e: DragEvent) => { | |
e.preventDefault(); | |
const files = Array.from(e.dataTransfer.files); | |
const isValidFiles = files.every(file => { | |
const isImage = file.type.startsWith('image/'); | |
const isValidSize = file.size <= 5 * 1024 * 1024; // 5MB 제한 | |
return isImage && isValidSize; | |
}); | |
if (!isValidFiles) { | |
alert('유효하지 않은 파일이 포함되어 있습니다. 이미지 파일(최대 5MB)만 업로드 가능합니다.'); | |
return; | |
} | |
onUpload(e.dataTransfer.files); | |
}, | |
[onUpload] | |
); |
<Image | ||
src={image.preview} | ||
alt="" | ||
width={IMAGE_SIZE} | ||
height={IMAGE_SIZE} | ||
className={styles.image} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
접근성 개선 필요
이미지의 alt 속성이 비어있습니다. 스크린 리더 사용자를 위해 의미 있는 대체 텍스트를 제공해야 합니다.
- alt=""
+ alt={`업로드된 이미지 ${image.id}`}
📝 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.
<Image | |
src={image.preview} | |
alt="" | |
width={IMAGE_SIZE} | |
height={IMAGE_SIZE} | |
className={styles.image} | |
/> | |
<Image | |
src={image.preview} | |
alt={`업로드된 이미지 ${image.id}`} | |
width={IMAGE_SIZE} | |
height={IMAGE_SIZE} | |
className={styles.image} | |
/> |
export type ImageManagerTypeAProps = { | ||
/** | ||
* 이미지 파일 크기 제한 (MB) | ||
* @default 10 | ||
*/ | ||
maxFileSize?: number; | ||
/** | ||
* 이미지 파일 최대 개수 | ||
* @default 5 | ||
*/ | ||
maxFiles?: number; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Props 유효성 검사 추가 필요
maxFileSize와 maxFiles props에 대한 유효성 검사가 없습니다. 음수나 0과 같은 잘못된 값이 전달될 수 있습니다.
export type ImageManagerTypeAProps = {
maxFileSize?: number;
maxFiles?: number;
};
export const TypeA = ({
maxFileSize = 10,
maxFiles = 5,
}: ImageManagerTypeAProps) => {
+ if (maxFileSize <= 0) throw new Error('maxFileSize must be greater than 0');
+ if (maxFiles <= 0) throw new Error('maxFiles must be greater than 0');
Committable suggestion skipped: line range outside the PR's diff.
const newFiles = Array.from(files).map((file) => ({ | ||
id: crypto.randomUUID(), | ||
file, | ||
preview: URL.createObjectURL(file), | ||
})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
메모리 관리 최적화 필요
파일 업로드 시 이전 미리보기 URL을 해제하지 않고 있습니다. 이는 메모리 누수로 이어질 수 있습니다.
const newFiles = Array.from(files).map((file) => ({
id: crypto.randomUUID(),
file,
preview: URL.createObjectURL(file),
}));
-setImages((prev) => [...prev, ...newFiles]);
+setImages((prev) => {
+ // 이전 미리보기 URL 해제
+ prev.forEach(image => URL.revokeObjectURL(image.preview));
+ return [...prev, ...newFiles];
+});
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (5)
apps/web/src/components/common/ImageManager/TypeA/TypeA.tsx (2)
13-24
: Props에 대한 추가적인 타입 검증이 필요합니다.현재 런타임에서만 유효성 검사가 이루어지고 있습니다. 컴파일 타임에서도 검증할 수 있도록 Props 타입을 개선하면 좋을 것 같습니다.
export type ImageManagerTypeAProps = { - maxFileSize?: number; - maxFiles?: number; + maxFileSize?: Exclude<number, 0 | negative>; + maxFiles?: Exclude<number, 0 | negative>; };
43-46
: 에러 메시지를 더 구체적으로 개선하면 좋을 것 같습니다.현재 에러 메시지가 다소 일반적입니다. 사용자가 어떤 파일이 문제인지 알 수 있도록 구체적인 정보를 제공하면 좋을 것 같습니다.
if (oversizedFiles.length > 0) { - toast.error(`파일 크기는 ${maxFileSize}MB 이하여야 해요.`, 3000); + toast.error( + `${oversizedFiles.map(f => f.name).join(', ')} 파일이 ${maxFileSize}MB를 초과했어요.`, + 3000 + ); return; } if (invalidFiles.length > 0) { - toast.error('이미지 파일만 업로드할 수 있어요.', 3000); + toast.error( + `${invalidFiles.map(f => f.name).join(', ')} 파일은 이미지 형식이 아니에요.`, + 3000 + ); return; }Also applies to: 62-65
apps/web/src/components/common/ImageManager/TypeA/ImageUploader.tsx (1)
9-12
: 상수를 공통 파일로 이동하는 것을 고려해주세요.
KEY
상수는 다른 컴포넌트에서도 재사용될 수 있으므로, 공통 상수 파일로 이동하는 것이 좋습니다.-const KEY = { - ENTER: 'Enter', - SPACE: ' ', -} as const;
@web/constants/keyboard.ts
파일을 생성하여 다음과 같이 이동하는 것을 제안드립니다:export const KEYBOARD_KEY = { ENTER: 'Enter', SPACE: ' ', } as const;apps/web/src/utils/validateFiles.ts (2)
1-1
: 상수값의 가독성 개선이 필요합니다.상수값을 더 명확하고 유지보수하기 쉽게 분리하는 것이 좋습니다.
다음과 같이 변경을 제안합니다:
-const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB +const MB = 1024 * 1024; +const MAX_FILE_SIZE = 5 * MB; // 5MB
10-14
: 에러 메시지에 허용되는 파일 형식 정보 추가가 필요합니다.사용자에게 더 명확한 가이드를 제공하기 위해 지원되는 이미지 형식을 에러 메시지에 포함하는 것이 좋습니다.
다음과 같이 변경을 제안합니다:
if (!isValidFiles) { throw new Error( - '유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' + '유효하지 않은 파일이 포함되어 있어요. JPG, PNG, GIF, WebP 형식의 이미지 파일(최대 5MB)만 업로드 가능해요.' ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/src/components/common/ImageManager/TypeA/ImageUploader.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/TypeA.tsx
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/UploadedImages.css.ts
(1 hunks)apps/web/src/components/common/ImageManager/TypeA/UploadedImages.tsx
(1 hunks)apps/web/src/utils/index.ts
(1 hunks)apps/web/src/utils/validateFiles.ts
(1 hunks)apps/web/tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/tsconfig.json
- apps/web/src/utils/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/common/ImageManager/TypeA/UploadedImages.tsx
- apps/web/src/components/common/ImageManager/TypeA/UploadedImages.css.ts
🔇 Additional comments (2)
apps/web/src/components/common/ImageManager/TypeA/TypeA.tsx (1)
91-98
: 메모리 관리가 잘 구현되어 있습니다!컴포넌트 언마운트 시 URL.revokeObjectURL을 통한 메모리 정리가 적절히 구현되어 있습니다.
apps/web/src/components/common/ImageManager/TypeA/ImageUploader.tsx (1)
61-86
: 접근성을 고려한 구현이 잘 되어있습니다! 👍키보드 접근성과 ARIA 속성이 적절히 구현되어 있습니다.
<ImageUploader> | ||
<div | ||
className={styles.textContent({ | ||
isCenter: !isImageUploaded, | ||
})} | ||
> | ||
<Icon name="plusPicture" size={24} color="grey500" /> | ||
<Text.Span color="grey600" fontSize={18} fontWeight="medium"> | ||
이곳에 이미지를 드래그하거나 클릭하여 업로드 | ||
</Text.Span> | ||
{!isImageUploaded && ( | ||
<Text.Span color="grey300" fontSize={18} fontWeight="medium"> | ||
최대 {maxFiles}장, 각 {maxFileSize}MB이하 | ||
</Text.Span> | ||
)} | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
접근성 개선이 필요합니다.
드래그 앤 드롭 영역에 대한 접근성 속성이 누락되어 있습니다. 스크린 리더 사용자를 위한 ARIA 속성을 추가하면 좋을 것 같습니다.
- <ImageUploader>
+ <ImageUploader
+ role="button"
+ aria-label={`이미지 업로드 영역. 최대 ${maxFiles}장, 각 ${maxFileSize}MB 이하`}
+ tabIndex={0}
+ >
<div
className={styles.textContent({
isCenter: !isImageUploaded,
})}
+ role="presentation"
>
📝 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.
<ImageUploader> | |
<div | |
className={styles.textContent({ | |
isCenter: !isImageUploaded, | |
})} | |
> | |
<Icon name="plusPicture" size={24} color="grey500" /> | |
<Text.Span color="grey600" fontSize={18} fontWeight="medium"> | |
이곳에 이미지를 드래그하거나 클릭하여 업로드 | |
</Text.Span> | |
{!isImageUploaded && ( | |
<Text.Span color="grey300" fontSize={18} fontWeight="medium"> | |
최대 {maxFiles}장, 각 {maxFileSize}MB이하 | |
</Text.Span> | |
)} | |
</div> | |
<ImageUploader | |
role="button" | |
aria-label={`이미지 업로드 영역. 최대 ${maxFiles}장, 각 ${maxFileSize}MB 이하`} | |
tabIndex={0} | |
> | |
<div | |
className={styles.textContent({ | |
isCenter: !isImageUploaded, | |
})} | |
role="presentation" | |
> | |
<Icon name="plusPicture" size={24} color="grey500" /> | |
<Text.Span color="grey600" fontSize={18} fontWeight="medium"> | |
이곳에 이미지를 드래그하거나 클릭하여 업로드 | |
</Text.Span> | |
{!isImageUploaded && ( | |
<Text.Span color="grey300" fontSize={18} fontWeight="medium"> | |
최대 {maxFiles}장, 각 {maxFileSize}MB이하 | |
</Text.Span> | |
)} | |
</div> |
const handleChange = useCallback( | ||
(e: ChangeEvent<HTMLInputElement>) => { | ||
if (e.target.files) { | ||
const files = Array.from(e.target.files); | ||
|
||
if (!validateFiles(files)) { | ||
toast.error( | ||
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | ||
); | ||
return; | ||
} | ||
|
||
if (validateFiles(files)) { | ||
onUpload(e.target.files); | ||
} | ||
} | ||
}, | ||
[onUpload, toast] | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
중복된 파일 유효성 검사 로직을 제거해주세요.
handleChange
함수에서도 validateFiles
검사가 중복되어 있습니다.
const handleChange = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
if (e.target.files) {
const files = Array.from(e.target.files);
if (!validateFiles(files)) {
toast.error(
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.'
);
return;
}
- if (validateFiles(files)) {
- onUpload(e.target.files);
- }
+ onUpload(e.target.files);
}
},
[onUpload, toast]
);
📝 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 handleChange = useCallback( | |
(e: ChangeEvent<HTMLInputElement>) => { | |
if (e.target.files) { | |
const files = Array.from(e.target.files); | |
if (!validateFiles(files)) { | |
toast.error( | |
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | |
); | |
return; | |
} | |
if (validateFiles(files)) { | |
onUpload(e.target.files); | |
} | |
} | |
}, | |
[onUpload, toast] | |
); | |
const handleChange = useCallback( | |
(e: ChangeEvent<HTMLInputElement>) => { | |
if (e.target.files) { | |
const files = Array.from(e.target.files); | |
if (!validateFiles(files)) { | |
toast.error( | |
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | |
); | |
return; | |
} | |
onUpload(e.target.files); | |
} | |
}, | |
[onUpload, toast] | |
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3;
요 ai 리뷰 확인 가능하실까요? 유효성 검사를 두 번 수행하는 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kongnayeon 네, 맞습니다! validateFiles
검사가 두 번 수행되고 있어요. 첫 번째 검사에서 유효하지 않은 파일이 있으면 이미 early return 되기 때문에, 두 번째 검사는 불필요합니다. AI 리뷰에서 제안한 것처럼 두 번째 검사를 제거하고 바로 onUpload
를 호출하는 것이 좋을 것 같습니다.
(`・ω・´)ゞ
const isValidFiles = files.every((file) => { | ||
const isImage = file.type.startsWith('image/'); | ||
const isValidSize = file.size <= MAX_FILE_SIZE; | ||
return isImage && isValidSize; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
이미지 타입 검증 로직 강화가 필요합니다.
현재 구현은 MIME 타입 검증이 충분하지 않으며, 허용되는 이미지 형식이 명확하지 않습니다.
다음과 같이 변경을 제안합니다:
+const ALLOWED_IMAGE_TYPES = [
+ 'image/jpeg',
+ 'image/png',
+ 'image/gif',
+ 'image/webp'
+] as const;
+
export function validateFiles(files: File[]) {
const isValidFiles = files.every((file) => {
- const isImage = file.type.startsWith('image/');
+ const isImage = ALLOWED_IMAGE_TYPES.includes(file.type);
const isValidSize = file.size <= MAX_FILE_SIZE;
return isImage && isValidSize;
});
Committable suggestion skipped: line range outside the PR's diff.
export function validateFiles(files: File[]) { | ||
const isValidFiles = files.every((file) => { | ||
const isImage = file.type.startsWith('image/'); | ||
const isValidSize = file.size <= MAX_FILE_SIZE; | ||
return isImage && isValidSize; | ||
}); | ||
|
||
if (!isValidFiles) { | ||
throw new Error( | ||
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | ||
); | ||
} | ||
|
||
return isValidFiles; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
함수의 반환 값과 에러 처리 로직 개선이 필요합니다.
함수가 true
를 반환하거나 에러를 발생시키는 현재 구조는 예측하기 어렵습니다. 또한 빈 배열에 대한 처리가 누락되어 있습니다.
다음과 같이 변경을 제안합니다:
export function validateFiles(files: File[]) {
+ if (files.length === 0) {
+ throw new Error('업로드할 파일을 선택해주세요.');
+ }
+
const isValidFiles = files.every((file) => {
const isImage = file.type.startsWith('image/');
const isValidSize = file.size <= MAX_FILE_SIZE;
return isImage && isValidSize;
});
if (!isValidFiles) {
throw new Error(
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.'
);
}
-
- return isValidFiles;
}
📝 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 validateFiles(files: File[]) { | |
const isValidFiles = files.every((file) => { | |
const isImage = file.type.startsWith('image/'); | |
const isValidSize = file.size <= MAX_FILE_SIZE; | |
return isImage && isValidSize; | |
}); | |
if (!isValidFiles) { | |
throw new Error( | |
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | |
); | |
} | |
return isValidFiles; | |
} | |
export function validateFiles(files: File[]) { | |
if (files.length === 0) { | |
throw new Error('업로드할 파일을 선택해주세요.'); | |
} | |
const isValidFiles = files.every((file) => { | |
const isImage = file.type.startsWith('image/'); | |
const isValidSize = file.size <= MAX_FILE_SIZE; | |
return isImage && isValidSize; | |
}); | |
if (!isValidFiles) { | |
throw new Error( | |
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | |
); | |
} | |
} |
const handleChange = useCallback( | ||
(e: ChangeEvent<HTMLInputElement>) => { | ||
if (e.target.files) { | ||
const files = Array.from(e.target.files); | ||
|
||
if (!validateFiles(files)) { | ||
toast.error( | ||
'유효하지 않은 파일이 포함되어 있어요. 이미지 파일(최대 5MB)만 업로드 가능해요.' | ||
); | ||
return; | ||
} | ||
|
||
if (validateFiles(files)) { | ||
onUpload(e.target.files); | ||
} | ||
} | ||
}, | ||
[onUpload, toast] | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3;
요 ai 리뷰 확인 가능하실까요? 유효성 검사를 두 번 수행하는 것 같아요!
import { Icon } from '../../../../../../../packages/ui/src/components/Icon'; | ||
import { Text } from '../../../../../../../packages/ui/src/components/Text'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p1;
apps/web은 절대경로 인식이 되니까 절대경로로 작성하면 될 것 같아요!
import { Icon } from '../../../../../../../packages/ui/src/components/Icon'; | |
import { Text } from '../../../../../../../packages/ui/src/components/Text'; | |
import { Icon } from '@repo/ui/Icon'; | |
import { Text } from '@repo/ui/Text'; |
관련 이슈
close: #93
변경 사항
프롬프트 내에서 사용하는 ImageManager TypeA 컴포넌트를 구현했어요. TypeB는 @kongnayeon 님께서 담당하실 예정이고, TypeB 폴더 내에서 작업해주시면 됩니다!
next/image에 의존하는 컴포넌트이어서 apps/web 내 components/common에 위치해두었어요!
레퍼런스
Summary by CodeRabbit
새로운 기능
버그 수정
스타일