Skip to content
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

[FE] issue53: 디테일 페이지 구현 #88

Merged
merged 19 commits into from
Jul 19, 2022
Merged

Conversation

airman5573
Copy link
Collaborator

요약

디테일 페이지를 구현했습니다.

세부사항

  1. 스터디 타이틀
  2. 모집중/모집완료 상태
  3. 모집기간
  4. 스터디 한줄 설명
  5. 스터디 태그
  6. 스터디 설명(markdown)
  7. 스터디에 참여중인 인원수
  8. 후기

등을 디테일 페이지에서 보여줍니다.

close #53

@airman5573 airman5573 added 🚀 feature New feature or request 😁 frontend New frontend feature labels Jul 19, 2022
@airman5573 airman5573 requested a review from nan-noo July 19, 2022 07:12
@airman5573 airman5573 self-assigned this Jul 19, 2022
'@constants': resolve(__dirname, '../src/constants.ts'),
'@api': resolve(__dirname, '../src/api'),
'@context': resolve(__dirname, '../src/context'),
'@detail-page': resolve(__dirname, '../src/pages/detail-page'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅇ여기에는 왜 main-page는 ㅇ없나요?!

그리고 pages가 있는데 그 내부 폴더들도 따로 만든 이유가 무엇인가요?

Comment on lines +31 to +32
<Route path="/" element={<MainPage />} />
<Route path="/study/:studyId" element={<DetailPage />} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외 페이지 처리도 해주면 좋을 것 같네요!
path="*" element={<ErrorPage />}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나중에 로그인/로그아웃 했을 때만 갈 수 있는 페이지도 처리합시당

import axiosInstance from './axiosInstance';

const getStudyDetail = async (studyId: string): Promise<{ study: StudyDetail }> => {
const response = await axiosInstance.get<{ study: StudyDetail }>(`/api/study/?study-id=${studyId}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

url 오류!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정 했슴다!

Comment on lines 10 to 12
const getStudyReviews = async (studyId: string, size: number, loadAll: boolean): Promise<StudyReviewResponse> => {
const url = loadAll ? `/api/studies/${studyId}/review` : `/api/studies/${studyId}/review?size=${size}`;
const response = await axiosInstance.get<StudyReviewResponse>(url);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size를 옵셔널로 해서 구현하는 게 더 낫지 않을까요?

  • size가 들어오면 일부, size가 없으면 전체

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋네요 !

Comment on lines +24 to +26
type DynamicImageContainerFn = (props: Pick<AvatarProps, 'size'>) => SerializedStyles;

const dynamicImageContainer: DynamicImageContainerFn = props => css`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타입 나누니까 훨씬 보기 좋네요!


const StudyReviewList: React.FC<{ reviews: Array<StudyReview> }> = ({ reviews }) => {
return (
<div css={css``}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 ??

currentMemberCount,
maxMemberCount,
owner,
onClickRegisterBtn,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

애는 왜 handleRegisterButtonClick이 아닌가요?

owner,
onClickRegisterBtn,
}) => {
const handleClickRegisterBtn = () => onClickRegisterBtn(studyId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네이밍 지켜주세요!

<S.StudyWideFloatBox>
<div className="left">
<div className="deadline">
<strong>{yyyymmddTommdd(deadline)}</strong>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 함수 네이밍이.. 흠 formatDate~~ 이런건 어떨까요

return useQuery<{ study: StudyDetail }, unknown>([QK_FETCH_STUDY_DETAIL, studyId], () => getStudyDetail(studyId));
};

export default useFetchDetail;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요건 재활용 목적이 아닌 로직 분리를 위한 커스텀 훅인가요?

@nan-noo nan-noo merged commit 291a0f9 into develop Jul 19, 2022
@nan-noo nan-noo linked an issue Jul 19, 2022 that may be closed by this pull request
@verus-j verus-j deleted the feat/53-detail-page-2 branch July 23, 2022 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 feature New feature or request 😁 frontend New frontend feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FE] 디테일 페이지 구현
2 participants