-
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
[FE] feature: 이름, 프로젝트명에 대한 유효성 검사 및 에러 메세지 안내 추가 #351
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.
수고했어요 올리!
useEffect(() => { | ||
isWithinMaxLength(revieweeName, MAX_VALID_REVIEW_GROUP_DATA_INPUT) | ||
? setRevieweeNameErrorMessage('') | ||
: setRevieweeNameErrorMessage(LENGTH_ERROR_MESSAGE); | ||
}, [revieweeName]); | ||
|
||
useEffect(() => { | ||
isWithinMaxLength(projectName, MAX_VALID_REVIEW_GROUP_DATA_INPUT) | ||
? setProjectNameErrorMessage('') | ||
: setProjectNameErrorMessage(LENGTH_ERROR_MESSAGE); | ||
}, [projectName]); |
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.
2개의 useEffect
로 나눠서 관리하는 거 좋은 것 같네요!
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.
유효하지 않은 확인코드(추후에 비밀번호로 바뀔 것)은 영문(대/소문자),숫자 에러문구를 보여주고 프로젝트명과 리뷰이 이름은 문자 개수 오류만 보여주네요
(pr에서 문자 개수 오류만 보여준다는 부분은 프로젝트명과 리뷰이 이름에 대한 거겠죠?)
추후에 바뀌는 비밀번호가 4~20자 여야해서 이 부분도 나중에 반영하면 되겠네요
🚀 approve 합니다 😆
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.
코드 리팩토링도 함께 진행해서 깔끔해졌네요👍
폼에 비밀번호 input이 추가된 뒤에 고민해보면 좋을 것 같아서 간단한 코멘트 하나 남겼어요.
고생했어요
// TODO: 디바운스 시간을 모든 경우에 0.3초로 고정할 것인지(전역 상수로 사용) 논의하기 | ||
const DEBOUNCE_TIME = 300; | ||
const MAX_VALID_REVIEW_GROUP_DATA_INPUT = 50; |
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.
컴포넌트 내부에 상수로 최댓값을 갖고 있는 것도 좋지만, prop로 받아서 조금 더 유연하게 처리하는 방법도 추후 고민해볼 수 있을 것 같아요!
🚀 어떤 기능을 구현했나요 ?
Input에 새롭게 추가된 유효성 검사 함수 적용
버튼을 활성화시키는 조건은 한꺼번에(예: 확인 코드의 경우 공백 + 최대 글자수 + 영문 및 숫자로만 이루어졌는지 검사) 확인합니다.
단, 유저에게 Input 하단에 안내하는 에러 문구는 글자수 제한만 알려줍니다.
이렇게 개별 유효성 검사(빈 값 단독, 길이 단독 등)를 수행해야 할 때가 있으므로 사용처에서 여러 함수를 import 해옵니다.
테스트 편리성을 위해 검증 함수를 나눈 것도 있습니다.
이때 Input 컴포넌트에서 max 속성을 통해 max 이상으로 입력받을 수 없게끔 제한할 필요가 있을까? 라는 의문이 들었습니다.
=> 프로젝트 이름 등을 복붙해왔을 때 잘리는 것보다 50자 안내를 띄우고 알아서 조정할 수 있게끔 하는 게 나을 것 같다.
현재는 입력이 비어 있거나 max값을 넘으면 버튼이 비활성화되고, 에러 문구는 최대 글자 수에 대해서만 안내합니다.
확인 코드 최대 62자 제한 추가
-> 따라서 사용자에게 최대 62자임을 알려줄 필요가 없다고 생각해서 별도의 안내 문구는 띄우지 않았습니다.
(하지만...곧 없어질 확인 코드입니다...)
🔥 어떻게 해결했나요 ?
📝 어떤 부분에 집중해서 리뷰해야 할까요?
📚 참고 자료, 할 말