[FE] issue133, 173: 스터디 가입 기능 구현 및 스터디원에 스터디장 추가 #174
Conversation
… feat/133-join-study
frontend/src/api/postJoiningStudy.ts
Outdated
|
|
||
| const postNewStudy = async (studyId: number) => { | ||
| const response = await axiosInstance.post(`/api/studies/${studyId}`); | ||
| return response; |
There was a problem hiding this comment.
response.data를 return하는건 어떨까요?
body에 errorMessage가 들어있을 수 있으니까요.
There was a problem hiding this comment.
아 이거 postNewStudy랑 맞춘건데 둘 다 고치겠습니다ㅎㅎ
생각해보니 errorMessage가 있었네요
|
|
||
| transform-origin: top; | ||
| animation: slidein 0.1s ease; | ||
| @keyframes slidein { |
There was a problem hiding this comment.
아래로 스윽 내려오는거니까 slideDown이 어떨까요?
| @@ -18,12 +27,33 @@ import useFetchDetail from '@detail-page/hooks/useFetchDetail'; | |||
| const DetailPage = () => { | |||
| const { studyId } = useParams() as { studyId: string }; | |||
|
|
|||
| const { isLoggedIn } = useContext(LoginContext); | |||
There was a problem hiding this comment.
useAuth 훅이 이미 있으니, 이 훅을 조금 개선해서 사용하는건 어떨까요?
| const studyDetailQueryResult = useFetchDetail(Number(studyId)); | ||
| const { mutate } = useMutation<AxiosResponse, Error, number>(postJoiningStudy); |
There was a problem hiding this comment.
useFetchDetail처럼 한번 감싸는 hook을 만드는건 어떨까요?
예를들면, usePostJoiningStudy 이런 식으로요.
import문도 줄이고, 코드도 더 부드럽게 읽힐것 같아서요 :D
There was a problem hiding this comment.
이건 나중에 같이 리팩토링할 때 얘기하면 좋을 것 같아서 남겨놨습니다. useQuery나 useMutation 한 줄만 커스텀 훅으로 감싼 경우가 많은데 해당 페이지에서 필요한 로직들도 같이 넣어두면 더 좋을 것 같아서요! 이왕 묶을거라면 아예 useDetailPage 훅으로 묶는 게 좋을 것 같아요.
| }; | ||
|
|
||
| if (!studyId) { |
There was a problem hiding this comment.
이 로직이 login상태 체크보다 위로 보내는건 어떨까요?
There was a problem hiding this comment.
const { isLoggedIn } = useContext(LoginContext);
이거보다 위에를 말하는 건가요??
const handleRegisterBtnClick = () => {
if (!isLoggedIn) {
alert('로그인이 필요합니다.');
return;
}
...이 로직은 버튼 클릭 핸들러라서 스터디 아이디가 우선 확인되긴 합니다!
There was a problem hiding this comment.
아 제가 depth를 잘못 봤네요.
handleRegisterBtnClick안에 if (!studyId) {가 있는줄 알았습니다 :D
| top: 5px; | ||
| left: 20px; | ||
| stroke: ${theme.colors.tertiary.base}; | ||
| fill: ${theme.colors.tertiary.base}; |
There was a problem hiding this comment.
내부 색을 노란색으로 채우려고 했습니다 :D
| /> | ||
| </a> | ||
| </S.Owner> | ||
| {members.slice(0, DEFAULT_VISIBLE_STUDY_MEMBER_CARD_COUNT - 1).map(({ id, username, imageUrl, profileUrl }) => ( |
There was a problem hiding this comment.
DEFAULT_VISIBLE_STUDY_MEMBER_CARD_COUNT - 1 1을 뺀 이유가 있을까요?
There was a problem hiding this comment.
스터디장을 포함해서 (더보기 누르기 전) 6명을 보여줘야하기 때문에, 스터디원은 스터디장을 제외한 5명만 필요합니다.
There was a problem hiding this comment.
그렇군요!
스터디장이 항상 마지막에 있는걸 어떻게 확신하나요?
There was a problem hiding this comment.
스터디장은 스터디원 섹션에서 가장 첫 번째에 있습니다! (S.Owner 컴포넌트가 스터디장입니다.)
스터디장과 나머지 스터디원은 다른 props로 받고 있기 때문에 스터디원과 스터디장의 순서는 상관이 없습니다
| useEffect(() => { | ||
| mutate(codeParam); | ||
| }, []); | ||
| const { mutate } = useMutation<TokenQueryData, Error, string>(getAccessToken); |
There was a problem hiding this comment.
이것도 hook으로 감싸면 좋을것 같아요 :D
| } | ||
| mutate(codeParam, { | ||
| onError: error => { | ||
| alert(error.message); |
There was a problem hiding this comment.
error.message가 있는지 확인하는 과정이 있으면 좋을것 같아요 :D
| export const COLORS = { | ||
| YELLOW200: '#FFD54F', |
There was a problem hiding this comment.
100, 300도 추가하려다 나중에 추가하려고 남겨두었습니다 :D
요약
스터디 상세 페이지에서 스터디 가입 기능 구현 및 스터디원에 스터디장 추가
세부사항
close #133 #173