[배수민] sprint5 #177
[배수민] sprint5 #177
Conversation
…ithub-actions [Fix] delete merged branch github action
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨습니다!
첫 리액트 미션인데 어렵지않게 잘 해주셨네요 ㅎㅎ
주요 리뷰 포인트
- URLSearchParams 사용하기
- svg 컴포넌트화해서 사용하기
- key 역할 이해하기
- 유지보수를 고려한 개발
src/api/Items.js
Outdated
| pageSize = 4, | ||
| orderBy = "favorite", | ||
| }) { | ||
| const query = `page=${page}&pageSize=${pageSize}&orderBy=${orderBy}`; |
There was a problem hiding this comment.
URLSearchParams 객체 사용해서 여러개의 파라미터를 관리해보는 방법으로 바꾸면, 쿼리 스트링의 파싱, 조작, 인코딩 등의 작업을 간편하게 처리할 수 있으면서도 실수를 줄일 수 있겠죠? :)
아래 아티클 참고해보세요!
참고
There was a problem hiding this comment.
URLSearchParams 사용하는 방식으로 변경하였습니다!
There was a problem hiding this comment.
굳굳! 스타일링에 관련된 props를 늘리지않고 type을 받는 방식으로 구조화하셨네요. 이렇게되면 명확한 props만 남게되어 유지보수에도 좋고 파일을 쉽게 확장하는것도 가능해지겠죠?
왜 props의 종류가 많아지는게 좋지 않은지는 아래 아티클 참고해보세요!
There was a problem hiding this comment.
확장성 고려해서 컴포넌트를 만들어야 겠다는 생각이 드네요
src/components/Card.js
Outdated
| import defaultImg from "../img/img_default.svg"; | ||
| function Card({ data }) { |
There was a problem hiding this comment.
import 구문과 컴포넌트 시작 사이에 공백 한칸 넣어주세요!
src/components/Card.js
Outdated
| </div> | ||
|
|
||
| <div className="card__icon__group"> | ||
| <img src={heartIcon} alt="하트 아이콘" className="heart__icon" /> |
There was a problem hiding this comment.
해당 하트 아이콘 클릭하면 사용자에게 피드백을 주기 위해 색이 채워지거나 border 두께가 두꺼워지거나 색깔이 진해지지않을까요? ㅎㅎ 이런 인터렉션을 구현하려면 svg를 컴포넌트화하여 props를 보내주는게 좋을것같아요.
예시를 들어드릴게요!
There was a problem hiding this comment.
- HeartIcon 컴포넌트 만들기 (예시)
import React from 'react';
function HeartIcon({ isActive = false }) {
return (
<svg
width="24"
height="24"
viewBox="0 0 24 24"
fill={isActive ? "#FF0000" : "none"}
stroke={isActive ? "#FF0000" : "#000000"}
strokeWidth="2"
strokeLinecap="round"
strokeLinejoin="round"
>
<path d="M20.84 4.61a5.5 5.5 0 0 0-7.78 0L12 5.67l-1.06-1.06a5.5 5.5 0 0 0-7.78 7.78l1.06 1.06L12 21.23l7.78-7.78 1.06-1.06a5.5 5.5 0 0 0 0-7.78z" />
</svg>
);
}
export default HeartIcon;- 사용할때
<HeartIcon isActive={} />
There was a problem hiding this comment.
하트 아이콘 컴포넌트로 변경하였습니다!
src/components/Dropdown.js
Outdated
| {dropdownList?.map((item) => ( | ||
| <li key={item.value} onClick={() => onClickDropdownItem(item)}> | ||
| {item.name} | ||
| </li> | ||
| ))} |
There was a problem hiding this comment.
key는 형제 컴포넌트들 사이에서 식별자 역할을 하기때문에, key에 쓰인 값에 중복된 값이 있을 시 비교 알고리즘이 제대로 작동하지않아 렌더링 최적화가 안디고, 상태 및 이벤트 바인딩또한 의도치 않은 사이드 이펙트를 발생시킬수있어요. item.value가 절대 중복되지않는 값이 맞는지 확인해보고, 다른 값을 사용하거나 방어 코드를 써줄까요? :)
There was a problem hiding this comment.
key-index 형식으로 방어코드로 변경하였습니다!
src/components/Nav.js
Outdated
| <NavLink to="items" style={getLinkStyle}> | ||
| 중고마켓 | ||
| </NavLink> |
There was a problem hiding this comment.
이정도는 함수를 따로 만들 필요없이 인라인으로 작성해주셔도 무방할것같아요.
만약 조건이 더 추가되는것을 고려하거나, 보다 일관된 방식으로 클래스이름 조합을 관리하고싶다면 라이브러리의 도움을 받아보는건 어떨까요?
There was a problem hiding this comment.
인라인으로 수정하였습니다.
| const bestPageSize = devicePageSize[deviceType]["best"]; | ||
|
|
||
| const allPageSize = devicePageSize[deviceType]["all"]; | ||
|
|
||
| const onClickNextPage = () => { | ||
| setPaginationCurrentPage(paginationCurrentPage + 1); | ||
| }; | ||
|
|
||
| const onClickPrevPage = () => { | ||
| setPaginationCurrentPage(paginationCurrentPage - 1); | ||
| }; | ||
|
|
||
| const onClickPage = (page) => { | ||
| setPaginationCurrentPage(page); | ||
| }; |
There was a problem hiding this comment.
아이템 목록을 보여주는 역할의 컴포넌트에 페이지네이션 로직까지 결합되어있으니 복잡해보여요. 해당 로직(페이지네이션 관련)은 페이지네이션을 사용하는 컴포넌트라면 꾸준히 재사용될거예요. 로직을 재사용하기위해 usePagination과 같은 커스텀 훅으로 분리해보면 어떨까요? :)
There was a problem hiding this comment.
usePagination 훅으로 분리하였습니다! 분리했는데도 코드가 생각보다 많네요
질문에 대한 답변
네, 지금은 useEffect를 사용해 API를 호출하는 방식으로 작성하시는게 맞습니다.
에러가 난 상황을 그대로 사용자에게 보여주는것보단 따로 처리를 해주는게 낫겠죠? |
5 participants
요구사항
기본
중고마켓
중고마켓 반응형
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
심화
주요 변경사항
배포 사이트 링크
https://sprint-misson5-suminbae.netlify.app/items
스크린샷
멘토에게