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-fe: 공고 페이지에서 사용할 useDashboardCreateForm훅 구현 #278

Merged
merged 10 commits into from
Aug 7, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 6, 2024

Original issue description

목적

공고 페이지 구현

작업 세부사항

  • 공고 페이지 구현

참고 사항

아래의 별표줄 밑에 요구사항 ID만 작성해주세요. Prefix 금지!


RECRUIT_ROUTE

closes #277

@github-actions github-actions bot added feature 새로운 기능 frontend 프론트엔드 labels Aug 6, 2024
@lurgi lurgi changed the title feat-fe: 공고 페이지 구현 feat-fe: 공고 페이지에서 사용할 useDashboardCreateForm훅 구현 Aug 7, 2024
@lurgi lurgi marked this pull request as ready for review August 7, 2024 05:02
Copy link
Contributor Author

github-actions bot commented Aug 7, 2024

1723006964.604829

@lurgi
Copy link
Contributor

lurgi commented Aug 7, 2024

코드가 더럽지만 일단.... ㅠㅜ

Copy link
Contributor

@seongjinme seongjinme left a comment

Choose a reason for hiding this comment

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

러기 정말 고생하셨어요! 소소한 부분에 대하여 코멘트 남겨드렸으니 체크 부탁드릴게요.

  • 훅/테스트 코드 내 일부 key값에 대한 변경 요청 (order_index -> orderIndex)
  • setQuestionxxxx 함수 내 index 기준값(3)에 대한 상수화 제안

Copy link
Contributor

Choose a reason for hiding this comment

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

API 스펙과 일치하도록 타입을 잘 정의해 주신 것 같습니다 👍

question: string;
choices: {
choice: string;
order_index: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 API 스펙 문서에서 key값이 모두 camelCase로 변경되었어요. 이 부분도 반영해 주시면 감사하겠습니다!

Suggested change
order_index: number;
orderIndex: number;


export interface QuestionOption {
choice: string;
order_index: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

요 부분도 마찬가지입니다 :)

Suggested change
order_index: number;
orderIndex: number;


const setQuestionPrev = (index: number) => () => {
setApplyState((prevState) => {
if (index > 3) {
Copy link
Contributor

Choose a reason for hiding this comment

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

요 매직 넘버(3)는 훅 코드 상위나 다른 곳에 상수 처리하면 어떨까 제안드려 봅니다 🙏

Copy link
Contributor

@llqqssttyy llqqssttyy left a comment

Choose a reason for hiding this comment

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

엄청난 작업이었네요..
책임 분리에 대해서는 마감 후에 논의하고, 간단한 변경사항만 반영해주시고 다음 이슈로 넘어가시죠~

frontend/src/api/dashboard.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@seongjinme seongjinme left a comment

Choose a reason for hiding this comment

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

수정해주신 내용 모두 체크했습니다. 수고하셨어요!

@lurgi lurgi merged commit 8e257ca into fe/develop Aug 7, 2024
11 checks passed
@lurgi lurgi deleted the fe-277-RECRUIT_ROUTE branch August 7, 2024 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 새로운 기능 frontend 프론트엔드
Projects
Status: 완료
Development

Successfully merging this pull request may close these issues.

3 participants