-
Notifications
You must be signed in to change notification settings - Fork 5
[feat, refactor] #190 팀 참여 페이지 작성 / 팀 생성, 수정 페이지 리팩토링 #200
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
|
✅ Preview Deployment |
|
✅ Preview Deployment |
jjanie00
left a comment
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(() => { | ||
| setName(initialName); | ||
| setPreview(initialPreview); | ||
| setFile(undefined); | ||
| setNameError(false); | ||
| setImageError(''); | ||
| }, [initialName, initialPreview]); |
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 로 props 로 전달되는 initialName 이 변경되면, 내부 file 상태도 초기화시킨 거군요~!
이전에 업로드한 파일 정보도 함께 초기화시킬 수 있어서 좋은 방법이네요 👍
| const { memberships } = useMemberships(true); | ||
|
|
||
| const [link, setLink] = useState(''); | ||
| const [error, setError] = useState<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.
error 초깃값을 undefined 로 보시고 일부러 비워두신 건지 궁금합니다~!
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.
undefined 를 에러가 없는 상태로 구분하려고 했습니다!
| useEffect(() => { | ||
| setError(undefined); | ||
| }, [link]); | ||
|
|
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.
유저가 링크를 수정할 경우, 이전 에러 메시지가 남아있지 않도록 기존의 에러 메시지를 삭제하는 패턴이 맞을까요?
해당 방식은 만약 사용자가 이전과 동일한, 잘못된 값을 다시 입력하더라도 에러 메시지가 사라졌다가 다시 나타나는, 불필요한 상태 업데이트가 발생할 수 있습니다. 입력값이 바뀌어도 기존 에러 메시지와 동일한 문제가 반복된다면, setError(undefined) 를 하지 않고 에러 메시지를 그대로 두는 방식도 추후 고려해보시면 좋을 것 같습니다 !
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 url = await uploadImage(data.file); | ||
| body.image = url; | ||
| } else if (data.removeImage) { | ||
| body.image = null; | ||
| } | ||
| return patchGroupById({ | ||
| groupId: teamId, | ||
| body: { name: data.name, ...(imageUrl ? { image: imageUrl } : {}) }, | ||
| body, |
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.
uploadImage 함수로 이미지 URL 을 받은 후, 필요한 데이터를 한꺼번에 body 객체에 모아 요청을 보내는 방식 깔끔하네요 👍
ARON-Y
left a comment
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.
유효성 검사랑 에러처리도 꼼꼼하게 잘하셨네요 수고하셨습니다 현호님!🫡
🔗 이슈 번호
Closes #190
📋 작업 사항
팀 참여 페이지 작성
팀 생성, 수정 페이지 리팩토링
디자인 수정 (spinner 추가)
프로필 이미지 삭제하는 아이콘 추가
(10mb -> 4.2mb) 넘어가는 파일 선택 시 오류 처리
에러처리 코드는 페이지로 분리
기타 수정
이미지 없는 팀의 기본 이미지 -> 아이콘으로 대체