-
Notifications
You must be signed in to change notification settings - Fork 26
[정상인] sprint6 #111
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 #111
The head ref may contain hidden characters: "React-\uC815\uC0C1\uC778-sprint6"
Conversation
|
스프리트 미션 하시느라 수고 많으셨어요. |
태그 인풋은 엔터를 누를 때 등록되게 해서 따로 form을 구성했는데 이 방법이 어색한 느낌이 들어서 어떤지 궁금합니다.코드 보고 답변드리겠습니다 ! 😉 |
| .btn-small-40 { | ||
| height: 42px; | ||
| padding: 12px 23px; | ||
| border-radius: 8px; | ||
| border: none; | ||
| font-weight: 600; | ||
| font-size: 16px; | ||
| color: var(--gray100); | ||
| } |
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.
btn-small-40에서 40의 의미가 무엇인지 감이 잘 안잡히네요 ! 🤔
btn-small, btn-medium 등 스타일을 명확히 잘 나눴어요 !
뒤에 붙는 숫자가 무엇을 의미하는지 잘 모르겠으나 혹시나 height를 의미하는거라면 구조적으로 자주 사용되는 height-48과 같은 형태의 클래스도 만들어서 재사용해볼 수도 있겠네요 !
| const ErrorMessage = ({ errorMessage }) => { | ||
| return <div className="error-message">{errorMessage}</div>; | ||
| }; |
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.
오호 에러메시지를 재사용 해보려는건가요? 😉
| const LoadingSpinner = () => { | ||
| return ( | ||
| <div className="spinner-container"> | ||
| <div className="spinner" /> | ||
| </div> | ||
| ); | ||
| }; |
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.
굿굿 ! 로딩 스피너도 많이 재사용될 것으로 보이네요 👍👍
추 후 props를 추가하여 확장하기도 용이해보여요 ! 초안으로서 훌륭합니다 👍
| const getProducts = async ({ page = 1, orderBy = "recent", pageSize = 10 }) => { | ||
| return await instance.get("/products", { | ||
| params: { page, orderBy, pageSize }, | ||
| }); | ||
| }; |
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.
await과 async를 빼도 되겠네요 !
| const getProducts = async ({ page = 1, orderBy = "recent", pageSize = 10 }) => { | |
| return await instance.get("/products", { | |
| params: { page, orderBy, pageSize }, | |
| }); | |
| }; | |
| const getProducts = ({ page = 1, orderBy = "recent", pageSize = 10 }) => { | |
| return instance.get("/products", { | |
| params: { page, orderBy, pageSize }, | |
| }); | |
| }; |
해당 코드는 어차피 instance.get이 Promise 객체이므로 반환 타입은 Promise이며, 중간에 따로 기다린 후 처리하는 로직이 없기에 async와 await을 제외해도 될 것으로 보여요 😉
| return useQuery({ | ||
| queryKey: ["getProducts", page, pageSize, orderBy], | ||
| queryFn: () => getProducts({ page, orderBy, pageSize }), | ||
| staleTime: 300000, | ||
| select: (response) => response.data, | ||
| }); |
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.
오호 리액트 쿼리를 시도해보셨군요? 🫢
| const isDisabled = | ||
| formData.name && | ||
| formData.price && | ||
| formData.description && | ||
| formData.tags.length > 0; |
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.
크으 ~ 복잡한 조건은 별칭을 붙여두시는 습관 너무 좋습니다 👍👍
굳이 조건문을 보지 않더라도 해당 값은 불가능한 조건임이 명시되어 가독성이 좋네요 👍
| const handleDeleteImage = () => { | ||
| if (fileInputRef.current) { | ||
| fileInputRef.current.value = ""; | ||
| } | ||
| setPreviewImage(""); | ||
| setFormData((prev) => ({ ...prev, images: [] })); | ||
| setErrors((prev) => ({ ...prev, image: "" })); | ||
| }; | ||
|
|
||
| const handleChange = (e, category) => { | ||
| setFormData((prev) => ({ ...prev, [category]: e.target.value })); | ||
| }; | ||
|
|
||
| const handleSubmitAddItem = (e) => { | ||
| e.preventDefault(); | ||
| console.log(formData); | ||
| }; | ||
|
|
||
| const handleSubmitTag = (e) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (!inputValueTag) { | ||
| setErrors((prev) => ({ ...prev, tag: "태그를 입력하세요" })); | ||
| return; | ||
| } | ||
|
|
||
| if (formData.tags.includes(inputValueTag)) { | ||
| setErrors((prev) => ({ ...prev, tag: "태그가 이미 존재합니다" })); | ||
| return; | ||
| } | ||
|
|
||
| const newTags = [...formData.tags, inputValueTag]; | ||
| setFormData((prev) => ({ ...prev, tags: newTags })); | ||
| setInputValueTag(""); | ||
| setErrors((prev) => ({ ...prev, tag: "" })); | ||
| }; | ||
|
|
||
| const handleDeleteTag = (index) => { | ||
| const newTags = formData.tags.filter((_, i) => i !== index); | ||
| setFormData((prev) => ({ ...prev, tags: newTags })); | ||
| }; |
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.
크으 ~ 핸들러들이 단일 책임 원칙이 잘 지켜진 것 같아요 ! 훌륭합니다 👍👍
단일 책임 원칙(SRP)는 객체는 단 하나의 책임만 가져야 한다는 원칙을 말한다.
| <form className="addItem-form" onSubmit={handleSubmitAddItem}> | ||
| <AddItemFormHeader formData={formData} /> | ||
| <AddItemImage | ||
| image={previewImage} | ||
| ref={fileInputRef} | ||
| error={errors.image} | ||
| onChange={handleChangeImage} | ||
| onDelete={handleDeleteImage} | ||
| /> | ||
| <AddItemName | ||
| value={formData.name} | ||
| onChange={(e) => handleChange(e, "name")} | ||
| /> | ||
| <AddItemDescription | ||
| value={formData.description} | ||
| onChange={(e) => handleChange(e, "description")} | ||
| /> | ||
| <AddItemPrice | ||
| value={formData.price} | ||
| onChange={(e) => handleChange(e, "price")} | ||
| /> | ||
| </form> | ||
|
|
||
| <form onSubmit={handleSubmitTag}> | ||
| <AddItemTag | ||
| value={inputValueTag} | ||
| onChange={(e) => setInputValueTag(e.target.value)} | ||
| error={errors.tag} | ||
| /> | ||
| </form> | ||
|
|
||
| <TagList tags={formData.tags} onDelete={handleDeleteTag} /> |
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을 구성했는데 이 방법이 어색한 느낌이 들어서 어떤지 궁금합니다.
⚠️ 지금 작성주신 코드는 HTML 표준에 어긋납니다 !
다음 문서를 볼까요?:
tl;dr
4.10.3 The form element
Content model:
Flow content, but with no form element descendants.
with no form element descendants.
즉. form 엘리먼트 내에 form 엘리먼트를 제외한 Flow content가 들어갈 수 있다. 라고 명시되어 있습니다.
Flow content가 뭔가요?
Most elements that are used in the body of documents
도큐먼트를 구성하는 거의 대부분의 엘리먼트입니다 !li태그,span태그,form태그,h1~h6등등입니다.
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>
<form>
... 이미지, 상품 정보들, 태그 모두 입력
</form>
</div>이슈: 태그를 입력하고 엔터를 누르면 전체 데이터가 onSubmit에 의해서 제출되어 버린다. 그런데 태그를 입력하고 엔터를 누르면 태그를 등록되게 하고싶다. 그래서 폼을 두개로 나누자!
현재구조
<div>
<form>폼 1</form> // 태그를 포함하여 모든 입력값을 제출하는 부분
<form>폼 2</form> // 태그 부분
</div>Q. 원래 구조처럼 하나의 폼으로 관리할 수 있는 방법이 있었는가? + 이렇게 두 개로 나눈 방법이 이상한? 방법 인지에 대해서 궁금했습니다 !
즉. form 엘리먼트 내에 form 엘리먼트를 제외한 Flow content가 들어갈 수 있다. 라고 명시되어 있습니다.
라고 답변 주신 부분이 form 안에 form이 올 수 없다는 내용으로 이해를 해서, 위 질문에 대한 답변이 맞을까요?
|
|
||
| return ( | ||
| <div className="product-all"> | ||
| <h1 className="product-category-description">전체 상품</h1> |
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.
h1 태그가 중첩되지 않게 조심해야겠군요 ! 😉
하나의 문서에 h1은 두 개 이상이 되면 안되므로 한 번 체크해봐야겠네요 !
|
|
||
| const ProductAllMenuBar = ({ | ||
| isOpenDropdown, | ||
| setIsOpenDropdown, | ||
| onClickMenu, | ||
| orderBy, | ||
| }) => { | ||
| const navigate = useNavigate(); | ||
| const isMobile = useMediaQuery({ maxWidth: 767 }); | ||
| const DROPDOWN_MENUS = Object.keys(ORDER_BYS); |
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.
DROPDOWN_MENUS는 컴포넌트 바깥에 선언해도 되겠어요 !
상수를 활용하여 하나의 상수를 만든 것으로 보여요.
컴포넌트 내부에 있다면 리렌더링 시 keys()가 불필요하게 호출될 것이므로 컴포넌트 바깥에 두는게 더 나아 보입니다. 😉
앞으로도 컴포넌트의 자원(props, state)을 사용하지 않는다면 함수 바깥에 선언해보는 것도 고려해보면 좋습니다 !
| const ProductAllMenuBar = ({ | |
| isOpenDropdown, | |
| setIsOpenDropdown, | |
| onClickMenu, | |
| orderBy, | |
| }) => { | |
| const navigate = useNavigate(); | |
| const isMobile = useMediaQuery({ maxWidth: 767 }); | |
| const DROPDOWN_MENUS = Object.keys(ORDER_BYS); | |
| const DROPDOWN_MENUS = Object.keys(ORDER_BYS); | |
| const ProductAllMenuBar = ({ | |
| isOpenDropdown, | |
| setIsOpenDropdown, | |
| onClickMenu, | |
| orderBy, | |
| }) => { | |
| const navigate = useNavigate(); | |
| const isMobile = useMediaQuery({ maxWidth: 767 }); |
|
크으 ~~ 상인님. 기초 프로젝트 잘 봤습니다 ㅎㅎㅎ 종종 모각코 채널에 계시던데 또 만간 퇴근하고 모각코 한 번 하시죠 😉 미션 수행하시느라 수고 많으셨습니다 ! 👏 |
|
감사합니다 !!! |
요구사항
배포링크
체크리스트 [기본]
체크리스트 [심화]
주요 변경사항
스크린샷
멘토에게