[유용민] sprint6 #196
[유용민] sprint6 #196
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
용민님 6번째 미션 작업 고생하셨습니다!
저도 tailwind를 좋아하는 해당 라이브러리를 사용하셔서 반가웠습니다~
제가 어떻게 컴포넌트를 나누는지에 대해 질문주셨는데, 일반적으로 저는 두가지 기준으로 컴포넌트 및 로직을 분리합니다.
하나는 재사용되는가. 하나는 가독성과 유지보수 측면에서 분리하는 것이 더 유리한가입니다.
제가 코멘트에도 남겨드렸 듯 코드를 잘 분리하면 읽는 사람이 코드를 잘 분석할 수 있게 됩니다.
따라서 반복사용되지 않아도 분리하는 것이 유리할 때가 꽤나 있습니다.
다만 이는 개개인마다 견해가 다를 것 같습니다~
추가로 스타일이 같다면 사용하는 스타일링 방법에 따라 컴포넌트로 분리하지 않아도 재사용할 수 있는 방법들이 있고 tailwind의 경우 그냥 작성하는 것을 권장하니 프로젝트에 따라 결정하시면 됩니다.
추가로 이와 관련해서 잘 설명해주는 프로그래머의 뇌라는 책이 있으니 궁금하시면 읽어보시는 것을 추천드려요!
다음 미션도 화이팅입니다~
| export default function Input({ id, type = "text", placeholder, ...props }) { | ||
| return ( | ||
| <input | ||
| id={id} | ||
| type={type} | ||
| placeholder={placeholder} | ||
| className="w-full p-4 bg-[#F3F4F6] placeholder:text-[#9CA3AF] rounded-md" | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
💊 제안
깔끔한 분리 좋습니다! 다만 추후 classname을 받을 수 있게 하시면 더 좋을 것 같아요.
이렇게 classname을 외부에서 받아야 기본 인풋위에 필요한 스타일을 적용시키며 확장성 있게 사용할 수 있습니다.
| export default function Input({ id, type = "text", placeholder, ...props }) { | |
| return ( | |
| <input | |
| id={id} | |
| type={type} | |
| placeholder={placeholder} | |
| className="w-full p-4 bg-[#F3F4F6] placeholder:text-[#9CA3AF] rounded-md" | |
| {...props} | |
| /> | |
| ); | |
| } | |
| export default function Input({ id, type = "text", placeholder, classname ="", ...props }) { | |
| return ( | |
| <input | |
| id={id} | |
| type={type} | |
| placeholder={placeholder} | |
| className={`w-full p-4 bg-[#F3F4F6] placeholder:text-[#9CA3AF] rounded-md ${classname}`} | |
| {...props} | |
| /> | |
| ); | |
| } | |
참고로 tailwind의 경우 작성한 클래스 네임의 순서가 뒤에 있다고 적용이 보장되지 않기 때문에
아래 문서를 읽어보시고 이러한 특징도 고려해서 컴포넌트를 수정해보시면 더 좋겠습니다!
There was a problem hiding this comment.
💊 제안
하나의 파일이 너무 큰 것 같아요~
내부적으로 아이템 생성을 위한 로직들과 state가 custom hook으로 분리되고 UI 들도 컴포넌트로 분리되면,
가독성 측면에서 더 좋을 것 같아요.
컴포넌트로 분리하는 것은 생각의 단위를 나누는 것과 같이 때문에 너무 크게 가지고 가시면 로직을 파악하기도 어렵고 유지보수시에도 불리합니다!
| const onImageChange = (event) => { | ||
| if (selectedImage) { | ||
| setShowWarning(true); | ||
| return; | ||
| } | ||
|
|
||
| const { | ||
| target: { files }, | ||
| } = event; |
There was a problem hiding this comment.
💬 여담
event 인자를 사용하는 부분이 없어서 처음부터 구조분해할당해서 받아도 될 것 같아요~
| const onImageChange = (event) => { | |
| if (selectedImage) { | |
| setShowWarning(true); | |
| return; | |
| } | |
| const { | |
| target: { files }, | |
| } = event; | |
| const onImageChange = ({ target }) => { | |
| if (selectedImage) { | |
| setShowWarning(true); | |
| return; | |
| } | |
| const file = target.files?.[0]; |
| if (e.key === "Enter" && tagInput.trim()) { | ||
| e.preventDefault(); | ||
| if (!tags.includes(tagInput.trim())) { | ||
| setTags([...tags, tagInput.trim()]); | ||
| } | ||
| setTagInput(""); | ||
| } |
There was a problem hiding this comment.
💊 제안
중복되는 태그가 생성되지 않도록 해주신 점 좋습니다!
다만 이렇게 되면 사용자가 해당 동작에 대한 피드백을 받지 못하므로 왜 태그가 추가되지 않고 인풋값은 초기화되는지에 대해 알 수가 없습니다. 이럴때 alert, toast, input error 와 같은 방식으로 피드백을 주시면 더 좋을 것 같아요.
|
|
||
| return ( | ||
| <div className="max-w-[120rem] mx-auto p-7"> | ||
| {/* <p>상태 확인: {isFormValid ? "✅ 유효함" : "❌ 유효하지 않음"}</p> */} |
There was a problem hiding this comment.
❗️ 수정요청
개발중 확인을 위한 코드는 지우고 올려주세요~
| {/* <p>상태 확인: {isFormValid ? "✅ 유효함" : "❌ 유효하지 않음"}</p> */} |
| <form | ||
| onSubmit={(e) => e.preventDefault()} |
| <div | ||
| type="button" | ||
| onClick={() => removeTag(tag)} | ||
| className="ml-2 bg-[#9CA3AF] p-1 rounded-full" | ||
| > |
There was a problem hiding this comment.
❗️ 수정요청
이런 클릭 가능한 요소의 경우 버튼을 사용해주세요. 더 적절한 태그가 있는데 div 태그에 onClick 을 주게되면 button에게 기대하는 동작을 제대로 수행하지 못합니다!
| <div | |
| type="button" | |
| onClick={() => removeTag(tag)} | |
| className="ml-2 bg-[#9CA3AF] p-1 rounded-full" | |
| > | |
| <button | |
| type="button" | |
| onClick={() => removeTag(tag)} | |
| className="ml-2 bg-[#9CA3AF] p-1 rounded-full" | |
| > | |
| type="file" | ||
| id="photo" | ||
| name="photo" | ||
| accept="image/*" |
There was a problem hiding this comment.
💊 제안
input의 accept 속성은 유저가 어떤 파일을 올려야하는지에 대한 힌트를 제공하는 속성입니다.
유저는 파일 업로드시 accept의 명시된 확장자 이외의 파일도 올릴 수 있으므로
실제 upload 함수에서 한번더 확장자를 검사해주시는 것이 좋습니다.
(사용자가 업로드창에서 옵션을 열어 확장자를 바꾸면 아래처럼 보입니다)
https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/accept
| setShowWarning(false); | ||
| }; | ||
|
|
||
| const handleTagKeyDown = (e) => { |
There was a problem hiding this comment.
💬 여담
한글로 태그 입력시 마지막 글자가 중복으로 입력되는 것을 막기위해서 state를 통해 로직을 구성하신 것 같아요~
이럴때 키보드 이벤트의 isComposing 속성을 이용해서 로직을 짜실 수도 있습니다.
2 participants
요구사항
https://15-sprint-mission-tawny.vercel.app/
기본
상품 등록
심화
상품 등록
주요 변경사항
스크린샷
멘토에게
태그는 엔터를 하면 추가 되도록 구현해 놓았습니다.
이미지는 input의 file속성을 이용하여 추가 가능하도록 구현하였습니다.
소제목이랑 Input의 스타일이 같아서 컴포넌트로 만들까 생각도 했는데 상품 소개의 경우에는 약간의 스타일이 달라서 textarea로 처리했습니다.
주강사님이라면 어디까지 컴포넌트로 작성하였을지 궁금합니다.
셀프 코드 리뷰를 통해 질문 이어가겠습니다.