[명지우] sprint6 #197
[명지우] sprint6 #197
Conversation
…nTextareaField로 분리
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
지우님 6번째 미션 제출 고생하셨습니다.
여러모로 구조에 대해 고민하신 것들이 느껴졌습니다.
협업을 하시면서 많이 배우신 것 같아 즐겁게 리뷰했습니다.
다음 미션도 화이팅이에요.
- 그전에 코멘트를 확인하시기 전에 작업하셨을 거라고 생각해요. 그 전 미션에서 드렸던 사항은 중복으로 달지 않았으니 참고해주세요~
| const BaseForm = ({ children, onSubmit, ...props }) => { | ||
| return ( | ||
| <form onSubmit={onSubmit} {...props}> | ||
| {children} | ||
| </form> | ||
| ); | ||
| }; | ||
|
|
||
| export default BaseForm; |
There was a problem hiding this comment.
❓ 질문
이렇게 form을 공통 컴포넌트로 빼야하는 이유가 있을까요?
단순히 form의 이름을 바꾸는 느낌이라 그냥 사용해도 될 것 같아요~
만약 의도가 있으시거나 추후 기능을 추가하실 생각이면 두셔도 좋습니다~
| @@ -0,0 +1,11 @@ | |||
| import styled from "@emotion/styled"; | |||
|
|
|||
| const BaseImageInput = ({ type = "file", onChange, ...props }) => { | |||
There was a problem hiding this comment.
💊 제안
BaseInput이라는 이름은 꼭 다른 image input이 있을것처럼 느껴지게 되네요. 다른 input이 없다면 그냥 ImageInput이여도 될 것 같아요!
| import styled from "@emotion/styled"; | ||
|
|
||
| const BaseImageInput = ({ type = "file", onChange, ...props }) => { | ||
| return <Input type={type} accept="image/*" onChange={onChange} {...props} />; |
There was a problem hiding this comment.
💊 제안
input의 accept 속성은 유저가 어떤 파일을 올려야하는지에 대한 힌트를 제공하는 속성입니다.
유저는 파일 업로드시 accept의 명시된 확장자 이외의 파일도 올릴 수 있으므로
실제 upload 함수에서 한번더 확장자를 검사해주시는 것이 좋습니다.
(사용자가 업로드창에서 옵션을 열어 확장자를 바꾸면 아래처럼 보입니다)
https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/accept
| width: 100%; | ||
| height: auto; | ||
| min-height: 8rem; | ||
| resize: none; |
There was a problem hiding this comment.
👍 칭찬
resize 막아두신 점 좋아요~
|
|
||
| const DeleteButton = ({ onClick, size = "s" }) => { | ||
| return ( | ||
| <DeleteButtonWrapper onClick={onClick} size={BUTTON_SIZE[size]}> |
There was a problem hiding this comment.
❗️ 수정요청
이런 클릭 가능한 요소의 경우 버튼을 사용해주세요. 더 적절한 태그가 있는데 div 태그에 onClick 을 주게되면 button에게 기대하는 동작을 제대로 수행하지 못합니다!
| <DeleteButtonWrapper onClick={onClick} size={BUTTON_SIZE[size]}> | |
| <DeleteButtonWrapper type="button" onClick={onClick} size={BUTTON_SIZE[size]}> |
버튼으로 바꿔주시고 type을 명시하시는 것을 추천드려요~
| const [btnAvailable, setBtnAvailable] = useState(false); | ||
| const [itemImage, setItemImage] = useState(null); | ||
| const [itemName, setItemName] = useState(""); | ||
| const [itemDescription, setItemDescription] = useState(""); | ||
| const [itemPrice, setItemPrice] = useState(""); | ||
| const [tag, setTag] = useState(""); |
There was a problem hiding this comment.
💊 제안
저는 관련있는 것들을 잘 모아두는 것이 가독성을 높여준다고 생각합니다. 지금의 경우 아이템을 생성하는 폼과 관련된 state 들을 각각의 useState로 관리하고 있습니다.
이런 경우 formValue 와 같은 하나의 state로 관리하시거나, useReducer를 이용해보시면 더 가독성 좋게 코드를 작성하실 수 있습니다.
https://ko.react.dev/learn/choosing-the-state-structure
https://ko.react.dev/learn/extracting-state-logic-into-a-reducer
이는 알고만 계시다가 추후 typescript 적용하실때 다시 생각해보시면 도움이 되실 것 같아요.
| import { memo } from "react"; | ||
| import ImageInputField from "@components/ImageInputField"; | ||
|
|
||
| const ItemNameInputField = ({ imageUrl, onChange, onDelete }) => { |
There was a problem hiding this comment.
❗️ 수정요청
해당 컴포넌트 명은 ItemImageInputField가 더 적절할 것 같아요.
| if (itemTags.has(tagValue)) { | ||
| toast(TAG_ALREADY_EXISTS_MESSAGE); | ||
| } |
There was a problem hiding this comment.
👍 칭찬
중복 태그 막으시고 유저에게 피드백 주시는 것 좋아요~
|
|
||
| export const useSubmitHandler = (formState) => { | ||
| const preventSubmitOnEnter = useCallback((e) => { | ||
| // enter로 폼 제출 방지 (태그 생성시 enter로 구분하기 때문이다. 단 textarea에서는 줄바꿈을 허용한다.) |
There was a problem hiding this comment.
💊 제안
form으로 인풋을 감싸게 되면 내부적으로 enter 키 입력시 submit 이벤트를 실행하게 됩니다. 이때 submit 버튼이 disabled 상태이면 제출 이벤트가 발생하지 않으므로 해당 폼 내부에서 버튼들에게 type을 잘 명시해주셨고 제출이 불가능한 상황에서 submit 버튼을 비활성화 해두셨다면 이는 불필요합니다~
There was a problem hiding this comment.
💊 제안
금액관련 인풋에 ,를 넣어 주신 것 좋습니다. 다만 지금의 경우 값이 입력되며 실시간으로 formatting을 수행하며 이에 따라 인풋이 입력될 당시의 커서 위치가 아니라 맨 뒤로 이동이 됩니다.
이런 경우 커서위치를 formatting 후 옮겨주시거나 판매가격 인풋에 포커스 시 다시 number 형태로 formatting하고 포커스가 떠나면 다시 금액 형태로 formatting 하는 방식으로 해주시면서 커서 위치를 유지시켜주셔야합니다~
2 participants
판다마켓 6
🌐 배포 url: https://myungjiwoo-pandamarket.netlify.app/additem
기본 요구사항
체크 리스트 (기본)
체크 리스트 (심화)
추가 기능
구현 포인트
Base~ 컴포넌트: 최소 단위 입력 컴포넌트~Field 컴포넌트: 공통 인터페이스를 추가한 확장 컴포넌트 (label, error messge 등)Item~Field 컴포넌트: 도메인 전용 컴포넌트스크린샷
멘토에게