-
Notifications
You must be signed in to change notification settings - Fork 1
✨ feat: 페이지네이션 구현 #25
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
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.
❗ 해당 파일에서 충돌이 일어나는 이유가 최신 develop branch에서 merge를 안해와서 그런 것 같습니다~ pr merge 하기 전에 최신 변경사항 반영 해주세용
src/components/common/Pagination.tsx
Outdated
| key={page} | ||
| type="button" | ||
| onClick={() => handleClick(page)} | ||
| className={`flex h-10 w-10 items-center justify-center rounded text-sm ${ |
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.
| className={`flex h-10 w-10 items-center justify-center rounded text-sm ${ | |
| className={`flex size-10 items-center justify-center rounded text-body2 ${ |
💬 height와 width가 같은 경우 size로 쓸 수 있습니다~
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.
네 감사합니다!! tailwind가 익숙하지 않다보니...감사합니당
src/components/common/Pagination.tsx
Outdated
| className={`flex h-10 w-10 items-center justify-center rounded text-sm ${ | ||
| currentPage === page | ||
| ? 'bg-[#FF8D72] font-bold text-white' | ||
| : 'text-black hover:bg-gray-100' |
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.
❓ hover 스타일은 임의로 추가하신 건가요?
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.
ux적으로 저런 호버 있는 사이트들이 많아서 넣었는데 다시 보니까 그렇게 필요하지 않을 듯 합니다
Moon-ju-young
left a comment
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.
추가로 최신 반영되지 않은 develop 브랜치를 merge하신 거 같아요! 다음 번부터는 develop 브랜치를 최신화 하고 merge하시는 게 좋을 것 같습니다!
Moon-ju-young
left a comment
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.
branch에 최신 변경 사항이 적용이 안 되어있어요!! 그 때문에 --spacing 설정이 다른 듯합니다. #17 참고하시고 최신 develop branch merge 하셔서 단위 수정 부탁드립니다!
Moon-ju-young
left a comment
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.
수정사항 모두 완료하신 건가요? 최신 사항 반영 (spacing 설정)이 아직 완료되지 않은 것 같네요~
📌 변경 사항 개요
📝 상세 내용
🔗 관련 이슈
🖼️ 스크린샷(선택사항)
💡 참고 사항
<Pagination
totalPages={23} // 총 페이지 수
currentPage={currentPage} // 현재 페이지
onChangePage={setCurrentPage} // 함수
/>
이런식으로 Props 받아서 사용하시면 됩니다