-
Notifications
You must be signed in to change notification settings - Fork 0
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(member, time): 관리자 페이지 계정 추가 기능 #232
Conversation
…to feature/228
🦋 Changeset detectedLatest commit: 89d89d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this 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.
현재 저도 SelectOptions
사용하는 부분 없는 것으로 확인됩니다.
프로젝트 볼륨이 더 커지기 전에 디렉토링 방식을 수정하는 부분에 동의해요. 활동 페이지나 도서 추가 등 지금 진행되고 있는 멤버스 개발이 마무리 된 후 진행하면 좋을 것 같아요 :)
const [id, setId] = useState<string>(''); | ||
const [name, setName] = useState<string>(''); | ||
const [address, setAddress] = useState<string>(''); | ||
const [email, setEmail] = useState<string>(''); | ||
const [phoneNumber, setPhoneNumber] = useState<string>(''); | ||
const [major, setMajor] = useState<string>(''); | ||
const [grade, setGrade] = useState<number>(1); | ||
const [birth, setBirth] = useState<string>(''); | ||
const [interest, setInterest] = useState<string>('NULL'); | ||
const [githubUrl, setGithubUrl] = useState<string>(''); | ||
const [studentStatus, setStudentStatus] = useState<string>('CURRENT'); |
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.
useState
하나로 관리하는 더 편하지 않을까 싶어요
apps/member/src/types/manage.ts
Outdated
export interface AddMemberRequestType { | ||
id: string; | ||
password: string; | ||
name: string; | ||
email: string; | ||
contact: string; | ||
department: string; | ||
grade: number; | ||
birth: string; | ||
address: string; | ||
interests: string; | ||
githubUrl: string; | ||
studentStatus: string; | ||
imageUrl: 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.
password
와 imageUrl
은 필수값이 아니라서 해당 부분은 optional
으로 수정해도 좋을 것 같아요.
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.
LGTM!!
@SWARVY build CI 실패 로그 확인해주세요 |
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.
수고하셨습니다! CI 문제 해결되고 머지 해주시면 될 것 같아요!
Summary
관리페이지에 멤버 추가 기능을 구현합니다
Tasks
useSuspenseQuery
를 통해 멤버 관리 섹션에서 멤버 목록을 불러오고 있었으나,Suspense
컴포너트가 적용되어있지 않아 컴포넌트가 데이터 패칭 중 블링킹 하는 현상을 수정했습니다.ETC
이번에 생성한
AddMemberForm
의 경우 Section 디렉토리 내부에 넣어놓았습니다. 그 이유는 해당 폼 컴포넌트가 다른 페이지에서 재사용되지 않으며 현행 디자인대로 다른 디렉토리로 분리하여 관리할 경우 불필요한 코스트가 들 것이라고 생각했기 떄문입니다.이와 더불어, 현재 디렉토링 방식에 대하여 전체적으로 분석해본 결과, 재사용되지 않는 컴포넌트들이 불필요하게 디렉토링 되어있다고 느껴집니다.
대표적으로 관리페이지에서 사용되는
AddScheduleForm
입니다. 일정의 경우 관리탭에서 추가 / 삭제를 담당하고있기 때문에, 다른 페이지에서는 사용되지 않는 컴포넌트입니다. 하지만,manage
디렉토리에 존재하는것이 아니고schedule
디렉토리에 존재하기떄문에 컴포넌트를 찾기 어렵습니다. (그냥 ctrl-f 혹은 ctrl-shift-f로 찾으면 되지만 모두 그렇게 해결할거라면 디렉토링은 중요하지 않으니까요)따라서, 지금 현행 디렉토링 방식을 재사용성의 유무에 따라 재분류 할 것을 건의드립니다. 멤버스의 기능이 점점 커지고 있기때문에 지금도 코스트가 상당량 들어갈 것으로 예상되지만 지금 못한다면 이후에는 볼륨이 더 늘어나서 더 힘들어질 것으로 보입니다.
또,
select.ts
에 존재하는SelectOptions
타입을 임시 비활성화 하였습니다.해당 타입을 임시적으로 비활성화 한 이유는 다음과 같습니다.
key: string
이라는 모호한 타입을 사용하고있어SELECT_OPTIONS
객체를 참조했을 때 인텔리센스가 정상적으로 작동하지 않음.SELECT_OPTIONS
객체에서만 해당 타입을 사용하고있고, 제거해도 정상 동작에 문제 없음혹시 다른 부분에서 사용되고있다면 말씀해주세요, 해당 부분까지 고려하여 타입 자체를 재설계하는 방향으로 진행해보겠습니다.
그리고 책 추가는 바로 다음 작업으로 이어서 할게요 뀨잉
Screenshot