-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/helper text constants로 빼기 #847 #856
The head ref may contain hidden characters: "feature/helper_text_constants\uB85C_\uBE7C\uAE30_#847"
Conversation
src/constants/helperText.ts
Outdated
onlyNumber: '숫자만 입력 가능합니다.', | ||
onlyHttps: 'https:// 로 시작해야 합니다.', | ||
underMinLen: (min: number) => `${min}글자 이상 입력해주세요.` as const, | ||
overMaxLen: (max: number) => `최대 ${max}글자 입력해주세요.` as const, |
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.
underMinLen
, overMaxLen
말고도 minLength
, maxLength
는 어떨지 의견남깁니당!!
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.
minLength만 했을 때, 에러인지 명확하지 않아서 under를 붙인거 였는데 이젠 error 객체안에 있으니 상관없을 듯 하네요... 좋은 의견인거 같습니다.
src/constants/helperText.ts
Outdated
onlyNumber: '숫자만 입력 가능합니다.', | ||
onlyHttps: 'https:// 로 시작해야 합니다.', | ||
underMinLen: (min: number) => `${min}글자 이상 입력해주세요.` as const, | ||
overMaxLen: (max: number) => `최대 ${max}글자 입력해주세요.` as const, |
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.
최대 글자수 ${max}글자를 초고했습니다. 로 수정하겠습니다!
src/pages/SignUp/SignUp.tsx
Outdated
const SignUp = () => { | ||
const TOTAL_STEPS = 3; | ||
const TOTAL_STEPS = 3; | ||
const stepInfoMsg = { |
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.
이것도 UPPER_SNAKE_CASE로 하면 어떨까요?
required: `작성자가 아닐 시 ${REQUIRE_ERROR_MSG}`, | ||
required: COMMON.error.required, |
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/constants/helperText.ts
Outdated
correct: '비밀번호가 일치합니다.', | ||
}, | ||
error: { | ||
incorrect: '비밀번호가 일치하지 않습니다.', |
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.
이부분 저는 mismatch로 사용햇는데 용어를 통일하는게 좋을까요?
setIsInvalidTitle(true); | ||
} | ||
if (authorTrim.length > 30) { | ||
setAuthorHelperText('저자명은 30자 이내여야 합니다.'); | ||
setAuthorHelperText(COMMON.error.maxLength(30)); |
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.
확실히 이렇게 사용하니까 훨씬 전달력 있는 것 같습니다!!
- 작성자가 이니면 비밀번호가 필요합니다 추가 - maxLength 최대 글자수 글자를 초과했습니다 로 변경 - stepInfoMsg -> STEP_INFO_MESSAGE 로 UPPER_SNAKE_CASE 적용 - #847
…FIRM_PASSWORD로 변경 #847
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.
굿이에용~~
연관 이슈
작업 요약
Helper Text 및 프론트에서 띄우는 input 관련 에러메세지를 constans로 뺐습니다.
작업 상세 설명
리뷰 요구사항
8분