-
Notifications
You must be signed in to change notification settings - Fork 6
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: 프로필 설정 페이지 구현 및 공용 폰트 업데이트 #30
Feat: 프로필 설정 페이지 구현 및 공용 폰트 업데이트 #30
Conversation
@seoyoung-min is attempting to deploy a commit to the Eujin Ahn's projects Team on Vercel. A member of the Team first needs to authorize it. |
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.
서영님, 프로필 수정 기능 구현하시느라 정말 고생많으셨습니다!!!
보다보니 어느새 2시간이 흘러있네요 ㅋㅋㅋㅋ 그래도 하나하나 보면서 서영님께서 많이 생각하시고 작성한 코드라는 생각이 내내 들었습니다. 덕분에 시간이 금방갈 정도로 몰입해서 본 것 같습니다.
어깨너머 리액트 훅 폼에 대해 많이 배운것 같습니다. 감사합니다.🩷
src/app/_api/user/updateProfile.ts
Outdated
|
||
interface updateProfileParams { | ||
userId: Number; | ||
data: UserProfileEditType; |
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.
소현님 매번 이 부분을 놓치네요ㅠㅠ 앞으로 더욱 꼼꼼하게 체크하겠습니다!
감사합니다 🙇♀️👏
src/app/_api/user/updateProfile.ts
Outdated
|
||
//프로필 수정 | ||
const profileData: UserProfileInfoType = { | ||
nickname, | ||
description, | ||
backgroundImageUrl, | ||
profileImageUrl, | ||
}; | ||
const result = await axiosInstance.patch(`/users/${userId}`, profileData); | ||
|
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.
서영님 혹시 profileData 부분은 정의하지 않고 바로 사용하는 방법은 어떨까요..? 그렇게 된다면 타입을 선언하지 않아도 될 것 같고, UserProfileInfoType이 많이 사용되지 않는것 같아서 공용type에서도 제외하면 좋을 것 같아서요..! 🤔
const result = await axiosInstance.patch(`/users/${userId}`, {
nickname,
description,
backgroundImageUrl,
profileImageUrl,
});
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.
정말 너무 좋은 생각인 것 같습니다!
UserProfileInfoType을 2군데에서 사용하려고 공용으로 빼두었는데, 한 군데에서 쓸모 없게 된 상황이었어요! 공용 타입에서 빼는 것을 생각못하고 있었는데, 덕분에 공용 타입 삭제도 하고 코드도 훨씬 깔끔해졌네요😻👍
src/app/_api/user/updateProfile.ts
Outdated
if (result.status === 204 && (newBackgroundFileList !== null || newProfileFileList !== null)) { | ||
//1. presignedUrl 생성요청 |
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.
서영님 이부분은 배경이나 프로필 이미지가 바뀔때만 실행하는 로직(즉, new url이 있는 경우)이 맞나요?
그렇다면 배경이나 프로필 이미지가 동일하다면 먼저 return을 시키는 방향으로 가면 어떨까요..?🤔 return문이 맨 마지막에 있는 것 보다 먼저 나오면 코드 읽기가 더 쉬울것 같아서요..!
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 { onClickMoveToPage } = useMoveToPage(); | ||
return ( | ||
<> | ||
<div>마이페이지</div> | ||
<button onClick={onClickMoveToPage('account/profile')}>프로필설정</button> |
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.
뿌듯하네요😊 ㅋㅋㅋㅋ
alignItems: 'center', | ||
|
||
position: 'relative', | ||
|
||
borderRadius: '30px', | ||
|
||
overflow: 'hidden', |
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.
서영님 혹시 이렇게 각기 다른 속성이 한개씩만 있는 경우에는 공백없이 작성하는 것도 좋을 것 같은데 서영님 생각은 어떠신가요?!
css 컨벤션을 정했지만 그 이유가 가독성 측면이어서 지금처럼 4~5줄은 붙여도 될 것 같아서 의견드려봅니다..!
+. 하지만, 가독성은 개인차도 있을 것 같아서 참고로만 봐주시면 좋을 것 같습니다.
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.
오! 저도 순서만 지켜진다면, 각기 다른 속성 한개씩만 있는 경우 공백없이 작성하는 것 좋아보입니다!! 내일 안건으로 이야기 나눠보면 좋을 것 같아요 :)👍
src/lib/constants/toastMessage.ts
Outdated
|
||
export const editProfileToastMessage = { | ||
editProfileSuccess: '프로필을 수정했습니다.🥰', | ||
editProfileError: '프로필 수정에 실패했어요. 다시 시도해주세요.🥲', | ||
imageSizeError: '사진이 너무 커요. 5MB 이하 사진을 넣어주세요.🥹', | ||
}; |
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.
이모티콘 넘 기엽네요 서영님😊
그리고 토스트에러메세지 넣으면서 생각해본건데 다국어기능 지원을 위해 처음부터 아래처럼 ko와 en을 나누어 상수화해도 좋을 것 같다는 생각이 들었습니다, 그렇게되면 같은 Key를 사용할 수 있고, 언어부분(ko/en)만 객체의 키로 접근하거나, 파라미터로 보내주면 더 간결하게 다국어를 지원할 수 있을 것 같습니다.
export const toastMessage = {
ko: {
requiredLogin: '로그인이 필요해요.',
limitFollow: `최대 ${MAX_FOLLOWING.toLocaleString('ko-KR')}명까지 팔로우할 수 있어요.`,
editProfileSuccess: '프로필을 수정했습니다.🥰',
editProfileError: '프로필 수정에 실패했어요. 다시 시도해주세요.🥲',
imageSizeError: '사진이 너무 커요. 5MB 이하 사진을 넣어주세요.🥹',
},
// 언어변경 시, en 추가
en: {
editProfileSuccess: 'abcd',
editProfileError: '...',
imageSizeError: '...',
},
};
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.
감사합니다 소현님😻 이모지 넣는 것이 괜찮을지 논의가 필요한데, 우선은 넣어두었어요!ㅎㅎ
정말 좋은 생각인 것 같습니다! 우선 아래와 같이 수정해두었습니다! (gpt야 번역 고마워~)
export const editProfileToastMessage = { | |
editProfileSuccess: '프로필을 수정했습니다.🥰', | |
editProfileError: '프로필 수정에 실패했어요. 다시 시도해주세요.🥲', | |
imageSizeError: '사진이 너무 커요. 5MB 이하 사진을 넣어주세요.🥹', | |
}; | |
const toastMessage = { | |
ko: { | |
uploadImageError: '이미지를 업로드에 실패했어요. 다시 업로드해주세요.🥲', | |
createListError: '리스트 생성에 실패했어요. 다시 시도해주세요.🥲', | |
updateProfileSuccess: '프로필을 수정했습니다.🥰', | |
updateProfileError: '프로필 수정에 실패했어요. 다시 시도해주세요.🥲', | |
imageSizeError: '사진이 너무 커요. 50MB 이하 사진을 넣어주세요.🥹', | |
}, | |
en: { | |
uploadImageError: 'Failed to upload the image. Please try again.🥲', | |
createListError: 'Failed to create the list. Please try again.🥲', | |
updateProfileSuccess: 'Profile updated successfully.🥰', | |
updateProfileError: 'Failed to update the profile. Please try again.🥲', | |
imageSizeError: 'The image is too large. Please insert an image smaller than 50MB.🥹', | |
}, | |
}; | |
export default toastMessage; | |
profileImageUrl?: string; | ||
profileImageUrl?: string /**TODO: 옵셔널 없애기 */; | ||
backgroundImageUrl?: 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.
이 부분 온보딩 완성후에 타입 같이 살펴보아요 서영님~~!
src/lib/utils/debounce.ts
Outdated
|
||
export default function debounce<T extends (...args: any[]) => any>(callback: T, delay: number) { | ||
let timeout: ReturnType<typeof setTimeout>; | ||
|
||
return (...args: Parameters<T>): ReturnType<T> => { | ||
let result: any; | ||
|
||
if (timeout) clearTimeout(timeout); | ||
timeout = setTimeout(() => { | ||
result = callback(args); | ||
}, delay); | ||
|
||
return result; | ||
}; |
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.
서영님, 이 디바운싱은 닉네임을 입력하면 리액트훅폼에서 onChange가 발생하는 것을 delay 시켜주는 함수가 맞나요?!
위의 코드에서 볼때는 value를 전달하고 있는 것 같아서 궁금해서 여쭤봅니다!!
+. 만약 value를 전달한다면 onChange는 그대로 발생하게하고, 에러검증하는 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.
특정 함수(callback)을 실행이 delay되는 형태로 바꾸어 return 해주는 함수입니다!
- 디바운스 함수에 닉네임중복검사 함수(=callback)를 전달
- 닉네임중복검사 실행이 delay되는 형태의 새로운 함수를 return
- 새로운 함수인
debouncedOnNicknameChange(e.target.value)
로 닉네임중복검사 함수를 실행 - => onChange가 아닌 닉네임중복검사 함수 실행이 delay되도록 함
즉 debounce 함수의 return은 새로운 디바운싱함수(?)이고, 디바운싱함수의 return은 callback함수의 실행값(여기서는 닉네임중복검사 실행결과 boolean) 입니다!
혹시 현재 실행되는 방식이 소현님께서 말씀하신 방법이 맞을까요??
그리고 소현님 말씀을 듣고 나니 debounce라는 함수명이 안어울리는 것 같기도 하네요ㅠㅠ!!
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.
서영님! 상세히 설명 해 주셔서 감사합니다.
-
우선 jsdoc에 return에 대한 설명도 추가하면 좋을 것 같습니다.
(ex) @returns {function} 함수(callback)을 실행이 delay되는 형태로 바꾸어 return 해주는 함수
-
사실 제가 생각한 방식은 서영님께서 구현하신 방식이랑은 조금 다르게 생각했습니다!
- onChange 이벤트는 그대로 발생한다.
- 다만, api 호출날릴때 들어가는 value를 onChange value가 아닌 debounced value로 넣는다.
- debounced value는 value를 받아서 delay 후에 value를 return 한다. (useDebounce hook으로 구현)
- 예를 들어, ㄱ,ㅏ,ㄴ,ㅏ,ㄷ,ㅏ에 대한 중복검사를 했다면, debounced value를 받아서 가,나,다에 대한 중복검사를 실행하는 형태
- 사실 위 방법도 단점인 부분도 있어서 서영님께서 구현하신 방법으로 api 요청을 delay시키는 방법으로 가는것도 좋아보입니다. (+. 서영님 말씀대로 파일명만 조금 더 알맞게 수정하면 더욱 좋을 것 같습니다! )
|
||
const fileToBase64 = async (file: File, setter: (arg: string) => void) => { | ||
const reader = new FileReader(); | ||
reader.readAsDataURL(file); | ||
reader.onloadend = () => { | ||
setter(reader.result as string); | ||
}; | ||
}; | ||
|
||
export default fileToBase64; |
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.
👍👍👍
src/styles/font.css.ts
Outdated
//위에 지우기 | ||
|
||
export const headlineLarge = style({ | ||
fontSize: '3.2rem', | ||
fontWeight: '600', | ||
lineHeight: '4.0rem', | ||
}); | ||
|
||
export const headlineMedium = style({ | ||
fontSize: '2.8rem', | ||
fontWeight: '600', | ||
lineHeight: '3.6rem', |
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.
서영님 리뷰확인하여 다시 리뷰 남겼습니다~ 감사합니다. 🥰
} else { | ||
newBackgroundImageRegister.onChange(e); |
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.
오호! 완전 이해했습니다 서영님~! 보통 쓰던 방식과는 달라서 여쭤봤는데 명확히 해결되었습니다! ㅎㅎ
src/lib/utils/debounce.ts
Outdated
|
||
export default function debounce<T extends (...args: any[]) => any>(callback: T, delay: number) { | ||
let timeout: ReturnType<typeof setTimeout>; | ||
|
||
return (...args: Parameters<T>): ReturnType<T> => { | ||
let result: any; | ||
|
||
if (timeout) clearTimeout(timeout); | ||
timeout = setTimeout(() => { | ||
result = callback(args); | ||
}, delay); | ||
|
||
return result; | ||
}; |
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.
서영님! 상세히 설명 해 주셔서 감사합니다.
-
우선 jsdoc에 return에 대한 설명도 추가하면 좋을 것 같습니다.
(ex) @returns {function} 함수(callback)을 실행이 delay되는 형태로 바꾸어 return 해주는 함수
-
사실 제가 생각한 방식은 서영님께서 구현하신 방식이랑은 조금 다르게 생각했습니다!
- onChange 이벤트는 그대로 발생한다.
- 다만, api 호출날릴때 들어가는 value를 onChange value가 아닌 debounced value로 넣는다.
- debounced value는 value를 받아서 delay 후에 value를 return 한다. (useDebounce hook으로 구현)
- 예를 들어, ㄱ,ㅏ,ㄴ,ㅏ,ㄷ,ㅏ에 대한 중복검사를 했다면, debounced value를 받아서 가,나,다에 대한 중복검사를 실행하는 형태
- 사실 위 방법도 단점인 부분도 있어서 서영님께서 구현하신 방법으로 api 요청을 delay시키는 방법으로 가는것도 좋아보입니다. (+. 서영님 말씀대로 파일명만 조금 더 알맞게 수정하면 더욱 좋을 것 같습니다! )
src/lib/constants/toastMessage.ts
Outdated
}, | ||
en: { | ||
uploadImageError: 'Failed to upload the image. Please try again.🥲', | ||
createListError: 'Failed to create the list. Please try again.🥲', | ||
updateProfileSuccess: 'Profile updated successfully.🥰', | ||
updateProfileError: 'Failed to update the profile. Please try again.🥲', | ||
imageSizeError: 'The image is too large. Please insert an image smaller than 50MB.🥹', | ||
}, | ||
}; |
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.
ㅋㅋㅋ 서영님 en은 그냥 주석으로 달아놓자는 의미였는데 이렇게 잘 작성을 해주셨네요 ㅎㅎ👍🍫
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 달인이시네용!!👍코드도 깔끔하게 잘 쓰셔서 부럽습니다👀 LGTM!
개요
작업 사항
참고 사항 (optional)
browser-image-compression
라이브러리를 사용했습니다. 많이 사용되고 최근까지도 업데이트 하는 것으로 확인했습니다.관련 이슈 (optional)
close 사파리 input 가로길이 고정 불가 에러 #29
스크린샷
마이페이지 > 프로필 설정 연결 / 프로필 조회하여 기본값 채우기 / router.back으로 뒤로가기
수정사항 있을 경우 버튼 활성화
이미지 업로드 + 미리보기
닉네임 에러
닉네임 중복검사 디바운스
리뷰어에게