-
Notifications
You must be signed in to change notification settings - Fork 33
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
[홍승원] sprint5 #62
The head ref may contain hidden characters: "React-\uD64D\uC2B9\uC6D0-sprint5"
[홍승원] sprint5 #62
Conversation
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.
승원님 이번 미션도 고생 많으셨습니다~! 👍
리엑트가 많이 낯설 수 있는데 state, props, hook 등 기본 개념을 잘 익히신 거 같습니다 :) 💯 리뷰 참고하셔서 컴포넌트 크기가 너무 큰 부분, 데이터 호출시 정렬과 관련된 문제, 그리고 중복되는 로직들을 다시 살펴보시면 더욱 좋을 거 같습니다 :)
진행하시다가 어려움이 있으시면 언제든 DM주세요~!
전체 상품의 개수로 버튼을 만들게 되면 버튼을 눌렀을 때 구동되는 페이지네이션의 기능이 의미가 없지 않나요? 그냥 보여주는 건 totalCount를 보여주는 개수로 나누면 되지만 검색했을 때의 결과 개수 같은 것들은 어떤식으로 처리를 해야할까요?
-> 검색했을 때도 totalCount가 주어집니다~! 똑같이 처리하실 수 있어요! :)
지금 구현한 옵션으로 순서 바뀌는 건 보여지는 것들 안에서 바뀌는데 모든 리스트의 순서가 바뀌게 하려면 페이지네이션을 사용하지 않고 전체를 다 읽어와서 정렬해야 하는데 이런 경우에는 페이지네이션도 이용하면서 정렬하려면 서버에서 이미 처리된 상태로 읽어오기만 하는 방법을 주로 사용하나요?
-> 네 맞습니다! 대부분의 정렬, 분류, 검색 등은 서버에서 처리하게 됩니다! 리뷰 참고해주세요 :)
import PaginationButton from "./PaginationButton"; | ||
import { Link } from "react-router-dom"; | ||
|
||
const Body = () => { |
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.
컴포넌트 단위가 너무 큽니다! 😢
베스트 상품, 모든 상품, 페이지네이션 각각의 모든 상태, api 로직, jsx가 섞여 있다 보니 코드를 빠르게 읽거나 찾기가 어렵습니다~! 컴포넌트를 나누어보시면 좋을 거 같아요 :)
let allProductForSort = []; | ||
let paginationNumber = []; | ||
|
||
while (hasMore) { |
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 getAllProduct = async (pageSize) => { | ||
if (option === "latest") { |
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.
위와 마찬가지입니다! 정렬은 서버에서 해야 합니다. 지금은 프론트엔드로 불러온 pageSize 개수 만큼의 데이터에서만 정렬이 되고 있는데, 그러면 실제로 유저가 보고 싶은 데이터를 보여줄 수 없습니다!
|
||
// 페이지네이션 버튼 클릭해서 불러오는 값 바꾸기 | ||
const buttonClick = (num) => { | ||
setClickedPage(() => num); |
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.
같은 역할을 하는 상태 값이 중복되어 정의된 거 같습니다~!
}; | ||
|
||
// 검색하기 위해서 엔터를 눌렀을 때 | ||
// 고민 : 전체 개수의 길이로 버튼의 개수를 만들려면 전체를 다 읽어봐야 하는데 그럼 페이지네이션의 의미가...? |
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.
setSearchProduct(response.list); | ||
} else if (window.innerWidth <= 1199) { | ||
const pagesize = 6; | ||
const response = await getProduct(pageNum, pagesize, value); |
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.
사실상 같은 목적을 가진 로직들이 여기저기 너무 흩어져 있습니다. 중복도 많이 생기고, 코드 흐름을 따라가기가 힘듭니다!
pageSize, searchValue 등의 값이 바뀔때 마다 계속해서 products 호출 관련 로직이 반복되고 있죠.
해당 로직을 한 곳에 모아보면 어떨까요?
useEffect(() => {
const response = await getProduct(pageNum, pagesize, value);
setAllProducts(response.list);
}, [pageNumber, pageSize, value])
대략적으로 위와 같이 productcs 를 불러오는 부분을 하나로 묶고 pageNumber, pageSize, value 등의 상태가 바뀌는 로직만 따로 분리할 수 있겠죠!
(대략적인 코드이므로 참고만 해주세요!)
|
||
<div className="all-product-cardContainer"> | ||
{isSearch | ||
? searchProduct.map((product, index) => ( |
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.
사실상 같은 렌더링을 하는 상태 값이라면 searchProduct, allProduct를 나누지 않을 수 있을 거 같네요 :)
|
||
// 클린업 | ||
return () => { | ||
window.removeEventListener("resize", handleResize); |
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.
클린업도 챙겨주셨군요 👍
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게