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

경북대 FE_이정민 4주차 과제 Step 3,4 #85

Open
wants to merge 25 commits into
base: userjmmm
Choose a base branch
from

Conversation

userjmmm
Copy link

@userjmmm userjmmm commented Jul 19, 2024

안녕하세요 멘토님, 경북대 FE 이정민입니다!
React Hook Form를 사용하여 기존의 form을 리팩터링한 코드를 제출합니다.

처음에 ProductDetail 페이지만 수정했을 때는 기존 코드를 productQuantity만 useState로 관리해서 React-Hook-Form의 편리성이 와닫지 않았는데, validation을 추가하는 과정에서 코드가 훨씬 깔끔해지는 걸 느꼈습니다.
useState를 많이 사용했던 OrderPage는 더더욱 잘 느꼈습니다 ㅎㅎ

다만 아직까지 코드도 미흡한 부분이 많고 React-Hook-Form을 활용하기 위한 개념도 부족한 만큼, 더 알아보야겠다고 느꼈습니다!
따라서 이후 배운 개념들을 바탕으로 실제 사용할 때 주의해야 할 점을 코드 리뷰로 보완할 예정입니다.

늘 감사합니다 😊

userjmmm and others added 25 commits July 17, 2024 14:29
@userjmmm userjmmm changed the title 경북대 FE_이정민 4주차 과제 Step 3 경북대 FE_이정민 4주차 과제 Step 3,4 Jul 20, 2024
Copy link

@sjoleee sjoleee left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~! 리뷰 남깁니다.

Comment on lines +28 to +55
const onSubmit = (data: FormValues) => {
if (data.message.trim() === "") {
alert("선물 메시지를 입력해주세요.");
return;
}

if (data.cashReceipt) {
if (data.cashReceiptNumber.trim() === "") {
alert("현금영수증 번호를 입력해주세요.");
return;
}

if (isNaN(Number(data.cashReceiptNumber))) {
alert("현금영수증 번호는 숫자만 입력해주세요.");
return;
}
}

alert("주문이 완료되었습니다.");
};

if (!productDetail || !productQuantity) {
return <Navigate to="/" />;
}

return (
<Box p={8}>
<form onSubmit={handleSubmit(onSubmit)}>
Copy link

Choose a reason for hiding this comment

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

handleSubmit의 첫번째 인자는 validation을 통과했을 때 실행되는 콜백입니다. 두번째 인자로 validation을 통과하지 못했을 때 실행되는 콜백을 받고 있으니 유효하지 않을때 실행될 로직들은 두번째 인자로 넘겨주면 어떨까요?

Copy link

Choose a reason for hiding this comment

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

전체적으로 유효성 검사 로직이 흩어져있어서 응집도가 낮다고 생각해요.
order를 위한 유효성 검사를 한 군데서 관리할 수는 없을까요~?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants