Qnrr 463 조민주 스터디 신청 회원 불러오기 UI 및 api 적용#122
Conversation
- labelField를 기존 formfield를 이용하는 방식으로 변경 - 스키마를 이용한 rhf 추가 - 토글 그룹 분리
- label 변경 - input 스타일 추가 - 멀티 셀렉터에 대소문자 판별 옵션 추가 및 코드 수정
aken-you
left a comment
There was a problem hiding this comment.
신청 리스트는 보다가 멈췄어요.
신청 리스트 부분은 slack으로 민주님이 연락 주시면 그 때 리뷰할게요!
제가 궁금한 점은 form에 관련된 부분인데요. 한 번에 이해하기는 힘들더라구요 😢
특히 shared/ui/form 폴더에 있는 파일들은 알아야 할 것 같은데, 이해하진 못했습니다..
혹시 설명 부탁드려도 될까요?
There was a problem hiding this comment.
기본 이미지가 바뀌었다기보단, 음...신청 리스트에 기본 이미지와 동일한 데 색상만 다른 경우가 있었습니다.
그래서 이미지를 또 추가하기보다는 svg 파일을 svgr을 통해 나타내는 방식으로 바꿨습니다.
There was a problem hiding this comment.
헉 여기 적힌 줄 모르고 제가 아래에 따로 작성을 했네요. 답변 이동했습니다!
넵 그렇습니다. 원하는 색상을 넣을 수도 있고, 제가 기본값으로 색상을 설정해둬서 만약 Avatar 파일을 이용하는 경우 따로 색을 변경하고 싶지 않은 이상 입력하지 않아도 괜찮습니다.
| @@ -0,0 +1 @@ | |||
| nodeLinker: node-modules | |||
There was a problem hiding this comment.
저도 yarn으로 설치했을 때, 이 파일이 생기더라고요. 흠 왜생기는지는 저도 잘 모르겠네요 🤔
There was a problem hiding this comment.
헉 이 파일이 추가됐군요. 혹시 모를 부분이라 추가하지 않으려 했는데 추가되었나봅니다..
저도 이 파일에 대해서 알아봤는데, 그냥 프로젝트에서 node-modules를 사용하는 방식이라고 명시해주는 거라고 하더라구요.
원래도 저희는 node-modules를 사용하고는 있었지만, 따로 명시되어 있지 않아 node-module인지, pnp인지 방식을 확실히 알 수 없다고 합니다.
그래서 저 파일을 이용해 이 프로젝트는 node-modules을 사용하는 파일이다! 라고 명시해주는 거라고 하더라구요.
문제가 되는 파일인 것 같지 않아 그냥 제 로컬에만 놔두려했는데...ㅜㅜ 커밋을 수정하는 과정에서 추가되었나봅니다.
수아님도 괜찮으시다면 이 파일은 그냥 추가해둘까 합니닷.
src/shared/ui/avatar/index.tsx
Outdated
| type ProfileImageSrc = string | undefined; | ||
|
|
||
| function getValidImageUrl(src: ProfileImageSrc) { | ||
| const s = (src ?? '').trim(); |
There was a problem hiding this comment.
s보다 더 의미있는 네이밍으로 짓는게 어떨까요?!
trimedSrc 이런 식으로요!
There was a problem hiding this comment.
넵. 사실 이 부분에서만 쓰이는 값이고 그렇게 중요한 값이 아니라서 놔뒀었는데 이 부분도 네이밍 더 자세하게 해 두도록 하겠습니다.
| const [isImageError, setImageError] = useState(false); | ||
|
|
||
| const resolvedImageUrl = useMemo(() => { | ||
| setImageError(false); | ||
|
|
||
| return getValidImageUrl(image); | ||
| }, [image]); | ||
|
|
||
| const showImage = !!resolvedImageUrl && !isImageError; |
There was a problem hiding this comment.
이 로직은 현재 들어온 이미지 URL이 로딩에 실패했는지 여부를 확인하기 위한 로직입니다.
만약 이미지 주소가 잘못되거나(404), 만료되거나 뭐 CORS같은 오류가 발생했을 때 태그에서 기본적으로 깨진 이미지 아이콘을 보여줍니다.
예를 들어 저희 프로젝트에서도 가끔 url에서 앞에 http 같은 주소를 붙여주지 않아서 깨지는 경우도 보셨을 거라고 생각하는데욧.
그때 깨진 이미지를 보여주기보다는 기본 프로필 아이콘을 보여주도록 설정한 로직입니다.
코드는 아래 AvatarImage에서 onError를 통해 이미지 로딩 실패를 확인하면 컴포넌트가 다시 렌더링 되면서 AvatarFallback을 통해 기본 이미지를 보여주는 로직입니다.
| <AvatarFallback> | ||
| <ProfileDefault | ||
| className="h-full w-full" | ||
| style={{ color: accentColor }} |
There was a problem hiding this comment.
accentColor는 어떤 스타일을 위한걸까요?
There was a problem hiding this comment.
위에 잠깐 언급했던 부분인데, 신청 리스트에서 기본 프로필 이미지와 동일하지만 색상만 다른 이미지가 들어가야 하는 버튼이 있습니다.
이걸 위해 png 파일을 추가하기보다는 기본 svg 파일을 이용해 기본적으로는 프로필 이미지의 색상으로 보여주지만, color 값을 따로 입력하면 해당 컬러값에 맞는 svg로 보여주기 위해 추가한 값입니다.
src/shared/ui/dropdown/multi.tsx
Outdated
| {filteredOptions.length > 0 ? ( | ||
| filteredOptions.map((option) => ( | ||
| {remainingOptions.length > 0 ? ( | ||
| remainingOptions.map((o) => ( |
src/shared/ui/dropdown/multi.tsx
Outdated
|
|
||
| return ( | ||
| <DropdownMenu open={isOpen} onOpenChange={setIsOpen}> | ||
| <DropdownMenu open={open} onOpenChange={(o) => !disabled && setOpen(o)}> |
| const original = user.profileImage?.resizedImages.find( | ||
| (img) => img.imageSizeType.imageTypeName === 'ORIGINAL', | ||
| )?.resizedImageUrl; |
There was a problem hiding this comment.
resizedImages에서 imageTypeName이 ORIGINAL인 사진은 1장뿐인걸까요?
그리고 이 함수가 이 파일에 위치하는 이유도 궁금합니다
There was a problem hiding this comment.
서버에 정확하게 여쭤본 건 아니지만 기본적으로 resizedImages와 같이 보여주는 경우는
이미지를 small, large 같은 값과 동시에 썸네일 데이터를 보내주기도 하고(아마 이 경우는 화질의 변경이 있다거나 사이즈의 차이가 있다거나 할 것 같습니다), ORIGINAL 값을 통해 원본 데이터를 줍니다. (많은 경우...?)
저도 이 경우라고 생각을 했고, 오리지널이라 명시된 값을 프로필 사진에 쓰기 위해 해당 코드를 추가했습니다.
사실 추가하면서 이 오리지널 함수를 따로 빼서 프로필 이미지를 하는 경우에 쓰면 좋겠다고 생각은 했는데......
뭔가 지금 리팩토링하긴 좀 애매하고 나중에 이 부분은 따로 유틸함수로 빼면 좋겠다고 생각하고 있었습니다.
There was a problem hiding this comment.
서버에 정확하게 여쭤본 건 아니지만 기본적으로 resizedImages와 같이 보여주는 경우는
이미지를 small, large 같은 값과 동시에 썸네일 데이터를 보내주기도 하고(아마 이 경우는 화질의 변경이 있다거나 사이즈의 차이가 있다거나 할 것 같습니다), ORIGINAL 값을 통해 원본 데이터를 줍니다.
이 부분은 서버 분들에게 다시 한 번 여쭤봐야겠네요!
|
Form 부분은 저도 자세히 설명드리긴 좀 어렵고 어느정도 이해하실 수 있는...? 코드 흐름을 말씀드릭겠습니다.
|
aken-you
left a comment
There was a problem hiding this comment.
폼과 관련된 부분 설명해주셔서 감사합니다!
나중에라도 폼쪽 코드는 자세히 보겠습니다 😢
| const original = user.profileImage?.resizedImages.find( | ||
| (img) => img.imageSizeType.imageTypeName === 'ORIGINAL', | ||
| )?.resizedImageUrl; |
There was a problem hiding this comment.
서버에 정확하게 여쭤본 건 아니지만 기본적으로 resizedImages와 같이 보여주는 경우는
이미지를 small, large 같은 값과 동시에 썸네일 데이터를 보내주기도 하고(아마 이 경우는 화질의 변경이 있다거나 사이즈의 차이가 있다거나 할 것 같습니다), ORIGINAL 값을 통해 원본 데이터를 줍니다.
이 부분은 서버 분들에게 다시 한 번 여쭤봐야겠네요!
| export function buildProfileInfoDefaultValues( | ||
| member: MemberInfo, | ||
| ): ProfileInfoFormValues { | ||
| return { | ||
| selfIntroduction: member.selfIntroduction ?? '', | ||
| studyPlan: member.studyPlan ?? '', | ||
| preferredStudySubjectId: member.preferredStudySubject | ||
| ? String(member.preferredStudySubject.studySubjectId) | ||
| : undefined, | ||
| availableStudyTimeIds: (member.availableStudyTimes ?? []) | ||
| .map((t) => t?.id) | ||
| .filter((x): x is number => typeof x === 'number') | ||
| .map(String), | ||
| techStackIds: (member.techStacks ?? []) | ||
| .map((t) => t?.techStackId) | ||
| .filter((x): x is number => typeof x === 'number') | ||
| .map(String), | ||
| }; | ||
| } | ||
|
|
||
| export function toUpdateUserProfileInfoRequest( | ||
| v: ProfileInfoFormValues, | ||
| ): UpdateUserProfileInfoRequest { | ||
| return { | ||
| selfIntroduction: v.selfIntroduction, | ||
| studyPlan: v.studyPlan, | ||
| preferredStudySubjectId: v.preferredStudySubjectId!, | ||
| availableStudyTimeIds: v.availableStudyTimeIds.map(Number), | ||
| techStackIds: v.techStackIds.map(Number), | ||
| }; | ||
| } |
There was a problem hiding this comment.
폼과 관련된 코드들은 위 파일과 같이 비슷한 형식이더라구요!
혹시 정해두신 컨벤션인걸까요?
There was a problem hiding this comment.
넵 컨벤션이라기엔 좀 민망하지만
이왕 form 변경하는 김에 동일한 형식으로 작성했습니다.
| const subjectOptions = useMemo( | ||
| () => | ||
| studySubjects.map(({ studySubjectId, name }) => ({ | ||
| value: String(studySubjectId), | ||
| label: name, | ||
| })), | ||
| [studySubjects], | ||
| ); | ||
|
|
||
| const timeOptions = useMemo( | ||
| () => | ||
| availableStudyTimes.map(({ availableTimeId, display }) => ({ | ||
| value: String(availableTimeId), | ||
| label: display, | ||
| })), | ||
| [availableStudyTimes], | ||
| ); | ||
|
|
||
| const techOptions = useMemo( | ||
| () => | ||
| techStacks.map(({ techStackId, techStackName }) => ({ | ||
| value: String(techStackId), | ||
| label: techStackName, | ||
| })), | ||
| [techStacks], | ||
| ); |
There was a problem hiding this comment.
불필요한 배열을 만들지 않기 위한 렌더 최적화을 위해서 사용했다고 보시면 됩니다.
subjectOptions를 예로 들자면 현재 코드는 studySubjects가 변경될 때만 새 배열을 만들어서 옵션을 업데이트 하고,
바뀌지 않았다면 이전 옵션값을 재사용합니다.
이 값들이 대부분 드롭다운이나 토글 셀렉트(토글 버튼 여러개 선택) 같은 컴포넌트에 props로 들어가는 값이다보니 매 렌더마다 map으로 새 배열을 만들면 참조가 달라져서 불필요한 리렌더가 발생할 수 있다고 생각했습니다.
위 새 값들은 데이터가 바뀌어서 렌더가 필요한 값도 아니고, 동일한 데이터에서 변경될 확률이 낮기 때문에 배열을 재사용해서 리렌더를 줄이는 방법을 사용했습니다.
- 스터디 현재 상태 api 추가 - 해당 api 값에 따른 신청 목록/매칭 목록 UI 변경
| else field.onChange(arg); | ||
| }); | ||
|
|
||
| injected = cloneElement(child, { |
There was a problem hiding this comment.
여기서 cloneElement를 쓰신 이유가 있을까요?
공식문서에서 이 함수가 불안정한 코드를 만들 수 있다고 봤어서요!
There was a problem hiding this comment.
아하 넵. 자식 컴포넌트 props를 모른 채 넣어 문제가 말생하거나 불필요한 리렌더가 발생할 수 있다고 하네요.
이 부분은 수정하겠습니다.
다만, 해당 pr에서 오류가 발생하지 않는다면 일단 머지하고, 이후 바로 따로 브랜치를 파서 해당 브랜치에서
- cloneElement 변경
- 주차 & 데이트 셀렉트
- 기타 오류 등등...
을 변경해서 다시 머지해도 될까요?
일단 어느정도 오류가 발견되더라도 QA 서버에 올려둬야 할 것 같아서...ㅜㅜㅜ
오류 관련 브랜치를 따로 파서 바로 작업하는 게 나을 것 같습니다.
| maxLength?: number; | ||
| onChange?: (value: string) => void; | ||
| onChange?: (e: React.ChangeEvent<HTMLTextAreaElement>) => void; | ||
| hideMeta?: boolean; |
There was a problem hiding this comment.
이게 정말 길고 긴 여정이 있었는데요.......;-;
저희 피그마에 보시면 글자수를 제한한 input 일 경우 0/500 처럼 글자수만 있는 경우도 있지만,
그와 같은 가로 선상에 해당 input에 대한 설명이나 에러 메세지를 띄우는 경우가 있습니다...
다른 입력 폼 같은 경우에도 동일하게 description을 작성하는 부분이 있는데,
이걸 위해 text-input 에서 폼 필드로 글자수를 매번 바뀔때마다 전달하기는 아니라는 생각이 들어서
이 값을 이용해 글자수 표시와 description을 같은 선상에 표시할 수 있도록 했습니다..
| canAddMore | ||
| ? 'IT, Back-end, AI' | ||
| : `최대 ${maxSelectable}개까지 선택 가능합니다` | ||
| } |
There was a problem hiding this comment.
우선순위는 낮지만, 나중에라도 이 placeholder를 그냥 props로 받도록 리팩토링하는 건 어떨까요?
shared 계층에 있는거라 여러 곳에서 사용할 걸 대비해서요!
There was a problem hiding this comment.
넵 좋습니다. 저도 고민했던 부분인데 사실 지금 급한 건 아니라 생각하며 넘겼던 부분이라...;-;
이 부분은 지금 pr이 아니더라도 나중에 수정해서 올리겠습니다!
| caseSensitive = false, | ||
| allowCustom = true, |
There was a problem hiding this comment.
이건 그냥 추가적인 오류 방지(?)를 위해서 넣어뒀습니다.
사용자가 입력하는 대로 추가가 되다보니, 영어로 입력할 시 위와 같은 방지를 해주지 않으면 it, It, IT가 각각 서버로 전송될 수 있다 생각되어 대 소문자 비교를 위해 추가했습니다.
| }); | ||
| } | ||
|
|
||
| export const useStudyStatus = () => { |
There was a problem hiding this comment.
저희 컨벤션에 끝에 Query를 붙이기로 했어요! (링크)
근데, 지금 생각해보면 뒤를 잘 보지 않아서, 이 컨벤션이 필요한가 싶기도 하네욥
There was a problem hiding this comment.
앗 잊어버렸습니다..
그래도 일단 컨벤션이니 이 부분 수정해서 pr 올리도록 하겠습니다!

수아님 죄송합니다...........form 이랑 연결되다보니 수정된 파일이 어마어마 하네요......
아직 신청 리스트는 api 하나가 나오지 않아 커밋하지 않았습니다.
일단 form 관련 커밋이 너무 많은 것 같아.......미리 올렸습니다.
리스트 관련 파일은 파일 두 개만 수정될 예정이라 나중에 올리고 다시 한 번 슬랙으로 연락 드리겠습니다.
-> 위와 같이 하려 했는데, yarn을 돌려보실 수 있다고 생각해 커밋하겠습니다. 수정 예정이라고 봐주시면 감사하겠습니다.
신청 리스트는 participation 폴더에, 나머지는 다 form 파일이라고 보시면 됩니다..
제 커밋 실수로 인해 어마어마한 pr을 드리게 되어 죄송합니다😱
🌱 연관된 이슈
QNRR-463☘️ 작업 내용