[유동환] sprint6 #200
[유동환] sprint6 #200
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
동환님 6번째 미션 제출 고생하셨습니다~
미션 5에서 드렸던 코멘트와 중복되는 코멘트는 지양하고 있습니다! 참고해주세요~
다음 미션도 화이팅입니다!
| const addItemImageWrapper = css` | ||
| ` |
There was a problem hiding this comment.
❗️ 수정요청
빈 css block은 지우시는 것을 추천드려요!
| const addItemImageWrapper = css` | |
| ` |
| } | ||
| ` | ||
|
|
||
| const ItemTag = css` |
There was a problem hiding this comment.
❗️수정요청
스타일을 정의할 때 ItemTag처럼 대문자로 시작하면 컴포넌트처럼 보일 수 있어 혼동을 줄 수 있습니다.
스타일 객체는 일반 변수로 간주되므로 첫문자를 소문자로 작성하는 것이 좋을 것 같아요!
| <input | ||
| id='fileUpload' | ||
| type='file' | ||
| accept='image/*' |
There was a problem hiding this comment.
💊 제안
input의 accept 속성은 유저가 어떤 파일을 올려야하는지에 대한 힌트를 제공하는 속성입니다.
유저는 파일 업로드시 accept의 명시된 확장자 이외의 파일도 올릴 수 있으므로
실제 upload 함수에서 한번더 확장자를 검사해주시는 것이 좋습니다.
(사용자가 업로드창에서 옵션을 열어 확장자를 바꾸면 아래처럼 보입니다)
https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/accept
| return <div>AddItem</div>; | ||
|
|
||
| const [imgPreviewUrl, setImgPreviewUrl] = useState(null); // 추가했을때 이미지프리뷰 | ||
| const [showWarning, setShowWarning] = useState(false); |
There was a problem hiding this comment.
💊 제안
boolean 타입을 추론할 수 있게 isWarning, isImageUploaded 같은 이름을 추천드립니다~
추가로 imgPreviewUrl state를 통해 업로드된 이미지가 존재하는지 파악할 수 있어 해당 state는 없어도 될 것 같아요~
| const handleSubmitButton = () => { | ||
|
|
||
| } |
There was a problem hiding this comment.
❗️ 수정요청
| const handleSubmitButton = () => { | |
| } |
| </div> | ||
| </label> | ||
| {imgPreviewUrl && ( | ||
| <div css={previewBox}> |
There was a problem hiding this comment.
❗️ 수정요청
해당 영역이 header위로 올라가네요. 확인해보시고 수정해보세요~
| <label>판매가격</label> | ||
| <input id="price" placeholder="판매가격을 입력해주세요" type='number' value={itemPrice} onChange={(e) => setItemPrice(e.target.value)}></input> |
There was a problem hiding this comment.
💊 제안
라벨과 인풋을 사용하셨으니 연결해서 라벨 클릭시 자동으로 인풋으로 포커스가 가게 해주세요.
| <label>판매가격</label> | |
| <input id="price" placeholder="판매가격을 입력해주세요" type='number' value={itemPrice} onChange={(e) => setItemPrice(e.target.value)}></input> | |
| <label htmlFor="price">판매가격</label> | |
| <input id="price" placeholder="판매가격을 입력해주세요" type='number' value={itemPrice} onChange={(e) => setItemPrice(e.target.value)}></input> |
| id="tag" | ||
| placeholder="태그를 입력해주세요" | ||
| value={itemTag} onChange={(e) => setItemTag(e.target.value)} | ||
| onKeyDown={(e) => { |
There was a problem hiding this comment.
💊 제안
onKeyDown 내부 로직이 길어지면 컴포넌트가 복잡해지므로, 별도의 핸들러 함수로 분리하면시면 좋겠습니다.
There was a problem hiding this comment.
❗️ 수정요청
한글로된 태그를 생성할 때 두번 입력이 발생하는 것 같은 현상이 있습니다. 이는 자모음으로 이루어진 한글을 입력중에 발생하는 현상입니다~
배포사이트에서 한글로 태그를 생성해보시고, 위의 동작을 고쳐보세요~
| if (!itemHashTag.includes(newTag)) { | ||
| setItemHashTag((prev) => [...prev, newTag]) | ||
| } |
There was a problem hiding this comment.
💊 제안
중복되는 태그가 생성되지 않도록 해주신 점 좋습니다!
다만 이렇게 되면 사용자가 해당 동작에 대한 피드백을 받지 못하므로, alert, toast, input error 와 같은 방식으로 피드백을 주시면 더 좋을 것 같아요.
There was a problem hiding this comment.
💊 제안
하나의 파일이 너무 큰 것 같아요~
내부적으로 아이템 생성을 위한 로직들과 state가 custom hook으로 분리되고 UI 들도 컴포넌트로 분리되면,
가독성 측면에서 더 좋을 것 같아요.
컴포넌트로 분리하는 것은 생각의 단위를 나누는 것과 같이 때문에 너무 크게 가지고 가시면 로직을 파악하기도 어렵고 유지보수시에도불리합니다!
2 participants
https://scintillating-ganache-14d305.netlify.app/additems
요구사항
기본
상품 등록
심화
주요 변경사항
스크린샷
데스크탑
모바일
멘토에게