-
Notifications
You must be signed in to change notification settings - Fork 39
[이태식] sprint6 #212
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 #212
The head ref may contain hidden characters: "React-\uC774\uD0DC\uC2DD-sprint6"
Conversation
GANGYIKIM
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.
태식님 6번째 미션 작업 고생하셨습니다!
태그 기능까지 추가되면 완벽해질 것 같아요.
추후 시간이 되시면 구현해보세요.
다음 미션도 화이팅입니다!
| @@ -0,0 +1,11 @@ | |||
| const Input = ({ placeholder, className, value, onChange }) => { | |||
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.
💊 제안
공통 컴포넌트는 다양하게 사용될 수 있어야 합니다!
Input의 type 속성도 받을 수 있으면 좋을 것 같아요~
| const Input = ({ placeholder, className, value, onChange }) => { | |
| const Input = ({ type, placeholder, className, value, onChange }) => { |
| @@ -0,0 +1,11 @@ | |||
| const Input = ({ placeholder, className, value, onChange }) => { | |||
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.
💊 제안
className을 받으시고 빈 문자열로 초기화해주셔야 className에 undefined가 들어가지 않습니다~
| const Input = ({ placeholder, className, value, onChange }) => { | |
| const Input = ({ placeholder, className = "", value, onChange }) => { |
| return ( | ||
| <div className="bg-secondary-100 text-secondary-800 font-regular flex items-center justify-between gap-8 rounded-[26px] py-5 pr-12 pl-16 text-lg"> | ||
| {children} | ||
| <button onClick={() => onDelete(children)} className="cursor-pointer"> |
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.
❗️ 수정요청
button type 속성의 기본값은 submit입니다~ 아래의 경우는 button이 더 적절할 것 같아요~
| <button onClick={() => onDelete(children)} className="cursor-pointer"> | |
| <button type="button" onClick={() => onDelete(children)} className="cursor-pointer"> |
|
|
||
| const Navbar = () => { | ||
| const location = useLocation(); | ||
| console.log(location); |
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.
❗️ 수정요청
| console.log(location); |
| useEffect(() => { | ||
| if (name && description && price && tag) setIsDisable(false); | ||
| else setIsDisable(true); | ||
| }, [name, description, price, tag]); |
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.
💊 제안
아래처럼 작성하시는 것이 더 가독성에 좋을 것 같아요
| useEffect(() => { | |
| if (name && description && price && tag) setIsDisable(false); | |
| else setIsDisable(true); | |
| }, [name, description, price, tag]); | |
| useEffect(() => { | |
| const isValidForm = name && description && price && tag; | |
| setIsDisable(!isValidForm); | |
| }, [name, description, price, tag]); |
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.
❗️ 수정요청
등록 버튼 활성화 조건중 tag는 문자열이 아니라 문자열의 배열입니다~ 태그 추가 기능을 구현하시면서 이 부분도 같이 수정하시면 좋겠습니다~
| <h2 className="text-2lg">상품명</h2> | ||
| <Input | ||
| value={name} | ||
| placeholder="상품명을 입력해주세요" | ||
| onChange={(e) => setName(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.
💊 제안
label로 작성하셔서 input과 연결해주시는 것을 추천드려요!
| <h2 className="text-2lg">상품명</h2> | |
| <Input | |
| value={name} | |
| placeholder="상품명을 입력해주세요" | |
| onChange={(e) => setName(e.target.value)} | |
| /> | |
| <label htmlFor="name" className="text-2lg">상품명</label> | |
| <Input | |
| id="name" | |
| value={name} | |
| placeholder="상품명을 입력해주세요" | |
| onChange={(e) => setName(e.target.value)} | |
| /> |
| <img src={PlusIcon} alt="더하기" className="size-48" /> | ||
| 이미지 등록 | ||
| {!preview && ( | ||
| <input type="file" className="hidden" onChange={uploadImage} /> |
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.
💊 제안
이미지 파일만 받을 수 있도록 구현하시면 더 좋을 것 같아요~
참고로 accept 속성을 이용하시고, handleUploadImg 함수 내부에서 추가적으로 확장자를 한번 더 검사하셔야합니다.
accpet 속성은 제안일 뿐 제한이 아니기 때문에 유저가 accept의 명시된 확장자 이외의 파일도 올릴 수 있습니다~
| }; | ||
| return ( | ||
| <> | ||
| <Input |
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.
❗️ 수정요청
태그가 추가되는 로직을 추가해주세요~
| </div> | ||
| <div className="flex flex-col gap-16"> | ||
| <h2 className="text-2lg">판매가격</h2> | ||
| <Input |
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.
💊 제안
지금과 같은 금액 input 인풋의 경우 number 타입이 더 적절할 것 같아요!
요구사항
기본
심화
주요 변경사항
스크린샷
https://leetaesik-sprint.netlify.app/additem

멘토에게