-
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 사용)을 고려해보시는 것이 좋겠습니다.