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

refactor(apply): redesign components of the my application page #761

Merged
merged 39 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
bc579f7
refactor: 과제보기 컴포넌트 스타일 코드 개선 작업
woowahan-cron Jun 25, 2024
86676d0
refactor: 과제보기 컴포넌트 새로고침 버튼 개선 작업
woowahan-cron Jul 2, 2024
4c7e910
refactor: 새로고침 버튼 역할 분리
woowahan-cron Jul 2, 2024
5480ba6
refactor: JudgmentButton 훅 분리 작업
woowahan-cron Jul 2, 2024
2cb8366
refactor: 예제 테스트 실행 역할 분리
woowahan-cron Jul 2, 2024
6f417f9
refactor: MyMissionItem 훅 분리
woowahan-cron Jul 2, 2024
d218f86
refactor: Apply, Refresh, Judgment 버튼 삭제 및 버튼(Button) 컴포넌트 사용
woowahan-cron Jul 2, 2024
604377f
refactor: 조건문 수정
woowahan-cron Jul 2, 2024
b1ed5fb
refactor: 새로고침 가능여부 조건
woowahan-cron Jul 2, 2024
989ea2f
chore: 명칭 통일을 위해 파일 이름 수정(useMissionJudgement -> useMissionJudgment)
woowahan-cron Jul 3, 2024
a8041dd
test: useRefresh 훅 테스트
woowahan-cron Jul 3, 2024
15eb864
test: useJudgment 훅 테스트
woowahan-cron Jul 3, 2024
a2e5ab5
test: useMission 훅 테스트
woowahan-cron Jul 3, 2024
0c1a2ad
refactor: useJudgment 코드 리뷰 반영하여 리팩터링
woowahan-cron Jul 8, 2024
c53dd81
refactor: useMission.test 코드 리뷰 반영하여 리팩터링
woowahan-cron Jul 8, 2024
4eeaa4b
refactor: useRefresh.test 코드 리팩터링
woowahan-cron Jul 8, 2024
6cafd49
refactor: 테스트 코드 리팩터링
woowahan-cron Jul 8, 2024
85349b1
refactor: useMission 훅 조건부 리팩터링
woowahan-cron Jul 8, 2024
680bd89
refactor: useRefresh 조건부 리팩터링
woowahan-cron Jul 9, 2024
407308a
refactor: useRefresh 헬퍼 함수 리팩터링
woowahan-cron Jul 9, 2024
101f1f8
refactor: 테스트 코드 공통 함수
woowahan-cron Jul 9, 2024
3d1048f
refactor: useJudgment 공통 함수 호출
woowahan-cron Jul 9, 2024
2221fdf
refactor: useMission 테스트 공통 함수 호출
woowahan-cron Jul 9, 2024
f01dfd7
refactor: useRefresh 테스트 공통 함수 호출
woowahan-cron Jul 9, 2024
a9fea3b
test: MyMissionItem 컴포넌트 테스트
woowahan-cron Jul 9, 2024
f38c0da
refactor: useMissionJudgment 개행 제거
woowahan-cron Jul 11, 2024
e8914a5
refactor: Button 컴포넌트 선언부 수정
woowahan-cron Jul 11, 2024
28a44c0
refactor: MissionDetail 컴포넌트 속성 수정 (관심사 분리 작업)
woowahan-cron Jul 11, 2024
24437bc
refactor: 하위 컴포넌트 계층 이동 (종속 관계)
woowahan-cron Jul 11, 2024
0a01049
refactor: MyApplicationItem module 스타일 일관성 맞추기 작업
woowahan-cron Jul 15, 2024
f1517c3
refactor: 응답 값 Judgment 추론
woowahan-cron Jul 15, 2024
bc9d6d7
refactor: 버튼 이름 상수화 작업
woowahan-cron Jul 15, 2024
20924fd
refactor: 판정 -> 채점 용어 수정 작업
woowahan-cron Jul 15, 2024
04bcc21
refactor: 테스트 명세 문구 수정
woowahan-cron Jul 15, 2024
b4277b4
refactor: useMission 의존성 분리 작업
woowahan-cron Jul 15, 2024
f84fe3a
test: useMission 훅 의존성 분리 작업 후 테스트 개선
woowahan-cron Jul 15, 2024
5e46853
refactor: MyMissionItem 컴포넌트 테스트
woowahan-cron Jul 16, 2024
c7213d2
refactor: useMission 훅 리팩터링
woowahan-cron Jul 24, 2024
acd9682
rename: hook 파일 이동
woowahan-cron Jul 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/src/components/@common/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ export const BUTTON_VARIANT = {
export type ButtonProps = React.ButtonHTMLAttributes<HTMLButtonElement> & {
variant?: typeof BUTTON_VARIANT[keyof typeof BUTTON_VARIANT];
cancel?: boolean;
disabled?: boolean;
};

const Button = ({
className,
variant = BUTTON_VARIANT.CONTAINED,
cancel = false,
disabled = false,
children,
...props
}: ButtonProps) => {
Expand All @@ -23,6 +25,7 @@ const Button = ({
className={classNames(className, styles[variant], styles.button, {
[styles.cancel]: cancel,
})}
disabled={disabled}
Copy link
Contributor

Choose a reason for hiding this comment

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

a: React.ButtonHTMLAttributes<HTMLButtonElement>가 이미 disabled를 포함하고 있는 것으로 알고 있는데요! 혹시 더 명시적으로 표현해주기 위한 수정 사항일까요? 🙂

Copy link
Author

Choose a reason for hiding this comment

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

네넵! 버튼을 커스텀해서 사용하고 있어서 명시적으로 표현하기 위해서였는데 ButtonProps 선언부에서는 굳이 disabled 속성을 표기해 줄 필요가 없겠군요! 그 부분만 적용해서 반영해보겠습니다.

{...props}
>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,27 @@
word-break: break-all;
}

.detail-status-container {
display: flex;
justify-content: space-between;
}

.detail-status-container ul {
display: flex;
gap: 0.25rem;
}

.guide-container {
display: flex;
gap: 0.25rem;
margin-left: auto;
color: var(--gray-600);
font-size: 0.815rem;
}

@media screen and (max-width: 800px) {
.title-container {
flex-direction: column;
gap: 0.875rem;
}
}
Copy link
Author

Choose a reason for hiding this comment

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

나중에 과제보기 버튼이 추가되면 우측에 버튼이 배치되어 있는 경우 가독성이 떨어지는 것을 대비하여
폭이 좁아질 경우 (800px) 기준으로 flex-directionrow -> column로 변경하여 대응했습니다.
이것이 적절한지도 검토 부탁드려요!

(실제로 지금 과제보기 버튼이 추가되어 있는 상태는 아니고 크롬 개발자 도구에서 버튼을 복사하여 배치한 후 캡처한 화면입니다)

800px 이상의 화면

image

800px 미만의 화면

image

Copy link
Contributor

Choose a reason for hiding this comment

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

좋습니당 :)
바꿔주신 걸 보니 버튼이 아래로 내려오면 '테스트 코드 실행이 끝나기까지 3~5분이 걸릴 수 있습니다'는 왼쪽 정렬로 해두어도 좋을 것 같네용ㅋㅋ

Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
import { PropsWithChildren } from "react";
import { Mission } from "../../../../types/domains/recruitments";
import { MY_MISSION_TOOLTIP_MESSAGE } from "../../../constants/messages";
import Tooltip from "../../@common/Tooltip/Tooltip";
import CommitHash from "../CommitHash/CommitHash";
import JudgmentResultText from "../JudgmentResult/JudgmentResult";
import styles from "./MissionDetail.module.css";

type MissionDetailProps = {
type MissionDetailProps = PropsWithChildren<{
judgment: Mission["judgment"];
};
}>;

const MissionDetail = ({ judgment }: MissionDetailProps) => {
const MissionDetail = ({ judgment, children }: MissionDetailProps) => {
return (
<div className={styles["detail-container"]}>
<JudgmentResultText judgment={judgment} />
<div className={styles["detail-status-container"]}>
<JudgmentResultText judgment={judgment} />
<div>{children}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

r: <MissionDetail />은 현재 <MyMissionItem />에서만 한 번 사용되는 것으로 보이는데요! 혹시 children으로 받는 부분이 이후에 커스텀을 대비한 요소일까요~? 지금 시점에서는 오히려 children으로 유연하게 열어두기보단<MyMissionItem /> 내부에 추가하는 편이 <MissionDetail />을 쓰는 쪽에서도 신경 쓸 게 덜하지 않나 하는 생각이 들어 여쭤봅니당 :)

Copy link
Author

Choose a reason for hiding this comment

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

앗 그러네요! 훅으로 분리했기 때문에 오히려 지금 children으로 넘겨 둔 Element는 MissionDetail 컴포넌트 이하로 옮겨두는 것이 더 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

그리고 말씀하신 것처럼 지금은 MyMissionItem에서 한 번만 사용되고 있기 때문에 MyApplicationItem 이하 같은 레벨의 컴포넌트로 구성되어 있는데 이것을 MyMissionItem 컴포넌트 하위로 옮겨두는 것이 더 좋을 것 같다는 생각이 듭니다!

  • MyApplicationItem 컴포넌트
    • MissionDetail (현재 위치)
    • MyMissionItem
      • MissionDetail(수정 후 위치)

이전에 훅은 hooks/에 모여 있으니 특정 컴포넌트에서만 사용되는 훅은 다음번 PR에서 옮기는 것을 제안해 주셨는데 컴포넌트는 관계가 훅보다는 복잡한 것 같고 나중에 한꺼번에 작업하기 부담이 될 것 같다는 생각이 들었어요!

  • 의존하고 있는 스타일이 있습니다. (ApplicationButtonStyles) -> 분리해도 되지만, 분리할 경우 동일한 속성 코드 선언의 반복으로 차후 수정 작업 시 일관성을 지키기 어렵겠다는 느낌이 드네요!

</div>
<CommitHash judgment={judgment} />
<div className={styles["guide-container"]}>
<p>테스트 코드 실행이 끝나기까지 3 ~ 5분이 걸릴 수 있습니다</p>
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import { Recruitment, RecruitmentStatus } from "../../../../types/domains/recrui
import { PARAM, PATH } from "../../../constants/path";
import { formatDateTime } from "../../../utils/format/date";
import { generateQuery } from "../../../utils/route/query";
import ApplyButton from "../MyApplicationButtons/ApplyButton";
import styles from "../MyApplicationItem.module.css";
import RecruitmentDetail from "../RecruitmentDetail/RecruitmentDetail";
import { BUTTON_LABEL, RECRUITMENT_STATUS } from "./../../../constants/recruitment";
import Button from "../../@common/Button/Button";

type MyApplicationFormItemProps = {
recruitment: Recruitment;
Expand Down Expand Up @@ -68,19 +68,24 @@ const MyApplicationFormItem = ({ recruitment, submitted }: MyApplicationFormItem

return (
<div className={classNames(styles["content-box"])}>
<div className={styles["text-container"]}>
<RecruitmentDetail startDate={formattedStartDateTime} endDate={formattedEndDateTime}>
{recruitment.title}
</RecruitmentDetail>
<div className={styles["content-wrapper"]}>
<div className={styles["title-container"]}>
<RecruitmentDetail startDate={formattedStartDateTime} endDate={formattedEndDateTime}>
{recruitment.title}
</RecruitmentDetail>

<div className={styles["button-container"]}>
<ApplyButton
isButtonDisabled={isButtonDisabled}
onClick={() => {
routeToApplicationForm(recruitment);
}}
label={applyButtonLabel}
/>
<ul className={styles["title-button-list"]}>
<li>
<Button
disabled={isButtonDisabled}
onClick={() => {
routeToApplicationForm(recruitment);
}}
>
{applyButtonLabel}
</Button>
</li>
</ul>
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 이후에 추가될 것을 대비한 ul일까용? 🙂

Copy link
Author

Choose a reason for hiding this comment

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

넵 맞습니다! 바로 다음 이슈에서 과제 보기 버튼을 추가할 예정이에요 ㅎㅎ

Copy link
Contributor

Choose a reason for hiding this comment

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

아하 해당 UI가 추가될 때 같이 추가해주면 더 좋을 것 같습니다 :) 이 PR에서 다루는 이슈에서는 필요하지 않을 요소인 것 같아서요~! 🙂

</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.divider {
border: 0.5px solid var(--gray-300);
}

.content-box {
position: relative;
display: flex;
Expand All @@ -11,29 +15,41 @@
border: 1px solid var(--gray-300);
}

.text-container {
.content-wrapper {
display: flex;
flex-direction: column;
gap: 0.875rem;
width: 100%;
}

.auto-judgment-detail-contour {
border: 0.5px solid var(--gray-300);
.title-container {
display: flex;
justify-content: space-between;
}

.title-text {
flex: 1;
}

.button-container {
position: absolute;
top: 1.25rem;
right: 1.5rem;
.title-button-list {
display: flex;
gap: 1rem;
}

.title-button-list li:not(:last-child) {
margin-right: 0.4rem;
}

@media screen and (max-width: 800px) {
.button-container {
position: static;
gap: 0.4rem;
margin-left: auto;
.title-container {
flex-direction: column;
gap: 0.875rem;
}

.title-button-list > li {
Copy link
Contributor

Choose a reason for hiding this comment

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

a: 혹시나 마크업을 수정해서 .title-button-list 바로 하위에 li 태그가 있지 않게 되면 스타일이 적용되지 않을 수도 있을 것 같아요. 지금 문제될 부분은 아니라고 생각하지만 위의 .title-button-list li:not(:last-child) 에서는 child combinator 대신 descendant selector를 써주셨길래 일관성 측면에서 맞춰도 좋을 것 같아 의견으로만 남겨둡니당 :) 선택자 우선순위 등의 문제로 넣으신거라면 그것도 좋고요!

Copy link
Author

Choose a reason for hiding this comment

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

오! 좋습니다 ㅎㅎ
헷갈릴 수도 있으니 일관성 맞추고 어떤 클래스에서 쓰이는 것인지도 분명하게 표시해 두면 좋겠다고 생각해서 반영해두겠습니다.

flex: 1;
}

.title-button-list > li > button {
width: 100%;
}
}
Loading