-
Notifications
You must be signed in to change notification settings - Fork 3
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/#351] 공연 등록하기 상세 이미지 클라 구현 #355
Conversation
비동기적으로 파일을 읽어와서 순서대로 미리보기 구현 max값인 10을 넘기면 작업 이행되지 않고 안내하기 정확한 속성 이름은 명세서 나오면 변경할 예정(구성도 변경 예정)
PR 작성하느라 고생 많았어요!! 라벨 잘 지정되었는지 확인 한 번 해 주기 🫶 |
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.
큰 문제 없어 보여서 어푸함미다앙!! 의문 생기는 부분이나 테스트 해봐야 할 것 같은 부분 PR에 나중에 반영한다고 정리되어 있어서 그 부분 반영하고 머지하거나 오래 걸릴 것 같으면 머지하구 반영하면 될 것 같아요!!
fileReader.onload = function (event) { | ||
const imageUrl = event.target?.result as string; | ||
resolve({ | ||
id: Date.now() + Math.floor(Math.random() * 1000), // ctrl 키로 동시에 이미지 선택해도 id 중복되지 않도록 랜덤 값 추가 |
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.
p5) 와웅 id idx로 주는 게 좋지 않은 방법이라고 해서 항상 id 값 줄 때 고민이 많았는데 이런 방법도 있군요!!
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.
p5) 👍
// 최대 10장 업로드 안내 | ||
if (previewImgs.length + files.length > 10) { |
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.
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.
고생하셨습니다~ 코멘트 한번 확인해주세요 !
useEffect(() => { | ||
setPreviewImgs(value || null); | ||
}, [value]); |
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.
p5) value 속성이 변해서 리렌더링이 발생했을 경우에만, DetailImage의 상태 업데이트 좋네요!
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) 저는 오히려 반대로,,, 이 로직이 필요한 이유가 뭘까요? 제가 지우고 테스트 했는데 정상적으로 동작해서요!
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.
헉 그러게요... 그냥 value에 따라 바뀌어야 한다고 생각했는데, 이미지 업로드하기 전에 previewImgs를 직접 바꿔줘서 value가 독단적으로 바뀔일이 없을 것 같습니다...! 이거를 그냥 살리고 뒤에 있는 필요없는 setPreviewImgs를 지우겠습니다.
if (files && files.length > 0) { | ||
// 최대 10장 업로드 안내 | ||
if (previewImgs.length + files.length > 10) { | ||
openAlert({ | ||
title: "가능한 이미지 수를 초과했습니다.", | ||
okText: "확인", | ||
}); |
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.
p5) DetailImage 컴포넌트 내부에 상태로 이미 존재하던 파일과, 새롭게 업로드 되는 파일의 개수가 도합 10개가 넘어가는지 확인하고 alert를 띄우는 로직 좋습니다 ! 디테일하게 에러 처리하셨네요
const processAllFiles = async () => { | ||
const newPreviewImgs = await Promise.all( | ||
Array.from(files).map((file) => processFile(file)) | ||
); | ||
const updatedPreviewImgs = [...previewImgs, ...newPreviewImgs]; | ||
setPreviewImgs(updatedPreviewImgs); | ||
onImagesUpload(updatedPreviewImgs); | ||
}; |
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.
p5) 여러 개의 프로미스를 병렬로 실행하기 위해 Promise.all 키워드를 사용하셨네요!
processFile 함수 자체가 promise를 반환하니까, 결국 모든 파일을 배열로 만들어 둔 뒤 processFile을 각각 실행해서 프로미스를 newPreviewImgs에 파일의 처리 결과를 배열로 저장해두셨네요. precessFile 자체를 async로 굳이 만들지 않고, 코드를 명확히 분리한 게 좋네요 !
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.
p5) 여기도 최고 입니다 !!
return new Promise<PreviewImageList>((resolve) => { | ||
const fileReader = new FileReader(); | ||
fileReader.onload = function (event) { | ||
const imageUrl = event.target?.result as string; |
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.
p5) onload 함수의 경우 파일을 바탕으로readAsDataURL 메서드를 자체적으로 호출한 뒤 Base64 인코딩 문자열의 형태로 result 형태로 반환한다더라구요! as string 잘 쓴 걸 보면 알 것 같긴 한데 혹시 모르실까봐 참고하시라고 코멘트합니다 ~
url: imageUrl, | ||
}); | ||
}; | ||
fileReader.readAsDataURL(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.
p5) 아! 그리고 readAsDataURL 메서드의 경우에는 파일리더를 통해 파일을 비동기적으로 읽고, 읽기가 성공하면 onload 이벤트가 발생한다고 하네요. 이런 연관성도 알아두면 재밌을 것 같아요 ~
<Spacing marginBottom="1.4" /> | ||
<S.FilesInputWrapper> | ||
<S.HiddenFileInput | ||
key={inputKey} |
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.
p5) inputKey 어디서 쓰나 계속 봤는데 여기서 사용했었네요 ㅎㅎ..
혹시 HiddenFileInput의 역할은 뭔지 설명 부탁드려도 될까요?
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.
p5) 저는 inputKey의 역할이 궁금해요!
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.
이 코드는 사실 포스터 업로드하는 코드 파일을 복제해서 사용했습니다. 그래서 HiddenFileInput을 그대로 가져와서 조금만 변형시켜서 작성했습니다.
일단, HiddenFileInput의 역할 설명을 어떤 것을 원하시는건지 잘 모르겠습니다.....
HiddenFileInput은 사진 파일 업로드 버튼입니다!
InputKey는 포스터 등록할때 정의한 것 그대로 사용하고 있습니다!
사진을 올리고 내렸다가 다시 같은 사진을 올렸을 때, 변화가 일어나지 않는다고 생각해서 onChange가 작동되지 않았고, 그래서 사진을 선택해도 프리뷰에 사진이 뜨지 않았습니다!!!
이 문제를 해결하기 위해서 inputKey로 선택한 시간을 정의해서 같은 사진을 다시 선택해도 문제 없도록 했습니다!!
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.
채현 고생했오 ~~
코멘트 확인해줘!
지금 상세 이미지 기능 같은 경우에는 presigned url을 get해서 put으로 이미지 업로드 하는 로직 추가해야 할 것 같아~!
그리고 이미지 들어가는 곳에 동영상 못 들어가도록 막는 것도 추가해줘!
고생했어 ~~👍
}; | ||
const removeImage = (id: 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.
p5) 키키 띄어쓰기 부탁드립니다 ~
}; | |
const removeImage = (id: number) => { | |
}; | |
const removeImage = (id: number) => { |
type PreviewImageList = { | ||
id: number; | ||
url: string; | ||
}; |
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.
p5) 얘는 타입으로 하신 이유가 있을까요?
저는 type을 사용할 때는 리터럴 타입으로 enum처럼 사용할 때 사용합니다! 그게 아니라면 interface로 선언하는 편이구요!
ts 관련해서 좋은 글들이 있어서 남길게요!
https://typescript-kr.github.io/pages/literal-types.html
https://toss.tech/article/template-literal-types
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.
오호 좋은글 감사합니다!!! 그냥 뭐 딱히 크게 사용될 일 없어서 type으로 쓰는게 더 개발하기 편할 것 같아서 그렇게 했는데, 객체는 interface를 사용하는게 더 권장되니까 반영했습니다!!
return ( | ||
<S.InputRegisterBox $marginBottom={2.8}> | ||
<S.InputTitle>공연 상세 이미지</S.InputTitle> | ||
<S.InputDescription>선택 사항입니다. ({previewImgs.length}/10)</S.InputDescription> |
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.
useEffect(() => { | ||
setPreviewImgs(value || null); | ||
}, [value]); |
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) 저는 오히려 반대로,,, 이 로직이 필요한 이유가 뭘까요? 제가 지우고 테스트 했는데 정상적으로 동작해서요!
fileReader.onload = function (event) { | ||
const imageUrl = event.target?.result as string; | ||
resolve({ | ||
id: Date.now() + Math.floor(Math.random() * 1000), // ctrl 키로 동시에 이미지 선택해도 id 중복되지 않도록 랜덤 값 추가 |
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.
p5) 👍
// 파일 순서대로 처리하기 위해 비동기 작업 | ||
const processFile = (file: File) => { | ||
return new Promise<PreviewImageList>((resolve) => { | ||
const fileReader = new FileReader(); | ||
fileReader.onload = function (event) { | ||
const imageUrl = event.target?.result as string; | ||
resolve({ | ||
id: Date.now() + Math.floor(Math.random() * 1000), // ctrl 키로 동시에 이미지 선택해도 id 중복되지 않도록 랜덤 값 추가 |
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.
p5) 오옹 이런 작업을!!! 최고채현!!
const processAllFiles = async () => { | ||
const newPreviewImgs = await Promise.all( | ||
Array.from(files).map((file) => processFile(file)) | ||
); | ||
const updatedPreviewImgs = [...previewImgs, ...newPreviewImgs]; | ||
setPreviewImgs(updatedPreviewImgs); | ||
onImagesUpload(updatedPreviewImgs); | ||
}; |
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.
p5) 여기도 최고 입니다 !!
<Spacing marginBottom="1.4" /> | ||
<S.FilesInputWrapper> | ||
<S.HiddenFileInput | ||
key={inputKey} |
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.
p5) 저는 inputKey의 역할이 궁금해요!
Quality Gate failedFailed conditions |
비동기적으로 파일을 읽어와서 순서대로 미리보기 구현
max값인 10을 넘기면 작업 이행되지 않고 안내하기
정확한 속성 이름은 명세서 나오면 변경할 예정(구성도 변경 예정)
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
✅ Key Changes
📢 To Reviewers
📸 스크린샷
🔗 참고 자료