-
Notifications
You must be signed in to change notification settings - Fork 0
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
Page/signup payment regist card #24
base: main
Are you sure you want to change the base?
Conversation
|
||
export default function CardInfoArea<T extends PaymentMethodCardInfo>({ register, formState }: CardInfoProps<T>) { | ||
const { t } = useTranslation(['common', 'page-payment']) | ||
const BirthDateInputFieldOnChangeEvent = useBirthDateInputFieldOnChangeEvent() |
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.
- 변수명이 대문자로 시작해서 컴포넌트랑 구분하기가 어렵네요.
- 이름만 봐서는 무슨 역할을 하는 변수인지 가늠이 안가요. onChange 핸들러를 위한 콜백이라면 그 점이 분명하게 나타나도록 onBirthDateChange 같이 핸들러라는 점을 쉽게 유추할 수 있도록 짓는 것을 추천드립니다.
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.
함수명 네이밍에 있어 룰을 명확히 했어야 했는데 실수했네요.. ㅠ
register: T extends PaymentMethodCardInfo ? UseFormRegister<T> : never; | ||
formState: T extends PaymentMethodCardInfo ? UseFormStateReturn<T> : never; |
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로 받지 않아도 됩니다. rhf의 를 사용해서 선언적으로 관리하는 방법이 있어요.
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.
FormProvider와 useFormContext가 있다는 게 작업당시 떠오르지 안았네요.. ㅠ
작업아형 적용하였습니다.
|
||
} | ||
return <form onSubmit={handleSubmit(cardSubmitAction)}> | ||
<CardInfoArea<PaymentMethodCardInfoWithPolicy> {...form}></CardInfoArea> |
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.
이 부분도 FormProvider 동일한 내용입니다
return policies?.billingAgree && | ||
policies?.paymentGateWayPolicy && | ||
policies?.privatePolicy && | ||
policies?.transferInformationAbroadPolicy && | ||
policies?.transferInformationToThirdPartiesPolicy | ||
}, [ |
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.
const [policies, setPolicies] = useState<DeepPartial<PaymentMethodPolicies> | undefined>(formState.defaultValues)
이 부분과 같이 묶어서 훅으로 뺄 수도 있을듯 하군요
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.
해당 부분 hook으로 별도 제작하여 적용해보았습니다.
src/lib/InputFilter/index.ts
Outdated
export const NumberInputFieldOnChangeEvent: ChangeEventHandler<HTMLInputElement> = (e) => { | ||
e.target.value = e.target.value.replace(/\D+/g, '') | ||
} | ||
|
||
export const CardNumberInputFieldOnChangeEvent: ChangeEventHandler<HTMLInputElement> = (e) => { | ||
NumberInputFieldOnChangeEvent(e) | ||
e.target.value = e.target.value.split(/(\d{1,4})/).filter(Boolean).join(' ') | ||
} | ||
|
||
export const CardExpireDateInputFieldOnChangeEvent: ChangeEventHandler<HTMLInputElement> = (e) => { | ||
NumberInputFieldOnChangeEvent(e) | ||
e.target.value = e.target.value.split(/(\d{1,2})/).filter(Boolean).splice(0, 2).join('/') | ||
} | ||
|
||
export const BirthDateInputFieldOnChangeEventForEn: ChangeEventHandler<HTMLInputElement> = (e) => { | ||
NumberInputFieldOnChangeEvent(e) | ||
const matchResult = /(\d{2})(\d{2})?(\d{4})?(.+)/.exec(e.target.value) | ||
if (matchResult) { | ||
e.target.value = [matchResult[1], matchResult[2], matchResult[3], matchResult[4]].filter(Boolean).slice(0, 3).join('/') | ||
} | ||
} | ||
|
||
export const BirthDateInputFieldOnChangeEventForKr: ChangeEventHandler<HTMLInputElement> = (e) => { |
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.
이 함수들은 전부 event를 파라미터로 받을 필요가 없어보여요. 특정 string 값을 받아서 리턴하는 것에 불과하기 때문에 event 대신 string parameter를 받도록 변경하고 함수명도 그에 맞춰 바꿀 수 있을 것 같습니다.
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.
해당 부분 작업하여 적용해 보았습니다.
Quality Gate passedIssues Measures |
작업 내용
카드 결제 등록을 위한 페이지 추가 및 구성 입력 폼 추가
추가된 사항
수정한 사항
시도하본 요소