-
Notifications
You must be signed in to change notification settings - Fork 39
[김진선] sprint6 #194
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
[김진선] sprint6 #194
The head ref may contain hidden characters: "React-\uAE40\uC9C4\uC120-sprint6"
Conversation
- 상품 이미지 업로드 (1장 제한, 미리보기) - 상품명, 소개, 가격, 태그 입력 필드 구성 - 태그 입력 및 삭제 기능 구현 - 실시간 유효성 검사 및 에러 메시지 출력 - 등록 버튼 유효성 검사에 따라 비활성화 처리
- 이미지 등록 기능 (1장 제한, 미리보기 포함) - 상품명, 소개, 가격 입력 필드 및 유효성 검사 - 태그 추가 및 제거 기능 (Enter로 입력) - Form 유효성에 따라 등록 버튼 비활성화 처리 - 스타일 모듈화 및 폰트 색상 적용
addiescode-sj
left a comment
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.
수고하셨습니다!
제어 / 비제어 컴포넌트의 특징을 아주 잘 이해하셨군요.
좋은 시도입니다!
다만 일관성 측면에서 개선해야할 부분이 조금 보여서 관련해서 자세히 코멘트 드렸습니다 :)
주요 리뷰 포인트
- 일관성을 위해 useForm을 통한 폼 컨트롤 방식 개선
| const handlePriceChange = (e) => { | ||
| const formatted = formatWithCommas(e.target.value); | ||
| // 1) React state 갱신 | ||
| handleChange( | ||
| "productPrice", | ||
| formatWithCommas | ||
| )({ target: { value: formatted } }); | ||
| // 2) DOM value도 즉시 갱신 | ||
| productPriceRef.current.value = formatted; | ||
| }; |
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.
현재 data 상태와 ref를 동시에 사용하고 있어 일관성이 떨어지는 문제가 있습니다.
또한, 비제어 컴포넌트를 사용하면서도 ref로 DOM에 직접 접근하는 것은 설계상 모순입니다.
협업을 가정한다면 설계 의도에 따라 일정한 패턴으로 코드를 설계하는게 보다 중요해지는데요! 일관성을 위해 useForm 훅에서 ref값을 관리하는 역할을 일임하고, field value에 대한 ref를 생성해 getter와 setter를 추가해서 내려주는 방식은 어떨까요?
이렇게 개선한다면 현재 컴포넌트는 useForm에서 관리되는 ref값을 받아와 표시해주는 역할로 간소화되면서 모든 컴포넌트가 일관된 패턴으로 사용되고, 해당 라인에 있는 코드도 단순히 useForm에 추가된 setter 함수를 사용해주는 역할로 축소되어 역할이 명확해지고, 일관성이 생기겠네요 :)
예시)
const handlePriceChange = useCallback(
(e) => {
const formatted = formatWithCommas(e.target.value);
setFieldValue("productPrice", formatted);
},
[setFieldValue]
);| const isSubmitEnabled = | ||
| data.productName.trim() !== "" && | ||
| data.productDescription.trim() !== "" && | ||
| data.productPrice.trim() !== "" && | ||
| Array.isArray(data.productTag) && | ||
| data.productTag.length > 0; |
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.
위 코멘트처럼 getter함수를 useForm 훅에서 내려받는다면 이런 형태가 되겠죠?
const {
errors,
handleChange,
handleBlur,
handleSubmit,
getFieldValue,
setFieldValue,
} = useForm({
initialValues: {
productName: "",
productDescription: "",
productPrice: "",
productTag: [],
uploadImage: null,
},
validations,
onSubmit: (formData) => {
// 콤마 제거 + 숫자형으로 변환
const numericPrice = Number(formData.productPrice.replace(/,/g, ""));
const payload = {
...formData,
productPrice: numericPrice,
};
console.log("제출 데이터:", payload);
},
});예시)
| const isSubmitEnabled = | |
| data.productName.trim() !== "" && | |
| data.productDescription.trim() !== "" && | |
| data.productPrice.trim() !== "" && | |
| Array.isArray(data.productTag) && | |
| data.productTag.length > 0; | |
| const isSubmitEnabled = | |
| getFieldValue("productName")?.trim() !== "" && | |
| getFieldValue("productDescription")?.trim() !== "" && | |
| getFieldValue("productPrice")?.trim() !== "" && | |
| Array.isArray(getFieldValue("productTag")) && | |
| getFieldValue("productTag").length > 0; |
| label="상품명" | ||
| name="productName" | ||
| defaultValue="" | ||
| ref={productNameRef} |
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.
계속 이어서 피드백 드리자면, 여기서의 ref값은 필요없으니 날려줄수있고요!
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.
위에서 코멘트 드린 (일관성 관련) 피드백 getter, setter는 useForm 자체에서 ref값을 관리하는 역할을 일임할수있게끔 이렇게 추가해주시면 될 것 같습니다.
예시)
const getFieldValue = useCallback((key) => {
return dataRef.current[key];
}, []);
const setFieldValue = useCallback(
(key, value) => {
dataRef.current[key] = value;
const message = runValidation(key, value);
setErrors((prev) => ({ ...prev, [key]: message }));
},
[runValidation]
);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.
ref값을 관리하는 역할을 useForm 훅에서 일임하려면 관리하려는 ref값도 이 안에 있어야겠죠?
const formRef = useRef(null);
const dataRef = useRef(options?.initialValues || {});| const handleChange = (key, sanitizeFn) => (e) => { | ||
| const raw = e.target.value; | ||
| const value = sanitizeFn ? sanitizeFn(raw) : raw; | ||
| setData((prev) => ({ ...prev, [key]: value })); | ||
| // Change 단계 검증 | ||
| const message = runValidation(key, value); | ||
| setErrors((prev) => ({ ...prev, [key]: message })); | ||
| }; |
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.
마지막으로 useForm훅을 사용하던 컴포넌트에서 적용하셨던 제어 + 비제어를 혼용하셨던 방식은 여기서 바꿔줍시다!
const handleChange = useCallback(
(key, sanitizeFn) => (e) => {
const raw = e.target.value;
const value = sanitizeFn ? sanitizeFn(raw) : raw;
// ref에 데이터 저장 (비제어 컴포넌트)
dataRef.current[key] = value;
// 에러 상태만 업데이트 (제어 컴포넌트)
const message = runValidation(key, value);
setErrors((prev) => ({ ...prev, [key]: message }));
},
[runValidation]
);| // URL.createObjectURL(file)로 임시 URL 생성해서 프리뷰 띄우는 함수 | ||
| const handleFileChange = () => { | ||
| const file = uploadImgRef.current?.files?.[0]; | ||
| if (file) { | ||
| const url = URL.createObjectURL(file); | ||
| setPreviewUrl(url); | ||
| onImageChange(file, url); // 부모에게 전달 | ||
| } | ||
| }; |
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.
주석도 꼼꼼히 달아주시고 프리뷰를 띄우는 처리도 잘해주셨네요 :)
다만 주석을 포함하더라도 함수 사이에는 공백을 띄우는게 보기 좋아요.
16,17 사이에 공백 한칸 띄워줄까요? :)
| <div className={styles.imageWrapper}> | ||
| <div className={styles.imageUploadBox} onClick={handleUpload}> | ||
| <FontAwesomeIcon icon={faPlus} className={styles.plus} /> | ||
| <div>이미지 등록</div> | ||
| </div> | ||
| {/* preview 이미지 */} | ||
| {previewUrl && ( | ||
| <div className={styles.imagePreviewWrapper}> | ||
| <img | ||
| src={previewUrl} | ||
| alt="미리보기" | ||
| className={styles.imagePreview} | ||
| /> | ||
| <div | ||
| type="button" | ||
| onClick={handleRemovePreviewButton} | ||
| className={styles.removeImageButton} | ||
| > | ||
| <FontAwesomeIcon icon={faTimesCircle} /> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> |
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.
가독성이 심각하게 떨어지진않는데, 항상 인라인으로 jsx 작성하실때엔 3 depth 이상 네스팅되어 작성되지않도록 유의해봅시다!
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.
validation 관련된건 또 다른 훅으로 쪼개신점 아주 잘하셨습니다 👍
이렇게 재사용 가능한 로직의 단위를 잘게 쪼개면 재사용이 쉽겠죠?
질문에 대한 답변
아주 좋은 시도입니다! 👍 본문 내에 자세히 코멘트 드려봤어요 :) |
요구사항
공통
기본
상품 등록
심화
상품 등록
주요 변경사항
스크린샷
배포
https://jinsunkimdev-panda-market.netlify.app/additem
멘토에게