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

feat: 지원현황/면접기록 목록페이지에서 합불상태를 조회할 수 있다. #213

Merged
merged 13 commits into from
Sep 18, 2024

Conversation

2yunseong
Copy link
Collaborator

주요 변경사항

Screenshot 2024-09-18 at 20 32 08 Screenshot 2024-09-18 at 20 32 16

리뷰어에게...

  • 지원현황/면접기록 페이지가 동일한 컴포넌트를 공유하고 있어서, 편의상 같은 브랜치에서 작업한 점 양해부탁드립니다.
  • 기존 컴포넌트를 리팩토링하였습니다.
  • 합불상태를 보여주는 Txt 컴포넌트는 feat: 합/불 상태관리 페이지 추가 #205 를 참고해 만들었습니다. 추후 원하신다면 재사용가능한 컴포넌트로 두루 사용하게 해도 괜찮을듯합니다.

관련 이슈

체크리스트

  • reviewers 설정
  • label 설정
  • milestone 설정

@2yunseong 2yunseong added enhancement 기능개선 혹은 UI/UX개선 제안 feature 기능개발 labels Sep 18, 2024
@2yunseong 2yunseong self-assigned this Sep 18, 2024
Copy link
Collaborator

@geongyu09 geongyu09 left a comment

Choose a reason for hiding this comment

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

좋네요! 코드를 보면서 많이 배워갑니다.
명절동안 수고많으셨습니다!


const openModel = (id: string) => {
setIsOpen(true);
const handleModalOpen = (id: string) => () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오...! 커링으로 작성한 것은 처음 보네요..!
저는 이렇게 사용해본 경험이 없어서 그런데, 해당 로직을 이런 형식으로사용했을 때 얻을 수 있는 장점이 무엇이 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단은 현재 커링을 쓴 이유는 컴포넌트를 분리함에 따라 이벤트 핸들러를 넘겨줘야 하는데,
기존 코드를 유지하기 위해 사용하였습니다.

마찬가지로 특정 이벤트 핸들러를 상위 컴포넌트에서 전달해주는데 각 요소 별로 전달해야할 값들이 있을 때 유용하다고 생각합니다^^

말이 장황할 수 있는데 현재 PR 코드 보시는 느낌 그대로라고 생각하시면 될 거 같습니다 !

<Txt>검색결과가 없습니다.</Txt>
) : (
<>
{boardRows.map((item) => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 부분 item. 이 반복되는 것 같아, ({id, title, subElements, passState,}) => {} 형태로 작성해도 좋아보입니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋네요 ! 수정해보도록 하겠습니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

const Board = ({
children,
onClick,
wapperClassname,
wrapperClassname,
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrapperClassName으로 작성하는게 좋아보입니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오타 제보 감사용 .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return <div>loading...</div>;
}

const { records } = data;
Copy link
Collaborator

Choose a reason for hiding this comment

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

구조 분해를 하나의 데이터만 하는 것은 좋은 코드가 아니여서 이렇게 하신건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

음, 꼭 그런건 아니구요
굳이 사용할 필요가 없어보여서 사용하지 않았습니다!

import { PropsWithChildren } from "react";
import { type PropsWithChildren } from "react";
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 이런 방식은 처음 보네요 !
혹시 어떤 차이가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

type import 입니다
이 import는 타입임을 명시적으로 나타내주는 것인데요
typescript 컴파일 시점에 "완전히" 사라집니다.
따라서 사이트이펙트나 런타임 번들 크기 등에 이점을 가져갈 수 있슴다

Copy link
Collaborator

Choose a reason for hiding this comment

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

오 감사합니다 !!! 되게 신기하네요.

Copy link
Collaborator

@smb0123 smb0123 left a comment

Choose a reason for hiding this comment

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

추석 동안 작업하시느라 수고하셨습니다 !!
항상 많이 배우는 것 같아요

@2yunseong 2yunseong merged commit 36ca61c into main Sep 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 기능개선 혹은 UI/UX개선 제안 feature 기능개발
Projects
None yet
3 participants