-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix/igw 64/127 고용주필수입력사항적용 #156
The head ref may contain hidden characters: "fix/IGW-64/127-\uACE0\uC6A9\uC8FC\uD544\uC218\uC785\uB825\uC0AC\uD56D\uC801\uC6A9"
Conversation
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.
수고하셨습니다~!!
src/components/Common/Input.tsx
Outdated
@@ -18,6 +18,7 @@ type InputProps = { | |||
placeholder: string; // 플레이스홀더 텍스트 | |||
value: string | null; // 입력 필드의 현재 값 | |||
onChange: (value: string) => void; // 입력값 변경 시 호출될 함수 | |||
onBlur?: (value: string) => void; // 입력 필드에서 포커스가 빠져나갈 때 호출될 함수 (선택적) |
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.
props가 점점 많아지는게 웃프네요😂
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.
안그래도 요즘 스터디에서 컴포넌트나 모듈 분리의 기준에 대한 내용을 진행했었는데, 적절하지 않은 예시가 딱 이 컴포넌트더라구요.. 🥲 좀 더 세부적인 레벨의 요소들로 분리해서 재조립해야하나 싶어요
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.
이 부분 한 번 얘기해보죠!!ㅎㅎㅎ 관련 스터디 내용도 나중에 알려주세용!😉
@@ -86,7 +88,7 @@ export const validateEmployerInformation = ( | |||
// 시급 유효성 검사(0이 아닌지) | |||
if ( | |||
!info.hourly_rate || | |||
extractNumbersAsNumber(String(info.hourly_rate)) === 0 | |||
extractNumbersAsNumber(String(info.hourly_rate)) < MINIMUM_HOURLY_RATE |
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.
상수화시키니까 너무 좋네요!
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.
저도 연도마다 시급이 변해야하다보니 상수화가 좋다고 생각했습니다~👍
src/utils/document.ts
Outdated
setWarning, | ||
}: { | ||
value: string; | ||
setWarning: React.Dispatch<React.SetStateAction<boolean>>; |
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.
제가 알기로 React.Dispatch 이렇게 바로 넘기는 것보다 (value:string) => void 형식의 함수를 넘기는 것이 더 안정적이다고 알고 있어요!
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.
이전엔 단순히 작성하기 쉬워서 순수 함수 타입을 넘긴다고 생각했는데, 가독성이나 결합도 측면에서도 확실히 더 좋은 방식이었네요. 수정하겠습니다~
Related issue 🛠
#127
Work Description ✏️
Uncompleted Tasks 😅
To Reviewers 📢