-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add HEIC/HEIF to JPEG conversion utility with toast progress up… #170
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @dasosann, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 HEIC/HEIF 파일을 JPEG로 변환하는 유틸리티의 사용자 경험을 개선합니다. 변환 과정 중에 토스트 메시지를 통해 실시간 진행률을 제공함으로써, 사용자는 현재 작업 상태를 명확하게 인지할 수 있게 됩니다. 이는 특히 여러 파일을 동시에 변환할 때 유용하며, 작업의 투명성을 높여줍니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
안녕하세요. HEIC 파일 변환 시 진행률을 토스트 메시지로 보여주는 기능 추가 감사합니다. 다만 현재 구현은 파일 변환을 병렬이 아닌 순차적으로 처리하여 여러 파일을 올릴 때 성능 저하를 유발할 수 있습니다. 또한, 파일 하나가 변환될 때마다 토스트 메시지를 띄우는 방식은 사용자 경험을 해치고 잠재적인 UI 버그를 일으킬 수 있습니다. Promise.all을 사용해 병렬 처리의 이점을 살리면서, 토스트 메시지 사용을 개선하는 방향으로 코드 수정을 제안합니다.
| if (heicFiles.length === 0) { | ||
| return files; | ||
| } | ||
|
|
||
| const convertedFiles = await Promise.all( | ||
| files.map(async (file) => { | ||
| if (/heic|heif/i.test(file.type) || /\.heic$|\.heif$/i.test(file.name)) { | ||
| try { | ||
| const jpegBlob = await heicTo({ | ||
| blob: file, | ||
| let convertedCount = 0; | ||
| const totalCount = heicFiles.length; | ||
|
|
||
| // 초기 메시지 표시 | ||
| Toast.alert(`변환 중... 0/${totalCount}`); | ||
|
|
||
| const convertedFiles: File[] = []; | ||
|
|
||
| for (const file of files) { | ||
| if (/heic|heif/i.test(file.type) || /\.heic$|\.heif$/i.test(file.name)) { | ||
| try { | ||
| const jpegBlob = await heicTo({ | ||
| blob: file, | ||
| type: 'image/jpeg', | ||
| quality: 0.9, | ||
| }); | ||
| const convertedFile = new File( | ||
| [jpegBlob], | ||
| file.name.replace(/\.(heic|heif)$/i, '.jpg'), | ||
| { | ||
| type: 'image/jpeg', | ||
| quality: 0.9, | ||
| }); | ||
| return new File( | ||
| [jpegBlob], | ||
| file.name.replace(/\.(heic|heif)$/i, '.jpg'), | ||
| { | ||
| type: 'image/jpeg', | ||
| lastModified: file.lastModified, | ||
| }, | ||
| ); | ||
| } catch (e) { | ||
| Toast.alert(`${file.name} 파일의 변환에 실패했습니다.`); | ||
| return file; | ||
| } | ||
| lastModified: file.lastModified, | ||
| }, | ||
| ); | ||
| convertedFiles.push(convertedFile); | ||
|
|
||
| // 변환 완료마다 진행률 업데이트 | ||
| convertedCount++; | ||
| Toast.alert(`변환 중... ${convertedCount}/${totalCount}`); | ||
| } catch (e) { | ||
| Toast.alert(`${file.name} 파일의 변환에 실패했습니다.`); | ||
| convertedFiles.push(file); | ||
| } | ||
| return file; | ||
| }), | ||
| ); | ||
| } else { | ||
| convertedFiles.push(file); | ||
| } | ||
| } | ||
|
|
||
| return convertedFiles; |
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.
안녕하세요. HEIC 변환 진행 상태를 표시하는 기능은 좋은 시도입니다. 하지만 현재 구현에는 두 가지 큰 문제점이 있습니다.
- 성능 저하:
for...of루프와await를 사용하여 파일 변환을 순차적으로 처리하고 있습니다. 이전의Promise.all을 사용한 병렬 처리에 비해 여러 파일을 변환할 때 응답 속도가 크게 느려질 수 있습니다. - 토스트 메시지 남발 및 버그 가능성: 변환이 성공할 때마다
Toast.alert를 호출하면 짧은 시간에 많은 토스트 메시지가 생성되어 사용자 경험을 해칩니다. 또한, 현재Toast컴포넌트의 구현 방식 상,Toast.alert를 빠르게 연속 호출하면unmount타이머 관련 race condition이 발생하여 예기치 않은 UI 버그를 유발할 수 있습니다.
이러한 문제들을 해결하기 위해, Promise.all을 사용하여 병렬 변환을 유지하고 토스트 메시지 사용을 최소화하는 방식으로 리팩토링하는 것을 강력히 권장합니다. 아래 제안 코드는 병렬 처리를 복원하고, 변환 시작과 개별 파일 변환 실패 시에만 토스트를 보여주도록 수정한 것입니다. 실시간 진행률 표시는 현재 Toast 컴포넌트로는 불안정하므로, 더 안정적인 대안(예: Toast 컴포넌트 수정 또는 다른 UI 사용)을 고려해보시는 것이 좋겠습니다.
if (heicFiles.length === 0) {
return files;
}
Toast.alert(`${heicFiles.length}개의 HEIC 파일을 변환합니다...`);
const convertedFiles = await Promise.all(
files.map(async (file) => {
if (/heic|heif/i.test(file.type) || /\.heic$|\.heif$/i.test(file.name)) {
try {
const jpegBlob = await heicTo({
blob: file,
type: 'image/jpeg',
quality: 0.9,
});
return new File(
[jpegBlob],
file.name.replace(/\.(heic|heif)$/i, '.jpg'),
{
type: 'image/jpeg',
lastModified: file.lastModified,
},
);
} catch (e) {
Toast.alert(`${file.name} 파일의 변환에 실패했습니다.`);
return file;
}
}
return file;
}),
);
return convertedFiles;
요약
heic 변환 중에 토스트 메시지 뜨게 설정
구현 사항
📸 스크린샷
Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.