-
Notifications
You must be signed in to change notification settings - Fork 39
[박광민] sprint6 #201
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 #201
The head ref may contain hidden characters: "React-\uBC15\uAD11\uBBFC-sprint6"
Conversation
…s를 사용하여 페이지 및 정렬 상태 관리 개선
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주차 미션 작업 고생하셨습니다.
컴포넌트를 잘 나눠주셔서 좋았습니다!
중고마켓 페이지에서 page, keyword 등을 urlParam으로 저장해 사용성을 높이신 것도 좋았습니다.
다음 미션도 화이팅이에요~
| !tags.length; | ||
|
|
||
| return ( | ||
| <div className={styles.AddItem}> |
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 요소를 사용하시면 UX측면에서도 좋고, form 단위로 조작하기도 좋습니다~
| <div className={styles.AddItemBar}> | ||
| <h2 className={styles.title}>상품 등록하기</h2> | ||
| <button | ||
| type="button" |
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.
❗️ 수정요청
등록 버튼은 영역을 제출하는 것이므로 submit type이 적절합니다.
| type="button" | |
| type="submit" |
| if (trimmed && !tags.includes(trimmed)) { | ||
| onChange([...tags, trimmed]); | ||
| } |
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.
💊 제안
중복되는 태그가 생성되지 않도록 해주신 점 좋습니다!
다만 이렇게 되면 사용자가 해당 동작에 대한 피드백을 받지 못하므로, alert, toast, input error 와 같은 방식으로 피드백을 주시면 더 좋을 것 같아요.
| <label className={styles.label}>태그</label> | ||
| <input | ||
| type="text" | ||
| value={input} | ||
| placeholder="태그를 입력해주세요" | ||
| onChange={(e) => setInput(e.target.value)} | ||
| onKeyDown={handleKeyDown} | ||
| className={styles.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.
💊 제안
라벨과 인풋을 사용하셨으니 연결해서 라벨 클릭시 자동으로 인풋으로 포커스가 가게 해주시면 더 좋겠네요~
| <label className={styles.label}>태그</label> | |
| <input | |
| type="text" | |
| value={input} | |
| placeholder="태그를 입력해주세요" | |
| onChange={(e) => setInput(e.target.value)} | |
| onKeyDown={handleKeyDown} | |
| className={styles.input} | |
| /> | |
| <label htmleFor="id" className={styles.label}>태그</label> | |
| <input | |
| id="id" | |
| type="text" | |
| value={input} | |
| placeholder="태그를 입력해주세요" | |
| onChange={(e) => setInput(e.target.value)} | |
| onKeyDown={handleKeyDown} | |
| className={styles.input} | |
| /> |
| const TagInput = ({ tags, onChange }) => { | ||
| const [input, setInput] = useState(""); | ||
|
|
||
| const handleKeyDown = (e) => { |
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.
❗️ 수정요청
한글로된 태그를 생성할 때 두번 입력이 발생하는 것 같은 현상이 있습니다. 이는 자모음으로 이루어진 한글을 입력중에 발생하는 현상입니다~
배포사이트에서 한글로 태그를 생성해보시고, 위의 동작을 고쳐보세요~
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 값을 useState로 제어하면 사용자가 입력할 때마다 컴포넌트가 리렌더링됩니다.
지금 코드상에서는 input 값을 바꿔주는 것이 아니라 제어 컴포넌트로 관리할 필요가 없을 것 같아요!
이를 비제어 컴포넌트로 바꾸셔서 불필요한 리렌더링을 줄이시는 것을 추천드려요!
| <p className={styles.text}>이미지 등록</p> | ||
| <input | ||
| type="file" | ||
| accept="image/*" |
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의 accept 속성은 유저가 어떤 파일을 올려야하는지에 대한 힌트를 제공하는 속성입니다.
유저는 파일 업로드시 accept의 명시된 확장자 이외의 파일도 올릴 수 있으므로
실제 upload 함수에서 한번더 확장자를 검사해주시는 것이 좋습니다.
(사용자가 업로드창에서 옵션을 열어 확장자를 바꾸면 아래처럼 보입니다)

https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/accept
| }; | ||
|
|
||
| const handleBlur = (e) => { | ||
| const raw = e.target.value.replace(/[^\d]/g, ""); |
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.
💊 제안
handleFocus 에서 동일한 정규식을 사용하고 있으므로 util로 추출해 관리하면 더 좋을 것 같아요~
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.
❗️ 수정요청
현재 이 컴포넌트는 value 값을 외부 상태로 관리하고 포맷팅된 값을 바로 onChange로 반영하고 있어 리렌더링이 되며 유저가 입력 중에 커서가 뒤로 이동되게 됩니다~
이런 경우 포메팅 후 수동으로 cursor를 옮겨주시거나 지금 작업하신 것처럼 blur 이벤트 시 포메팅을 수행하시면 됩니다~
가격을 입력해보시고 커서를 옮겨 , 뒤에서 삭제 및 수정등을 해보시며 동작을 확인해보시고 해당 현상을 수정해보세요~
https://minimo-sprint6.netlify.app/additem
요구사항
기본
상품 등록
심화
상품 등록
주요 변경사항
스크린샷
멘토에게