-
Notifications
You must be signed in to change notification settings - Fork 31
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 #130
The head ref may contain hidden characters: "react-\uC774\uD0DC\uBE48-sprint5"
[이태빈] Sprint5 #130
Conversation
전에 멘토링 때 React가 어렵다고 말씀하셨는데 우선 a태그로 접근을 먼저 하셔서 기능 구현부터 먼저 하신 것은 잘 하셨다고 생각합니다. React는 저도 여러번 강조드리다시피 생태계가 워낙 자주 바뀌고, 버전 앞자리가 바뀌면 현업 개발자들도 새로 러닝커브가 있습니다. |
pageSize: bestProducts, | ||
})); | ||
}, [device]); | ||
}; |
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.
제가 추후 더 상세히 리뷰를 달아드릴 예정인데 custom hook이 어떤 건지 처음에 어려우셨을텐데 잘 접근하려고 노력하신 점이 보입니다!
<img src="/assets/image/market/ic_arrow_left.svg" alt="왼쪽 화살표" /> | ||
</button> | ||
{pages.map((page) => { | ||
if (page > Math.ceil(totalCount / params.pageSize)) return; |
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.
그 코드리뷰 달아주시는거 어디부터 어디까지 해주신건지 범위를 볼줄 몰라서,, 처음부터 코멘트 달아주신 부분까지 보는건가요? 그리고 여기 pr코멘트에는 항상 4줄만 나오던데 코멘트해주신 코드로 들어가서 확인해야 하나요?
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.
네네~ 코멘트는 딱 해당 부분만 나와요. 그리고 해당 리뷰는 page > Math.ceil(totalCount / params.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.
좀 많이 돌아가서 과정이 길고 기억이 정확하게 나지 않아서 간략하게 설명해보겠습니다!
처음에는 pages.map이 아니라 그냥 함수로 페이지 수를 조절하려는 시도를 했습니다. pages의 배열의 내용을 바꾸려고 시도를 했던 것 같은데 많은 버그가 있었고, 여러번 시도를 하다가 구현이 되질 않아서 잠시 뒤로 미뤘었습니다. 그러다가 이제 문득 실제로 page를 나타낼 때 사용하는 데이터는 pages라는 배열이고, 그것을 렌더링 해주는 함수가 pages.map이니까 배열을 바꾸지 말고 직접적으로 렌더링 해주는 함수에 조건을 달면 배열을 굳이 바꾸지 않더라도 구현할 수 있지 않을까라는 생각이 들었습니다. 그래서 실제로 코드를 작성해보니 되었습니다.
제가 어떻게 코드를 작성했는지 설명하는게 처음하다보니 제대로 말씀드리지 못하는 것 같네요.. 혹시 다른 내용이 궁금하셨던거라면 코멘트 달아주시면 그 부분에 대해서 더 자세히 설명해보겠습니다!
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.
page > Math.ceil(totalCount / params.pageSize)
이 공식을 어떻게 유도했는지 물으신거라면
- 리스폰스의 객체에 totalCount값이 담긴걸 확인하고 활용하기로 결정
- 페이지 수는 총 컨텐츠 / 리퀘스트를 보내는 pageSize라는 것을 생각
- 소수점이 발생하는 경우 컨텐츠 유실이 발생하게 되어서 올림 처리를 하여서 모든 컨텐츠 유실 없도록 수정
- 디바이스의 크기에 따라 pageSize를 다르게 리퀘스트를 보내기 때문에 하드코딩에서 params.pageSize를 받도록 수정
- pages에는 다음으로 렌더링 할 page의 배열이 담겨있기 때문에 각 page가
page > Math.ceil(totalCount / params.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.
유도과정 좋아요. 멋져요!!
const instance = axios.create({ baseURL }); | ||
|
||
export const getProducts = async (params) => { | ||
const res = await instance.get("/products", { params }); |
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.
지난 시간에 배운 dto 기억나세요? keyword= 값이 없으면 이 params는 빼도 되겠죠?
<img src="/assets/image/market/ic_arrow_left.svg" alt="왼쪽 화살표" /> | ||
</button> | ||
{pages.map((page) => { | ||
if (page > Math.ceil(totalCount / params.pageSize)) return; |
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 [pages, setPages] = useState([1, 2, 3, 4, 5]); | ||
const [currentPage, setCurrentPage] = useState(1); | ||
const [active, setActive] = useState({ 1: true }); | ||
|
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.
나중에 pagination을 context로 표현하는 방법 고민해보세요!
const sortSelect = (orderBy) => { | ||
sortLoad(orderBy); | ||
}; | ||
|
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.
select를 custom hook으로 빼시면 어떨까요?
return ( | ||
<form | ||
onSubmit={(e) => { | ||
e.preventDefault(); |
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.
[심화학습] e.preventDefault()
를 지우지 않고 활용하는 사례를 한번 생각해보세요! (선택)
import "./SelectSort.css"; | ||
|
||
export const SelectSort = ({ sortLoad }) => { | ||
const [currentSort, setCurrentSort] = useState("최신순"); |
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.
이걸 영어로 바꿔보세요.
🐼판다마켓 : https://been-panda.vercel.app
요구사항
기본 요구사항
공통
중고마켓 페이지
심화 요구사항
공통
중고마켓 페이지
주요 변경사항
스크린샷
멘토에게