-
Notifications
You must be signed in to change notification settings - Fork 101
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(apply): implement the mission view page #763
Changes from all commits
5ef3804
7051503
dd5deca
fea2755
c6346a0
21d7d74
c1c0ce1
9dba94e
ea651a0
154df64
af2551c
d0537bc
c1ce22a
5be882f
731c5e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
"dependencies": { | ||
"axios": "^0.21.1", | ||
"classnames": "^2.3.1", | ||
"github-markdown-css": "^5.6.1", | ||
"highlight.js": "^11.10.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그런데 우아한테크코스에서 사용하는 |
||
"react": "^18.2.0", | ||
"react-datepicker": "^4.2.1", | ||
"react-dom": "^18.2.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,15 @@ const MyMissionItem = ({ mission, recruitmentId }: MyMissionItemProps) => { | |
); | ||
}; | ||
|
||
const routeToMissionView = () => { | ||
navigate( | ||
generatePath(PATH.MISSION_VIEW, { | ||
recruitmentId, | ||
missionId: String(mission.id), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a: missionId를 문자열로 변환하고 있는데, 이 부분이 꼭 필요한 것일까요~?(제가 코드를 잘 몰라서 하는 질문일수도 있습니다~!) 만약 API나 다른 코드에서 이 변환이 반드시 필요하다면 실수를 줄이기 위한 코드를 추가해 볼 수도 있을것 같아 보여요~! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추후 타입 정리도 리팩터링 작업 진행해보겠습니다~! |
||
}) | ||
); | ||
}; | ||
|
||
return ( | ||
<div className={classNames(styles["content-box"])}> | ||
<div className={styles["content-wrapper"]}> | ||
|
@@ -49,6 +58,15 @@ const MyMissionItem = ({ mission, recruitmentId }: MyMissionItemProps) => { | |
{missionItem.title} | ||
</RecruitmentDetail> | ||
<ul className={styles["title-button-list"]}> | ||
<li> | ||
<Button | ||
className={buttonStyles["assignment-button"]} | ||
disabled={missionItem.status !== MISSION_STATUS.SUBMITTING} | ||
onClick={routeToMissionView} | ||
> | ||
과제 보기 | ||
</Button> | ||
</li> | ||
<li> | ||
<Button | ||
className={buttonStyles["apply-button"]} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ export const PATH = { | |
SIGN_UP: "/sign-up", | ||
MY_APPLICATION: "/applications/me", | ||
ASSIGNMENT: "/assignment/:status", | ||
MISSION_VIEW: "/assignment/:recruitmentId/mission/:missionId", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a: 혹시 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. QA 진행 기간 중에 확인해 보고 코멘트 달아두겠습니다~! |
||
APPLICATION_FORM: "/application-forms/:status", | ||
LOGIN: "/login", | ||
FIND_PASSWORD: "/find", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ type MissionProps = { | |
recruitmentId: string; | ||
}; | ||
|
||
const missionLabel = (submitted: boolean, missionStatus: Mission["status"]) => { | ||
export const getMissionLabel = (submitted: boolean, missionStatus: Mission["status"]) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. a:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
const labelMap = { | ||
SUBMITTABLE: BUTTON_LABEL.BEFORE_SUBMIT, | ||
SUBMITTING: submitted ? BUTTON_LABEL.EDIT : BUTTON_LABEL.SUBMIT, | ||
|
@@ -22,7 +22,7 @@ const missionLabel = (submitted: boolean, missionStatus: Mission["status"]) => { | |
const useMission = ({ mission, recruitmentId }: MissionProps) => { | ||
const [missionItem, setMissionItem] = useState<Mission>({ ...mission }); | ||
|
||
const applyButtonLabel = missionLabel(mission.submitted, mission.status); | ||
const applyButtonLabel = getMissionLabel(mission.submitted, mission.status); | ||
|
||
useEffect(() => { | ||
setMissionItem(mission); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
.buttons { | ||
width: 100%; | ||
margin-top: 3.5rem; | ||
display: flex; | ||
justify-content: space-between; | ||
gap: 1rem; | ||
} | ||
|
||
.buttons li { | ||
flex: 1; | ||
} | ||
|
||
.buttons li > button { | ||
width: 100%; | ||
} | ||
|
||
.mission-viewer-body li { | ||
list-style: disc; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
import { generatePath, useNavigate, useParams } from "react-router-dom"; | ||
import Button, { BUTTON_VARIANT } from "../../components/@common/Button/Button"; | ||
import Container from "../../components/@common/Container/Container"; | ||
import useTokenContext from "../../hooks/useTokenContext"; | ||
import styles from "./MissionView.module.css"; | ||
import { PARAM, PATH } from "../../constants/path"; | ||
import { fetchMissionRequirements } from "../../api"; | ||
import { useEffect, useState } from "react"; | ||
import highlighter from "highlight.js"; | ||
import "github-markdown-css/github-markdown-light.css"; | ||
import "highlight.js/styles/github.css"; | ||
import { Mission } from "../../../types/domains/recruitments"; | ||
import { getMissionLabel } from "../../hooks/useMission"; | ||
import { MISSION_STATUS } from "../../constants/recruitment"; | ||
import { AxiosError } from "axios"; | ||
|
||
const MissionView = () => { | ||
const { token } = useTokenContext(); | ||
const navigate = useNavigate(); | ||
|
||
const { recruitmentId, missionId } = useParams<{ recruitmentId: string; missionId: string }>(); | ||
const [mission, setMission] = useState<Mission | null>(null); | ||
const description = mission?.description ?? ""; | ||
|
||
const isMissionSubmittable = | ||
!mission || | ||
mission?.submittable || | ||
mission?.status === MISSION_STATUS.SUBMITTABLE || | ||
mission?.status === MISSION_STATUS.SUBMITTING; | ||
|
||
const missionLabel = getMissionLabel( | ||
mission?.submitted ?? false, | ||
mission?.status ?? MISSION_STATUS.UNSUBMITTABLE | ||
); | ||
|
||
const goBack = () => { | ||
navigate(-1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. a:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 의도에 대해서는 구체적으로 논의된 적이 없어서요! |
||
}; | ||
|
||
const routeToAssignmentSubmit = () => { | ||
const isSubmitted = mission?.submitted; | ||
|
||
navigate( | ||
{ | ||
pathname: generatePath(PATH.ASSIGNMENT, { | ||
status: isSubmitted ? PARAM.ASSIGNMENT_STATUS.EDIT : PARAM.ASSIGNMENT_STATUS.NEW, | ||
}), | ||
}, | ||
{ | ||
state: { | ||
recruitmentId, | ||
currentMission: mission, | ||
}, | ||
} | ||
); | ||
}; | ||
|
||
const fetchRequirement = async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c: 데이터를 불러오는 로직이나 네비게이션 로직을 별도의 커스텀 훅으로 분리해보는건 어떨까요~? 재사용할 계획이 없다면, 분리하지 않을 수 있겠으나, 현재는 컴포넌트 네임의 역할을 확인하는데 한 곳에서 많은 코드를 봐야하는것 같아 제안드려 봅니다~! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 공원의 리뷰와 비슷한 관점에서 말씀해 주신 것 같아요! 이 부분은 앞으로의 리팩터링 작업에서 꼭 개선해 나가야 할 방향이 아닐까 싶어요~! 그때까지는 현재의 구조를 유지하면서 점진적으로 개선해 보겠습니다~! |
||
if (!recruitmentId || !missionId) { | ||
return; | ||
} | ||
|
||
try { | ||
const response = await fetchMissionRequirements({ | ||
token, | ||
recruitmentId: parseInt(recruitmentId, 10), | ||
missionId: parseInt(missionId, 10), | ||
}); | ||
|
||
setMission(response?.data); | ||
} catch (error) { | ||
alert((error as AxiosError).response?.data.message); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c: AxiosError인지 등은 UI Component가 신경쓰지 않아도 되는 부분이지 않을까요? api 레이어에 있는 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아, 맞아요! UI Component가 오류 형식을 받아서 메시지만 보여주는 게 가장 이상적인 구조라는 데 동의해요~ 지원플랫폼에서는 대부분 서버에서 받아온 오류 메시지를 그대로 표시하기보다는, 오류가 발생하면 클라이언트에서 정의한 메시지 상수를 보여주는 구조로 되어 있더라고요. 서버에서 받아온 오류 메시지를 직접 표현하는 처리는 몇 군데 없는 편이에요. 그런데 현재로서는 API 레이어에서 catch하는 구조가 아직 완벽하게 갖춰지지 않은 것 같아요! 이 부분은 앞으로의 리팩터링 작업에서 꼭 개선해 나가야 할 방향이 아닐까 싶어요~! 그때까지는 현재의 구조를 유지하면서 점진적으로 개선해 나가는 게 좋을 것 같아요 😊 |
||
|
||
goBack(); | ||
} | ||
}; | ||
|
||
useEffect(() => { | ||
if (!recruitmentId || !missionId) { | ||
goBack(); | ||
return; | ||
} | ||
|
||
fetchRequirement(); | ||
}, []); | ||
|
||
useEffect(() => { | ||
/* | ||
url - https://highlightjs.org/ | ||
<pre><code> 태그 내의 코드를 자동으로 감지하여 하이라이팅합니다. | ||
자동 감지가 실패할 경우, class 속성에 언어를 명시적으로 지정할 수 있습니다. | ||
*/ | ||
highlighter.highlightAll(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c: highlight 대상은 어떻게 찾나요 ?-? 라이브러리 문서에 아래와 같이 안내되고 있긴 하군요 🤔
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 주석에 간단히 남겨둘게요~ |
||
}, [description]); | ||
|
||
return ( | ||
<Container> | ||
<div | ||
className={`${styles["mission-viewer-body"]} markdown-body`} | ||
dangerouslySetInnerHTML={{ __html: description }} | ||
/> | ||
<ul className={styles.buttons}> | ||
<li> | ||
<Button type="button" variant={BUTTON_VARIANT.OUTLINED} onClick={goBack}> | ||
뒤로가기 | ||
</Button> | ||
</li> | ||
<li> | ||
<Button type="button" onClick={routeToAssignmentSubmit} disabled={!isMissionSubmittable}> | ||
{missionLabel} | ||
</Button> | ||
</li> | ||
</ul> | ||
</Container> | ||
); | ||
}; | ||
|
||
export default MissionView; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c: page 컴포넌트들은 Storybook도 같이 넣어두고 있었는데요. 이후 테스트 보강을 같이 해두면 좋을 것 같습니다. |
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.
기존 과제 내용이 Github 저장소 README.md의 스타일링을 고려하여 작성한 내용이다 보니, 그대로 스타일을 가져오는 것이 콘텐츠 제공자와 사용자 모두의 입장에서 좋을 것이라고 생각했어요.
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.
c: 해당 라이브러리를 선택하신 이유가 있을까요?
저도 정확히 같은 기능을 구현한 적은 없어서 리뷰하며 잠시 찾아보았는데, 아래와 같이 React 컴포넌트 형식으로, 코드 하이라이팅을 함께 지원하는 라이브러리도 있길래
해당 라이브러리를 최종 결정하신 이유도 궁금합니다.
https://www.npmjs.com/package/@uiw/react-markdown-preview
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.
라이브러리를 선택한 배경에 관해 궁금해하시는 것 같습니다!
서버에서는 DB에 마크다운 형식으로 가지고 있지만, 클라이언트에 전달할 때는 html로 변환해서 던져주게 됩니다.
그래서 이 상황에 적합한 라이브러리는
github-markdown-css
라고 판단했습니다 🙂