-
-
Notifications
You must be signed in to change notification settings - Fork 753
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: added pagination to bolgs #3579
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces pagination functionality to the blog index page. The changes involve adding state variables for tracking the current page, page range, and posts per page. New methods are implemented to handle page navigation, including changing pages, moving to previous and next pages. The rendering logic is updated to display a subset of blog posts based on the current page, and a pagination control is added to allow users to navigate through different pages of blog posts. Changes
Possibly related issues
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
Documentation and Community
|
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3579--asyncapi-website.netlify.app/ |
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)
pages/blog/index.tsx (3)
38-40
: Consider performance optimizations and code organization improvements
- Move
postsPerPage
to a constants file for reusability across components.- Consider using
useMemo
for derived state calculations.+ import { POSTS_PER_PAGE } from '@/constants/pagination'; - const postsPerPage = 12; + const postsPerPage = POSTS_PER_PAGE; + const totalPages = useMemo(() => Math.ceil(posts.length / postsPerPage), [posts.length, postsPerPage]);
88-90
: Optimize post slicing with memoizationThe current posts calculation should be memoized to prevent unnecessary recalculations on re-renders.
- const indexOfLastPost = currentPage * postsPerPage; - const indexOfFirstPost = indexOfLastPost - postsPerPage; - const currentPosts = posts.slice(indexOfFirstPost, indexOfLastPost); + const currentPosts = useMemo(() => { + const indexOfLastPost = currentPage * postsPerPage; + const indexOfFirstPost = indexOfLastPost - postsPerPage; + return posts.slice(indexOfFirstPost, indexOfLastPost); + }, [posts, currentPage, postsPerPage]);
155-155
: Add key prop based on unique identifierUsing array index as key prop might cause issues with React's reconciliation process if posts are reordered or filtered.
- {currentPosts.map((post, index) => ( - <BlogPostItem key={index} post={post} /> - ))} + {currentPosts.map((post) => ( + <BlogPostItem key={post.slug || post.id} post={post} /> + ))}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/blog/index.tsx
(4 hunks)
🧰 Additional context used
🪛 eslint
pages/blog/index.tsx
[error] 71-71: Expected blank line after variable declarations.
(newline-after-var)
[error] 72-72: Expected blank line before this statement.
(padding-line-between-statements)
[error] 175-175: Replace {·length:·Math.min(12,·Math.ceil(posts.length·/·postsPerPage)·-·pageRange[0]·+·1)·},
with ⏎··············{·length:·Math.min(12,·Math.ceil(posts.length·/·postsPerPage)·-·pageRange[0]·+·1)·},⏎·············
(prettier/prettier)
[error] 176-176: Insert ··
(prettier/prettier)
[error] 176-176: Expected blank line after variable declarations.
(newline-after-var)
[error] 177-177: Replace ··············
with ················
(prettier/prettier)
[error] 177-185: Expected blank line before this statement.
(padding-line-between-statements)
[error] 178-178: Insert ··
(prettier/prettier)
[error] 179-179: Replace ··················
with ····················
(prettier/prettier)
[error] 180-180: Insert ··
(prettier/prettier)
[error] 181-181: Replace ··················
with ····················
(prettier/prettier)
[error] 182-182: Insert ··
(prettier/prettier)
[error] 183-183: Insert ··
(prettier/prettier)
[error] 184-184: Insert ··
(prettier/prettier)
[error] 185-185: Replace ··············
with ················
(prettier/prettier)
[error] 186-186: Replace }
with ··}⏎············
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
pages/blog/index.tsx
[error] 71-71: Expected blank line after variable declarations
[error] 72-72: Expected blank line before this statement
[warning] 169-169: Invalid Tailwind CSS classnames order
[error] 175-175: Formatting error: Incorrect spacing and line breaks
[error] 176-177: Expected blank line after variable declarations and incorrect indentation
[warning] 181-181: Invalid Tailwind CSS classnames order
[warning] 190-190: Invalid Tailwind CSS classnames order
⏰ Context from checks skipped due to timeout of 180000ms (1)
- GitHub Check: Lighthouse CI
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3579 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 732 732
=========================================
Hits 732 732 ☔ View full report in Codecov by Sentry. |
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: 0
♻️ Duplicate comments (2)
pages/blog/index.tsx (2)
65-87
: 🛠️ Refactor suggestionImprove pagination handler implementation
Building upon previous feedback:
- Add input validation
- Extract magic numbers into named constants
- Add error handling for invalid page numbers
+ const PAGE_STEP = 3; + const PAGE_WINDOW = 9; const handlePageChange = (pageNumber: number) => { + const totalPages = Math.ceil(posts.length / POSTS_PER_PAGE); + if (pageNumber < 1 || pageNumber > totalPages) { + console.error(`Invalid page number: ${pageNumber}. Must be between 1 and ${totalPages}`); + return; + } setCurrentPage(pageNumber); if (pageNumber > pageRange[1] - 1) { - setPageRange([pageRange[0] + 3, pageRange[1] + 3]); + setPageRange([pageRange[0] + PAGE_STEP, pageRange[1] + PAGE_STEP]); } else if (pageNumber < pageRange[0] + 1) { - const newStart = Math.max(pageRange[0] - 3, 1); - const newEnd = newStart + 9; + const newStart = Math.max(pageRange[0] - PAGE_STEP, 1); + const newEnd = newStart + PAGE_WINDOW; setPageRange([newStart, newEnd]); } };
167-202
: 🛠️ Refactor suggestionImprove accessibility and maintainability of pagination controls
Building upon previous feedback:
- Add keyboard navigation
- Improve ARIA labels and roles
- Extract common CSS classes
+ const paginationButtonClass = (isActive: boolean) => + `mx-1 rounded-md border px-3 py-1 ${isActive ? 'bg-gray-300' : 'bg-white'}`; - <div className="mt-8 flex justify-center"> + <nav className="mt-8 flex justify-center" role="navigation" aria-label="Pagination"> <button onClick={handlePrevious} - className={`mx-1 rounded-md border px-3 py-1 ${currentPage === 1 ? 'bg-gray-300' : 'bg-white'}`} + className={paginationButtonClass(currentPage === 1)} + aria-label="Go to previous page" disabled={currentPage === 1} + onKeyDown={(e) => e.key === 'Enter' && handlePrevious()} > Previous </button> {/* ... */} <button key={pageNumber} onClick={() => handlePageChange(pageNumber)} - className={`mx-1 rounded-md border px-3 py-1 ${currentPage === pageNumber ? 'bg-gray-300' : 'bg-white'}`} + className={paginationButtonClass(currentPage === pageNumber)} + aria-label={`Go to page ${pageNumber}`} + aria-current={currentPage === pageNumber ? 'page' : undefined} + onKeyDown={(e) => e.key === 'Enter' && handlePageChange(pageNumber)} > {pageNumber} </button> {/* ... */} - </div> + </nav>
🧹 Nitpick comments (2)
pages/blog/index.tsx (2)
38-40
: Improve state management implementation
- Convert
postsPerPage
to a named constant- Add TypeScript types for
pageRange
state+ const POSTS_PER_PAGE = 12; + const INITIAL_PAGE_RANGE: [number, number] = [1, 10]; const [currentPage, setCurrentPage] = useState(1); - const [pageRange, setPageRange] = useState([1, 10]); + const [pageRange, setPageRange] = useState<[number, number]>(INITIAL_PAGE_RANGE); - const postsPerPage = 12;
89-91
: Optimize pagination calculations with memoizationThe current implementation recalculates these values on every render. Consider using
useMemo
to optimize performance.- const indexOfLastPost = currentPage * postsPerPage; - const indexOfFirstPost = indexOfLastPost - postsPerPage; - const currentPosts = posts.slice(indexOfFirstPost, indexOfLastPost); + const currentPosts = useMemo(() => { + const indexOfLastPost = currentPage * POSTS_PER_PAGE; + const indexOfFirstPost = indexOfLastPost - POSTS_PER_PAGE; + return posts.slice(indexOfFirstPost, indexOfLastPost); + }, [currentPage, posts, POSTS_PER_PAGE]);
Description
Related issue(s)
Summary by CodeRabbit