-
Notifications
You must be signed in to change notification settings - Fork 38
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 #184
The head ref may contain hidden characters: "React-\uC190\uC7AC\uD5CC-sprint6"
[손재헌] Sprint6 #184
Conversation
@@ -0,0 +1,18 @@ | |||
import Header from "../components/header/Header"; | |||
import AddItemForm from "../components/AddItemForm"; | |||
import { Helmet } from "react-helmet"; |
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.
리액트 헬멧을 쓰셨군요 ㅎㅎ 메타태그 작성할때 많이 쓰는건데 잘 쓰셨습니다 ㅎㅎ
<Helmet> | ||
<title>상품 등록</title> | ||
</Helmet> | ||
<Header /> |
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.
헤더는 많은 페이지들이 공유하고 있으므로 Layout
이란 개념을 이용해서 구현 해 볼 수 있습니다.
아래의 블로그를 참고해보세요.
https://be-a-weapon.tistory.com/entry/React-라우터Router와-레이아웃Layout-쪼개기
|
||
function AddItemForm() { | ||
|
||
const [values, setValues] = useState(INITIAL_VALUES); |
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.
form
요소들을 하나의 객체로 관리하셨군요 ~ 저도 이렇게 자주 합니다.
대신에 상태 이름을 value
라기보다 productFormData
라던가 product
라던가 어떤 데이터를 나타내는지 좀 더 설명가능하면 변수면 좋겠네요 .
const handleChange = (name, value) => { | ||
setValues((preValues) => ({ | ||
...preValues, | ||
[name]: value, | ||
})) | ||
} | ||
|
||
const handleInput = (e) => { | ||
handleChange(e.target.name, e.target.value); | ||
} | ||
|
||
const handlePriceInput = (e) => { | ||
if(isNaN((Number)(e.target.value))) return; | ||
handleChange(e.target.name, e.target.value ? (Number)(e.target.value) : e.target.value); | ||
} |
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.
가장 기본이되는 handleChange
를 만들고 각각 요소에 대한 핸들러를 개발하신것도 잘 하셨어요.
저도 자주하는 패턴입니다.
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.
한 파일에 컴포넌트를 다 담지말고 addForm 폴더를 만들어서 한 파일에 한 컴포넌트씩 관리하면 좋을 것 같습니다.
return ( | ||
<form className="add-item-form" onSubmit={handleSubmit}> | ||
<AddItemFormHeader values={ values } /> | ||
<FileInputSection onChange={ handleChange } valuesImages={values.images} /> |
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.
FileInputSection
은 파일과 관련된 로직 및 UI들이 함께 있네요. 파일이라는 같은 관심사끼리 잘 응집되어 있기때문에 좋은 컴포넌트라고 생각합니다.
useEffect(() => { | ||
if(!valuesImages) return; | ||
const imgURL = URL.createObjectURL(valuesImages); | ||
setPrieview(imgURL); | ||
|
||
return () => { | ||
setPrieview(null); | ||
URL.revokeObjectURL(imgURL); | ||
} | ||
}, [valuesImages]); |
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.
이 부분은 추후 로직을 바꾸어야 할 것으로 예상이됩니다.
로컬 파일을 브라우저에서 업로드하여 미리보기를 할때에는 createObjectUrl을 사용하면 되지만 파일을 서버에 업로드 후 서버의 이미지 url을 받아왔을때에는 createObjectUrl을 사용 할 수 없고, 이미지 태그 src에 서버 이미지 url을 넣어주어야합니다.
이쪽 로직을 바꾸거나 preview 조건부 렌더링 로직을 바꾸어 주어야 합니다
괜찮습니다. 시간을 가지고 천천히 반영하는게 중요하지 꼭 차주에 반영을 해야하는건 아닙니다 ㅎㅎ 스프린트 미션 수고하셨습니다 !1 |
요구사항
기본
심화
주요 변경사항
멘토에게