Conversation
There was a problem hiding this comment.
제 코드리뷰 앞에 붙어있는 태그들의 의미는 아래와 같습니다.
- P0 : 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
- P1 : 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
- P2 : 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)
개발하시느라 고생많으셨습니다!
전체적으로 요구사항에 대해서 잘 구현하신 것 같습니다.
남긴 코멘트들이 꼭 해당 파일들이 아니어도 다른 파일들에도 적용될 수 있으니, 코드 전체적으로 리뷰 코멘트들을 반영해보시면 좋을 것 같습니다.
| <div id="root"></div> | ||
| <script type="module" src="/src/main.jsx"></script> | ||
| </body> | ||
| </html> |
There was a problem hiding this comment.
[P2]
- 해당 파일에서 챙길 수 있는 SEO 최적화나 웹 접근성 부분들을 공부해보면 나중에 도움이 될 것 같습니다.
meta description, OG 태그, 기본 언어 설정 등
| const [isTablet, setIsTablet] = useState( | ||
| window.innerWidth < TABLET_BREAKPOINT | ||
| ); |
There was a problem hiding this comment.
[P0]
해당 부분에서 초기값이 설정될 때 폰과 모바일 기기에서도 isTablet 값이 true가 될 것 같습니다. AND 연산자를 통해 모바일의 너비보다는 큰 경우를 챙겨주면 좋을 것 같습니다.
| setIsMobile(width < MOBILE_BREAKPOINT); | ||
| setIsTablet(width < TABLET_BREAKPOINT); |
There was a problem hiding this comment.
이 부분도 위에서 언급한 부분과 동일할 것 같네요.
| const UsedMarket = () => { | ||
| return ( | ||
| <section> | ||
| <div className={styles.used_market__container}> |
There was a problem hiding this comment.
[P2]
개인적인 취향의 문제일수도있으나 해당 컴포넌트와 module.scss가 used-market 이라는 점이 인식이 잘되는 경우에는 className을 정의할 때 해당 부분은 제외하고 정의하는 편이 추후 길어지는 className을 대비하여 더 가독성이 좋을 것 같아요.
styles.used_market__container => styels.container
| return ( | ||
| <li key={id} className={styles.item}> | ||
| <div className={styles.image_container} style={{ height: `${height}px` }}> | ||
| <img src={images[0]} alt={name} /> |
There was a problem hiding this comment.
[P0]
배열의 0번째에 이미지가 없을 경우의 예외처리를 해주면 좋을 것 같습니다.
| const COMMON_ITEM_HEIGHT = 220; | ||
|
|
||
| const PAGE_SIZE = 10; | ||
|
|
||
| const ORDER_LIST = ['좋아요순', '최신순']; | ||
|
|
||
| const COMMON_ITEM_DATA_PARAM = { | ||
| page: 1, | ||
| pageSize: PAGE_SIZE, | ||
| orderBy: 'favorite', | ||
| keyword: '', | ||
| }; | ||
|
|
There was a problem hiding this comment.
[P1]
코드 유지보수 측면에서 이렇게 정의된 상수 값들은 프로젝트 src/constants 와 같이 폴더를 만들어서 각 도메인 단위로 사용한다면 같은 도메인 내에서 재사용이 편리하고 추후 수정시 용이합니다.
| const fetchBestItems = async () => { | ||
| const response = await productFetch(BEST_ITEM_DATA_PARAM); | ||
| setBestItems(response.list); | ||
| }; | ||
|
|
||
| fetchBestItems(); |
There was a problem hiding this comment.
[P0]
async await를 통해 API를 호출하는 경우 try catch로 감싸서 에러 처리를 해주는 것이 좋습니다.
There was a problem hiding this comment.
[P1]
추가로 로딩상태를 추가하여 fetch를 해오는 동안 사용자가 품목들이 불러와지고 있음을 인지시켜주는 것이 좋습니다.
state를 통한 로딩상태와 해당 로딩상태에 맞는 스켈레톤 컴포넌트에 대해서 개발해보면 좋을 것 같습니다.
| const getPageNumbers = () => { | ||
| const pageNumbers = []; | ||
| const pageGroupSize = 5; | ||
|
|
||
| let startPage = | ||
| Math.floor((currentPage - 1) / pageGroupSize) * pageGroupSize + 1; | ||
| let endPage = Math.min(totalPage, startPage + pageGroupSize - 1); | ||
|
|
||
| if (endPage - startPage < pageGroupSize - 1) { | ||
| startPage = Math.max(1, endPage - pageGroupSize + 1); | ||
| } | ||
|
|
||
| for (let i = startPage; i <= endPage; i++) { | ||
| pageNumbers.push(i); | ||
| } | ||
|
|
||
| return pageNumbers; | ||
| }; |
There was a problem hiding this comment.
질문
해당 부분이 페이지가 많을 때 페이지를 옮기면 제대로 각 페이지의 그룹 단위로 페이지 숫자들이 변경될까요?
요구사항
기본요구사항
공통
중고마켓페이지
PC, Tablet, Mobile 디자인에 해당하는 중고마켓 페이지를 만들어 주세요.
중고마켓 페이지 url path는 별도로 설정하지 않고, '/'에 보이도록 합니다.
상단 네비게이션 바, 푸터는 랜딩 페이지와 동일한 스타일과 규칙으로 만들어주세요.
상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 "/products" 를 활용해주세요.
베스트 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 "/products"의 정렬 기준 favorite을 사용해주세요.
심화 요구사항
공통
중고마켓페이지
중고 마켓의 카드 컴포넌트 반응형 기준은 다음과 같습니다.
반응형에 따른 페이지 네이션 기능을 구현합니다.
주요 변경사항
스크린샷
멘토에게