Conversation
| </ul> | ||
| ) : ( | ||
| <p className="list-none">상품이 없습니다.</p> | ||
| )} |
There was a problem hiding this comment.
p3;
JSX 리턴부에서 삼항연산자가 중첩되면 가독성이 확 떨어져보입니다!
요거는 Loading과 Product들을 보여주는 부분으로 삼항연산자를 구분하고.
Product 보여주는 부분에서 위처럼 분기가 필요하다면, 따로 위에서 변수로 선언하여 사용하는게 좋을 것 같아요!
const AllProductList = () => {
...
const ProductList = products.length > 0 ? (
<ul className="product-list">
{products.map(product => (
<li key={product.id}>
<ProductCard product={product} />
</li>
))}
</ul>
) : (
<p className="list-none">상품이 없습니다.</p>
);
...
return (
<>
{loading ? <Loadingbar /> : ProductList}
</>
)
}| @@ -0,0 +1,14 @@ | |||
| function Loadingbar() { | |||
| return ( | |||
| <div class="load-wrapp"> | |||
There was a problem hiding this comment.
p3;
class는 자바스크립트에서 예약어이기 때문에, jsx에서 className으로 사용하셔야 합니다!
이미 아시는데, 깜박하신 것 같네용
| const handleTagRemove = tagToRemove => { | ||
| const updatedList = tagList.filter(tag => tag !== tagToRemove); | ||
| setTagList(updatedList); | ||
| onTagListChange(updatedList); |
There was a problem hiding this comment.
p3;
현재 AddItem.js에서 관리하고 있는 tag 리스트가 있어서
사실상 TagInput 컴포넌트에서 중복해서 tagList 상태를 관리할 필요가 없어보여요!
부모에 있는 values의 tags를 사용하면 충분히 상태 하나로 관리가 가능하고,
이렇게 setTagList가 발생할 때마다 onTagListChange가 동일하게 호출되는 일이 없을 것 같아요!
| }; | ||
|
|
||
| useEffect(() => { | ||
| if (reset) { |
There was a problem hiding this comment.
p3;
reset 이라는 상태도 제가 이해하기론 showTagList 상태와 동일하게 따라가는 것 같아요.
결과적으로 reset, tagList 모두 부모 컴포넌트에서 하나로 관리하는게 좋을 것 같아요!
| setShowTagList(false); | ||
| onTagListChange([]); // 부모 컴포넌트에 빈 태그 목록 전달 | ||
| } | ||
| }, [reset]); |
There was a problem hiding this comment.
p4;
react에서 setState 역할을 하는 것 제외하고는,
useEffect에 사용된 함수들도 deps에 작성해주어야 하는 것이 원칙적으로 맞습니다!
지금 상황에서는 deps를 생략해도 기능적으로 크게 문제가 있는건 아니지만
이렇게 deps를 생략하다보면 나중에 예상치 못한 부분에서 사이드이펙트가 발생합니다.
deps를 모두 지정하는 습관을 들이면 좋을 것 같아요!
참고로 onTagListChange라는 함수를 그냥 그대로 deps에 넣어버리면
함수가 계속 재생성되고, 그에 따라 useEffect가 계속 발동되기 때문에
onTagListChange를 useCallback등으로 감싸시는게 좋습니다!
| ); | ||
| } | ||
|
|
||
| export default TagInput; |
There was a problem hiding this comment.
실제로 태그 부분 인터페이스를 보니까 이쁘고 부드럽게 잘 구현하신 것 같아요!!👍
| type="text" | ||
| name="tag" | ||
| placeholder="태그를 입력 후 Enter를 눌러 추가하세요" | ||
| value={tagInput} |
There was a problem hiding this comment.
p4;
요거는 tagInputValue 등의 변수명을 사용하는게 더 적절할 것 같아용!
| } else { | ||
| setIsFormValid(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
p3;
isFormValid, setIsFormValid 요 부분을 보면서,
멘토링 때 언급 드렸던 것을 한 번 리마인드 해봅시다!
잘 생각해보면 isFormValid라는 상태는
사실상 이미 상태로 관리하고 있는 values 객체의 값들에 의해 계산되는 애죠?
요런 경우에는 isFormValid라는 상태를 정의하고
그때 그때 업데이트 하는 대신
values의 업데이트에 따라 isFormValid가 계산되도록 하면 더 좋을 것 같아요!
이럴때 보통 useMemo 훅을 많이 사용합니다.
const [values, setValues] = useState({
imgFile: '',
product: '',
content: '',
price: 0,
tag: [],
});
const isFormValid = useMemo(() => {
const { product, content, price, tag } = values;
return (product !== '' && content !== '' && price !== 0 && tag.length > 0)
}, [values])| if (resetTagInput) { | ||
| setResetTagInput(false); | ||
| } | ||
| }, [resetTagInput]); |
There was a problem hiding this comment.
p3;
resetTagInput의 변경을 감지하고
resetTagInput이 true가 되었을 때, resetTagInput을 다시 false로 변경한다.
이 useEffect를 어떤 의도로 작성하신건가용?
There was a problem hiding this comment.
p3; resetTagInput의 변경을 감지하고 resetTagInput이 true가 되었을 때, resetTagInput을 다시 false로 변경한다.
이 useEffect를 어떤 의도로 작성하신건가용?
submit 버튼 누르면 리셋이 되는데 그때 사실 api post? 보내는 역할을 할껀데.. 아직 api 연결은 안해서.. 그냥 리셋을 false 한다라고 설정만 해둔 상태입니다.
| </div> | ||
| </form> | ||
| </section> | ||
| </main> |
There was a problem hiding this comment.
p4;
요번에 시간이 없으셔서 못하셨지만, Form에 있는 각각의 인풋 아이템들을 컴포넌트화하면 더 깔끔할 것 같아요!
|
윤주님 고생하셨습니다! 어떤 훅을 언제 어떻게 써야하는지, 요런거는 사실 정답이 있는 것도 아니고 |
기능만 정신없이 만들어서 조금 빈약한 부분이 있습니다. |
스프린트 6미션
요구사항
기본
심화
스크린샷
멘토에게