-
Notifications
You must be signed in to change notification settings - Fork 0
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
✨ feat: Pagination 컴포넌트 구현 #87
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 [pageNumber, setPageNumber] = useState(currentPage) | ||
|
||
const offset = | ||
pageNumber % 5 == 0 |
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.
p4; == 는 타입 체킹이 되지 않는 연산자라 ===를 주로 사용하긴 합니다,,,
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.
허허 하나 빠뜨렸네요 ^^
{ | ||
length: | ||
totalPages - pageStartNumber + 1 < 5 | ||
? totalPages - pageStartNumber + 1 |
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.
p5; 그대로 두는거 보단 전체를 하나의 변수로 만들고 변수 이름을 잘 지으면 좀 더 가독성이 좋을거 같은데 다은님 편하신대로 하시면 됩니다.
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.
네엡 반영하겠습니다
currentPageCount 정도면 괜찮으실까요!
? totalPages - pageStartNumber + 1 | ||
: 5, | ||
}, | ||
(_, index) => index + 1 + 5 * offset, |
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.
p5; 이 코드도 위와 동일한 의견입니다~
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.
그거땜에 위에 빼놨는데 안썼네요?ㅋㅋㅋ
totalPages, | ||
handleChangePage, | ||
}: PaginationProps) => { | ||
const [pageNumber, setPageNumber] = useState(currentPage) |
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.
p3; storybook에 pageNumber state가 있는 것을 보니 제 생각에는 pageNumber와 setPageNumber 모두 props로 주입받아서 사용이 가능 할 거 같은데 Pagination 컴포넌트에도 pageNumber state가 있는 이유가 궁금합니다..!
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.
클릭하면 내부에서 자체적으로 페이지 넘버의 상태가 변해야하기 때문입니다!
> | ||
<Icon | ||
icon="arrow-right" | ||
size={11.8} |
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.
p3; arrow-left와 동일합니다!
> | ||
<Icon | ||
icon="arrow-left" | ||
size={11.8} |
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.
p3; figma로 보니까 size 17.5 인거 같더라고여 한번 다시 확인해보시겠습니까
요약
페이지네이션에 사용할 컴포넌트를 구현하였습니다!
사진 (구현 캡처)
자세한 동작 테스트는 스토리북을 확인하세요!
작업 내용
기타 (논의하고 싶은 부분)
타 직군 전달 사항
백엔드 전달 사항
디자이너 전달 사항
close #77