-
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
[JOJI-702] 페이지네이션 #17
The head ref may contain hidden characters: "feat/JOJI-702/\uD398\uC774\uC9C0\uB124\uC774\uC158"
Conversation
|
Warning Rate limit exceeded@MuteAim has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Walkthrough헤이 헤이 Chill... 이번 변경사항은 프로젝트의 구조를 재구성하고 페이지네이션 기능을 도입하는 것에 중점을 두고 있어요. 주요 변경 사항으로는 컴포넌트들의 디렉토리 이동, 새로운 페이지네이션 컴포넌트 추가, 그리고 공지사항 페이지에 페이지네이션 로직 구현이 포함되어 있습니다. 이 변경은 코드의 모듈성과 사용자 경험을 개선하는 데 초점을 맞추고 있어요. Changes
Sequence DiagramsequenceDiagram
participant User
participant NoticePage
participant usePagination
participant fetchPosts
User->>NoticePage: 페이지 접속
NoticePage->>usePagination: 초기 페이지 설정
NoticePage->>fetchPosts: 첫 페이지 데이터 요청
fetchPosts-->>NoticePage: 포스트 데이터 반환
NoticePage->>User: 포스트 렌더링
User->>NoticePage: 페이지 변경
NoticePage->>usePagination: 페이지 업데이트
NoticePage->>fetchPosts: 새 페이지 데이터 요청
fetchPosts-->>NoticePage: 새 포스트 데이터 반환
NoticePage->>User: 업데이트된 포스트 렌더링
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/app/post/page.js (3)
14-14
: 에러 처리 개선이 필요합니다.데이터 페칭 시 발생할 수 있는 에러에 대한 처리가 미흡합니다. 사용자에게 적절한 에러 메시지를 표시하고, 에러 상태를 관리하는 것이 좋습니다.
export default function PostPage() { const [posts, setPosts] = useState([]); const [currentPage, setCurrentPage] = useState(1); const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); useEffect(() => { const fetchPosts = async () => { try { setLoading(true); + setError(null); const response = await fetch(`${process.env.NEXT_PUBLIC_BASE_URL}/posts`); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } const json = await response.json(); setPosts(json.data); } catch (err) { + setError('데이터를 불러오는 중 오류가 발생했습니다.'); + console.error('Error fetching posts:', err); } finally { setLoading(false); } }; fetchPosts(); }, []);Also applies to: 20-28
46-69
: 페이지 그룹 크기를 상수로 정의하는 것이 좋습니다.코드 여러 곳에서 매직 넘버 '5'를 사용하고 있습니다. 이는 유지보수성을 저하시키고 실수를 유발할 수 있습니다.
+// 페이지 그룹당 페이지 수 +const PAGE_GROUP_SIZE = 5; // 이전 페이지 그룹으로 이동 const handlePrev = () => { - const prevGroupStart = Math.max(Math.floor((currentPage - 1) / 5) * 5, 1); + const prevGroupStart = Math.max(Math.floor((currentPage - 1) / PAGE_GROUP_SIZE) * PAGE_GROUP_SIZE, 1); setCurrentPage(prevGroupStart); }; // 다음 그룹으로 이동 const handleNext = () => { const nextGroupStart = Math.min( - Math.floor((currentPage - 1) / 5) * 5 + 6, + Math.floor((currentPage - 1) / PAGE_GROUP_SIZE) * PAGE_GROUP_SIZE + PAGE_GROUP_SIZE + 1, totalPages); setCurrentPage(nextGroupStart); }; const getPageRange = () => { const pages = []; - const rangeStart = Math.floor((currentPage - 1) / 5) * 5 + 1; - const rangeEnd = Math.min(rangeStart + 4, totalPages); + const rangeStart = Math.floor((currentPage - 1) / PAGE_GROUP_SIZE) * PAGE_GROUP_SIZE + 1; + const rangeEnd = Math.min(rangeStart + PAGE_GROUP_SIZE - 1, totalPages); for (let i = rangeStart; i <= rangeEnd; i++) { pages.push(i); } return pages; };
121-151
: 페이지네이션 접근성 개선이 필요합니다.현재 구현은 스크린 리더 사용자와 키보드 사용자를 위한 접근성이 부족합니다. ARIA 레이블과 키보드 네비게이션을 추가하는 것이 좋습니다.
- <div className="flex justify-center items-center gap-2 mt-6"> + <nav aria-label="페이지 네비게이션" className="flex justify-center items-center gap-2 mt-6"> <Button onClick={handlePrev} disabled={currentPage === 1} + aria-label="이전 페이지 그룹으로 이동" className="text-gray-500 disabled:opacity-50" > < </Button> {getPageRange().map((page) => ( <Button key={page} onClick={() => handlePageChange(page)} + aria-label={`${page} 페이지로 이동`} + aria-current={page === currentPage ? 'page' : undefined} className={`${ page === currentPage ? "bg-orange-500 text-white" : "text-gray-600" }`} > {page} </Button> ))} <Button onClick={handleNext} disabled={currentPage === totalPages} + aria-label="다음 페이지 그룹으로 이동" className="text-gray-500 disabled:opacity-50" > > </Button> - </div> + </nav>
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.
멋집니다 ~~ 나중에 공부할 때도 참고해보기 좋을 것 같아요
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/app/_components/ui/pagination.jsx (3)
7-17
: 접근성 속성이 잘 구현되었으나, 추가 개선이 가능합니다.현재 구현은 좋지만, 다음과 같은 ARIA 속성을 추가하면 더 나은 접근성을 제공할 수 있습니다:
<nav role="navigation" aria-label="pagination" + aria-live="polite" + aria-relevant="additions text" className={cn("mx-auto flex w-full justify-center", className)} {...props} />
27-30
: 불필요한 빈 문자열을 제거하세요.
cn("")
에서 빈 문자열은 불필요하므로 제거하는 것이 좋습니다.- <li ref={ref} className={cn("", className)} {...props} /> + <li ref={ref} className={cn(className)} {...props} />
48-89
: 네비게이션 컴포넌트들의 접근성이 잘 구현되었습니다.Previous, Next, Ellipsis 컴포넌트들이 적절한 aria 레이블과 스크린 리더 지원을 포함하고 있습니다.
하나의 작은 제안: 한국어 사용자를 위해 "Previous"와 "Next" 텍스트를 현지화하는 것이 좋을 것 같습니다.
- <span>Previous</span> + <span>이전</span> - <span>Next</span> + <span>다음</span> - <span className="sr-only">More pages</span> + <span className="sr-only">더 많은 페이지</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/app/_components/ui/pagination.jsx
(1 hunks)
🔇 Additional comments (2)
src/app/_components/ui/pagination.jsx (2)
1-5
: 필요한 모든 의존성이 올바르게 임포트되었습니다.컴포넌트에 필요한 React, 아이콘 및 유틸리티 함수들이 적절하게 임포트되었습니다.
92-100
: 모든 컴포넌트가 올바르게 내보내기되었습니다.필요한 모든 컴포넌트들이 적절하게 export되어 있습니다.
-한 페이지에 보여줄 NOTICE 게시글을 정하고 싶으면 limit= 에다가 원하는 숫자를 입력하면됩니다.
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/app/post/page.js (2)
28-28
: API 요청 URL을 상수로 분리하는 것이 좋습니다.API 엔드포인트와 관련 파라미터를 상수로 분리하면 유지보수가 용이해질 것 같습니다.
// constants/api.js export const API_ENDPOINTS = { POSTS: `${process.env.NEXT_PUBLIC_BASE_URL}/posts`, }; export const DEFAULT_PARAMS = { LIMIT: 8, CATEGORY: 'NOTICE', };
94-94
: 라우팅 로직 개선이 필요합니다.
router.push()
호출 시 상대 경로를 사용하는 것이 좋습니다.-onClick={() => router.push(`/postDetail/${post.id}`)} +onClick={() => router.push(`./postDetail/${post.id}`)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/post/page.js
(3 hunks)
🔇 Additional comments (1)
src/app/post/page.js (1)
8-16
: shadcn Pagination 컴포넌트 사용이 적절합니다.이전 리뷰 피드백에 따라 shadcn Pagination 컴포넌트를 사용한 것이 좋습니다.
-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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/app/_components/ui/pagination.jsx (1)
7-16
: 네비게이션 컴포넌트의 접근성 개선이 필요합니다.현재 구현에 기본적인 ARIA 속성이 포함되어 있지만, 페이지네이션의 현재 상태를 더 명확하게 전달하기 위해 추가적인 ARIA 속성이 필요합니다.
<nav role="navigation" aria-label="pagination" + aria-current={`페이지 ${currentPage}/${totalPages}`} className={cn("mx-auto flex w-full justify-center", className)} {...props} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/_components/ui/pagination.jsx
(1 hunks)src/app/post/page.js
(3 hunks)
🔇 Additional comments (3)
src/app/_components/ui/pagination.jsx (3)
19-24
: 목록의 접근성을 개선해야 합니다.시맨틱 HTML을 잘 사용했지만, 목록의 접근성을 위한 속성이 누락되었습니다.
<ul ref={ref} + role="list" + aria-label="페이지 목록" className={cn("flex flex-row items-center gap-1", className)} {...props} />
32-45
: 키보드 네비게이션 지원을 추가해야 합니다.현재 구현은 마우스 사용자에게는 잘 작동하지만, 키보드 사용자를 위한 추가적인 지원이 필요합니다.
<a aria-current={isActive ? "page" : undefined} + tabIndex={0} + role="button" + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + e.target.click(); + } + }} className={cn(buttonVariants({ variant: isActive ? "outline" : "ghost", size, }), className)} {...props} />
78-89
: 접근성이 잘 구현되었습니다!
aria-hidden
과sr-only
클래스를 적절히 사용하여 스크린 리더 사용자를 위한 접근성이 잘 구현되었습니다.
- currentPage: 현재페이지를 반환 - totalPages: 총 페이지 수를 반환 - setTotalPages: totalPages 값을 변경하는 함수. 외부에서 총 페이지 수를 설정할 수 있음 - handlePageChange: 페이지를 변경하는 함수 - goToNextPage: 다음 페이지로 이동하는 함수 - goToPreviousPage: 이전 페이지로 이동하는 함수 - getPageRange: 페이지 범위를 계산하여 반환하는 함수
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.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/_components/hooks/usePaigination.js (1)
7-23
: 페이지 네비게이션 함수에 에러 처리를 추가하면 좋겠습니다.페이지 변경 시 발생할 수 있는 예외 상황에 대한 처리가 필요합니다. 예를 들어:
- 유효하지 않은 페이지 번호 입력에 대한 처리
- 페이지 변경 실패 시 사용자에게 피드백 제공
const handlePageChange = (page) => { + if (typeof page !== 'number' || isNaN(page)) { + console.error('유효하지 않은 페이지 번호입니다.'); + return; + } if (page >= 1 && page <= total) { setCurrentPage(page); + } else { + console.warn('페이지 범위를 벗어났습니다.'); } };src/app/post/page.js (1)
23-31
: 초기값 설정을 개선할 수 있습니다.
usePagination
훅의 초기값이 모두 1로 설정되어 있습니다. 페이지당 게시물 수(limit)를 고려하여 더 적절한 초기값을 설정하는 것이 좋겠습니다.- } = usePagination(1, 1); // 초기 페이지와 총 페이지 수 설정 + } = usePagination(1, 1, 5); // 초기 페이지, 총 페이지 수, 표시할 페이지 수 설정
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/_components/hooks/usePaigination.js
(1 hunks)src/app/post/page.js
(3 hunks)
🔇 Additional comments (3)
src/app/_components/hooks/usePaigination.js (1)
37-48
: 🛠️ Refactor suggestion그룹 네비게이션 함수를 반환값에 추가해야 합니다.
제안된 그룹 단위 이동 기능을 구현한다면, 해당 함수들도 반환해야 합니다.
return { currentPage, totalPages: total, setTotalPages, handlePageChange, goToNextPage, goToPreviousPage, getPageRange, + goToNextGroup, + goToPreviousGroup, };Likely invalid or redundant comment.
src/app/post/page.js (2)
96-96
: 게시물 상세 페이지 네비게이션이 잘 구현되었습니다.Next.js의
router.push
를 사용한 게시물 상세 페이지 이동 구현이 적절합니다.
33-45
:⚠️ Potential issue에러 처리를 개선해야 합니다.
현재 구현은 에러 발생 시 사용자에게 적절한 피드백을 제공하지 않으며, 로딩 상태 처리가 누락되어 있습니다.
const fetchPosts = async (page) => { + const [error, setError] = useState(null); setLoading(true); try { const response = await fetch( `${process.env.NEXT_PUBLIC_BASE_URL}/posts?page=${page}&limit=6&categories=NOTICE` ); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } const notice = await response.json(); setPosts(notice.data); setTotalPages(notice.meta.totalPages); + setError(null); } catch (error) { + setError('게시물을 불러오는 중 오류가 발생했습니다.'); + console.error('Failed to fetch posts:', error); + setPosts([]); } finally { setLoading(false); } };에러 상태를 표시하는 UI도 추가해주세요:
+ if (error) { + return ( + <div className="text-red-500 text-center mt-4"> + <p>{error}</p> + <Button + onClick={() => fetchPosts(currentPage)} + className="mt-2" + > + 다시 시도 + </Button> + </div> + ); + }Likely invalid or redundant 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.
수정해야할꺼 하나 더 남겨두었습니다 ㅎ 그것만 반영하고 머지해주셔도 좋을것 같아요
* docs: 디렉토리구조 재구성 및 문서화 -디렉토리 구조를 최적화하였습니다 * refactor: usePagination 오타수정 * refactor: 오타수정
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/_components/hooks/usePagination.js (1)
3-6
: 헤이 헤이 Chill... 페이지네이션 훅의 매개변수를 개선해보는게 어떨까요visiblePageCount가 getPageRange에서만 사용되는데, 이걸 좀 더 chill하게 활용할 수 있을 것 같아요. 페이지 그룹 크기로도 활용하면 좋을 것 같네요.
-const usePagination = (initialPage = 1, totalPages = 1, visiblePageCount=5) => { +const usePagination = ({ + initialPage = 1, + totalPages = 1, + pageGroupSize = 5 +} = {}) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md
(1 hunks)src/app/_components/hooks/usePagination.js
(1 hunks)src/app/_components/sections/heroSection.js
(1 hunks)src/app/layout.js
(1 hunks)src/app/page.js
(1 hunks)src/app/post/page.js
(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/app/layout.js
- src/app/page.js
🔇 Additional comments (4)
src/app/_components/sections/heroSection.js (1)
7-7
: 헤이 헤이 Chill... 임포트 경로 변경이 잘 되었네요프로젝트 구조 변경에 맞춰 경로가 잘 수정되었어요. chill chill 맞은 코드네요.
README.md (1)
95-111
: 헤이 헤이 Chill... 프로젝트 구조가 깔끔하게 정리되었네요새로운 디렉토리 구조가 잘 문서화되었어요. 특히 _components 아래 hooks, sections, ui로 나눈 구조가 chill하네요.
src/app/post/page.js (2)
33-45
: 헤이 헤이 Chill... 에러 처리를 추가하면 좋을 것 같아요에러가 발생했을 때 사용자에게 알려주는 게 더 chill할 것 같아요.
const fetchPosts = async (page) => { setLoading(true); + try { const response = await fetch( `${process.env.NEXT_PUBLIC_BASE_URL}/posts?page=${page}&limit=6&categories=NOTICE` ); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } const notice = await response.json(); setPosts(notice.data); setTotalPages(notice.meta.totalPages); + } catch (error) { + console.error('Failed to fetch posts:', error); + alert('게시물을 불러오는데 실패했어요. chill하게 다시 시도해보세요.'); + } setLoading(false); };
107-133
: 헤이 헤이 Chill... 페이지네이션 UI를 그룹 이동 방식으로 수정해야 할 것 같아요PR 목표에 맞게 5개 단위로 페이지를 이동할 수 있도록 수정이 필요해요. chill하게 수정해보죠.
<Pagination className="mt-6"> <PaginationContent> <PaginationPrevious - onClick={goToPreviousPage} + onClick={goToPreviousGroup} disabled={currentPage === 1} /> {getPageRange().map((page) => ( <PaginationItem key={page}> <PaginationLink isActive={currentPage === page} onClick={() => handlePageChange(page)} > {page} </PaginationLink> </PaginationItem> ))} <PaginationNext - onClick={goToNextPage} + onClick={goToNextGroup} disabled={currentPage === totalPages} /> </PaginationContent> </Pagination>
= 사용시 주석내용처럼 호출
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/_components/hooks/usePagination.js (3)
3-6
: 헤이 헤이 Chill... 파라미터 이름을 좀 더 명확하게 해보면 어떨까요visiblePageCount는 chill한 이름인데요, initialPage와 totalPages는 좀 더 명확하게 initialCurrentPage와 initialTotalPages로 바꾸면 더 chill할 것 같아요.
-const usePagination = (initialPage = 1, totalPages = 1, visiblePageCount=5) => { +const usePagination = (initialCurrentPage = 1, initialTotalPages = 1, visiblePageCount=5) => { - const [currentPage, setCurrentPage] = useState(initialPage); - const [total, setTotalPages] = useState(totalPages); + const [currentPage, setCurrentPage] = useState(initialCurrentPage); + const [total, setTotalPages] = useState(initialTotalPages);
25-35
: 헤이 헤이 Chill... 페이지 범위 계산 로직에 주석을 추가하면 좋을 것 같아요로직은 chill하게 잘 작성되어 있는데, 다른 개발자들을 위해 계산 방식을 설명하는 주석을 추가하면 더 좋을 것 같아요.
const getPageRange = () => { + // 현재 페이지를 중심으로 양쪽에 표시할 페이지 수를 계산 + // 예: visiblePageCount가 5이고 currentPage가 3일 때 + // 표시되는 페이지: [1, 2, 3, 4, 5] const range = []; const left = Math.max(1, currentPage - Math.floor(visiblePageCount / 2)); const right = Math.min(total, left + visiblePageCount - 1);
54-65
: 헤이 헤이 Chill... 반환 객체의 주석도 영어로 통일해주세요반환 객체는 chill하게 잘 구성되어 있어요. 다만 주석을 영어로 바꾸면 더 좋을 것 같아요.
return { currentPage, totalPages: total, setTotalPages, handlePageChange, goToNextPage, goToPreviousPage, getPageRange, - //그룹이동 함수 사용시 호출 + // Group navigation functions goToNextGroup, goToPreviousGroup, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/_components/hooks/usePagination.js
(1 hunks)
🔇 Additional comments (1)
src/app/_components/hooks/usePagination.js (1)
7-23
: 헤이 헤이 Chill... 기본 네비게이션 함수들이 잘 구현되어 있네요경계값 체크도 잘되어있고 로직도 chill chill 맞게 작성되어 있어요.
Summary by CodeRabbit
새로운 기능
버그 수정
기타 변경사항