feat: added pagination in blog page #3591
Conversation
Adi-204
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe pull request introduces pagination functionality to the blog page by implementing a new Changes
Assessment against linked issues
Suggested reviewers
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
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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. |
|
Hello, @coderabbitai[bot]! 👋🏼 |
|
@asyncapi-bot: i'll review the changes in this pr that adds pagination to the blog page. ✅ Actions performedReview triggered.
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3591--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
components/navigation/BlogPagination.tsx (1)
24-58: Improve calculatePageNumbers maintainability.The pagination logic is complex and could benefit from being broken down into smaller, more maintainable functions.
+ const addSequentialPages = (start: number, end: number) => { + for (let i = start; i <= end; i++) { + pageNumbers.push(i); + } + }; + + const addEllipsisAndEnd = (endPages: number) => { + pageNumbers.push('...'); + addSequentialPages(totalPages - endPages + 1, totalPages); + }; + const calculatePageNumbers = () => { const pageNumbers: (number | string)[] = []; if (totalPages <= pagesToShow) { - for (let i = 1; i <= totalPages; i++) { - pageNumbers.push(i); - } + addSequentialPages(1, totalPages); } else { if (currentPage <= 2) { - for (let i = 1; i <= 3; i++) { - pageNumbers.push(i); - } - pageNumbers.push('...'); - pageNumbers.push(totalPages - 2); - pageNumbers.push(totalPages - 1); - pageNumbers.push(totalPages); + addSequentialPages(1, 3); + addEllipsisAndEnd(3); } else if (currentPage >= totalPages - 1) { // ... rest of the logic🧰 Tools
🪛 eslint
[error] 24-24: Delete
··(prettier/prettier)
[error] 25-25: Replace
········with····(prettier/prettier)
[error] 25-25: Expected blank line after variable declarations.
(newline-after-var)
[error] 25-25: 'pageNumbers' is already declared in the upper scope on line 22 column 12.
(@typescript-eslint/no-shadow)
[error] 26-26: Replace
········with····(prettier/prettier)
[error] 26-56: Expected blank line before this statement.
(padding-line-between-statements)
[error] 27-27: Delete
······(prettier/prettier)
[error] 28-28: Delete
········(prettier/prettier)
[error] 29-29: Delete
······(prettier/prettier)
[error] 30-30: Replace
········with····(prettier/prettier)
[error] 31-31: Delete
······(prettier/prettier)
[error] 31-55: Unexpected if as the only statement in an else block.
(no-lonely-if)
[error] 32-32: Replace
················with········(prettier/prettier)
[error] 33-33: Replace
····················with··········(prettier/prettier)
[error] 34-34: Delete
········(prettier/prettier)
[error] 35-35: Replace
················with········(prettier/prettier)
[error] 36-36: Delete
········(prettier/prettier)
[error] 37-37: Replace
················with········(prettier/prettier)
[error] 38-38: Replace
················with········(prettier/prettier)
[error] 39-39: Replace
············with······(prettier/prettier)
[error] 40-40: Delete
········(prettier/prettier)
[error] 41-41: Replace
················with········(prettier/prettier)
[error] 42-42: Delete
········(prettier/prettier)
[error] 43-43: Delete
········(prettier/prettier)
[error] 44-44: Replace
················with········(prettier/prettier)
[error] 45-45: Delete
··········(prettier/prettier)
[error] 46-46: Delete
········(prettier/prettier)
[error] 47-47: Replace
············with······(prettier/prettier)
[error] 48-48: Replace
················with········(prettier/prettier)
[error] 49-49: Replace
················with········(prettier/prettier)
[error] 50-50: Replace
················with········(prettier/prettier)
[error] 51-51: Replace
················with········(prettier/prettier)
[error] 52-52: Replace
················with········(prettier/prettier)
[error] 53-53: Delete
········(prettier/prettier)
[error] 54-54: Replace
················with········(prettier/prettier)
[error] 55-55: Delete
······(prettier/prettier)
[error] 56-56: Replace
········with····(prettier/prettier)
[error] 57-57: Delete
····(prettier/prettier)
[error] 57-57: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Replace
····with··(prettier/prettier)
pages/blog/index.tsx (1)
Line range hint
166-188: Add loading state for pagination transitions.The current implementation might cause a jarring experience during page transitions as posts are loaded.
- {Object.keys(posts).length > 0 && isClient && ( + {Object.keys(posts).length > 0 && isClient && currentPosts.length > 0 && ( <ul className='mx-auto mt-12 grid max-w-lg gap-5 lg:max-w-none lg:grid-cols-3'> {currentPosts.map((post, index) => ( <BlogPostItem key={index} post={post} /> ))} </ul> )} - {Object.keys(posts).length > 0 && !isClient && ( + {(Object.keys(posts).length > 0 && !isClient) || (isClient && currentPosts.length === 0) && ( <div className='h-screen w-full'> <Loader loaderText='Loading Blogs' className='mx-auto my-60' pulsating /> </div> )}🧰 Tools
🪛 eslint
[error] 166-166: Replace
(Object.keys(posts).length·===·0)withObject.keys(posts).length·===·0(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/helpers/applyFilter.ts(1 hunks)components/navigation/BlogPagination.tsx(1 hunks)pages/blog/index.tsx(6 hunks)
🧰 Additional context used
🪛 eslint
pages/blog/index.tsx
[error] 2-2: 'useRef' is defined but never used.
(unused-imports/no-unused-imports)
[error] 2-2: 'useRef' is defined but never used.
(no-unused-vars)
[error] 1-16: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 27-27: Insert ··
(prettier/prettier)
[error] 28-28: Insert ··
(prettier/prettier)
[error] 30-30: Insert ··
(prettier/prettier)
[error] 31-31: Insert ··
(prettier/prettier)
[error] 33-33: Insert ··
(prettier/prettier)
[error] 34-34: Replace ······ with ········
(prettier/prettier)
[error] 55-55: Expected blank line after variable declarations.
(newline-after-var)
[error] 55-55: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 55-55: Delete ·
(prettier/prettier)
[error] 56-65: Expected blank line before this statement.
(padding-line-between-statements)
[error] 59-59: Delete ,·
(prettier/prettier)
[error] 59-59: Unexpected trailing comma.
(comma-dangle)
[error] 59-59: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 63-63: Delete ,
(prettier/prettier)
[error] 63-63: Unexpected trailing comma.
(comma-dangle)
[error] 75-91: Expected blank line after variable declarations.
(newline-after-var)
[error] 77-80: Expected blank line after variable declarations.
(newline-after-var)
[error] 81-90: Expected blank line before this statement.
(padding-line-between-statements)
[error] 86-86: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 86-86: Delete ·
(prettier/prettier)
[error] 92-109: Expected blank line before this statement.
(padding-line-between-statements)
[error] 93-93: Expected space(s) after "if".
(keyword-spacing)
[error] 93-93: Replace (router.isReady) with ·(router.isReady)·
(prettier/prettier)
[error] 93-108: Missing space before opening brace.
(space-before-blocks)
[error] 94-94: Expected blank line after variable declarations.
(newline-after-var)
[error] 94-94: Missing radix parameter.
(radix)
[error] 95-104: Expected blank line before this statement.
(padding-line-between-statements)
[error] 95-95: Expected space(s) after "if".
(keyword-spacing)
[error] 95-95: Replace (!pageFromQuery) with ·(!pageFromQuery)·
(prettier/prettier)
[error] 95-104: Missing space before opening brace.
(space-before-blocks)
[error] 99-99: Delete ,
(prettier/prettier)
[error] 99-99: Unexpected trailing comma.
(comma-dangle)
[error] 106-106: Expected blank line after variable declarations.
(newline-after-var)
[error] 107-107: Expected blank line before this statement.
(padding-line-between-statements)
[error] 166-166: Replace (Object.keys(posts).length·===·0) with Object.keys(posts).length·===·0
(prettier/prettier)
[error] 184-188: Replace ⏎··············blogsPerPage={blogsPerPage}⏎··············totalBlogs={posts.length}⏎··············paginate={paginate}⏎··········· with ·blogsPerPage={blogsPerPage}·totalBlogs={posts.length}·paginate={paginate}
(prettier/prettier)
components/navigation/BlogPagination.tsx
[error] 1-7: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 3-3: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 7-7: @/types/components/buttons/ButtonPropsType import should occur before import of ../icons/ArrowRight
(import/order)
[error] 9-10: Delete ⏎
(prettier/prettier)
[error] 9-10: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Replace ···· with ··
(prettier/prettier)
[error] 13-13: Delete ··
(prettier/prettier)
[error] 17-17: Delete ··
(prettier/prettier)
[error] 18-18: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 19-19: Missing radix parameter.
(radix)
[error] 20-20: Replace ···· with ··
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 22-22: Replace ···· with ··
(prettier/prettier)
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Replace ········ with ····
(prettier/prettier)
[error] 25-25: Expected blank line after variable declarations.
(newline-after-var)
[error] 25-25: 'pageNumbers' is already declared in the upper scope on line 22 column 12.
(@typescript-eslint/no-shadow)
[error] 26-26: Replace ········ with ····
(prettier/prettier)
[error] 26-56: Expected blank line before this statement.
(padding-line-between-statements)
[error] 27-27: Delete ······
(prettier/prettier)
[error] 28-28: Delete ········
(prettier/prettier)
[error] 29-29: Delete ······
(prettier/prettier)
[error] 30-30: Replace ········ with ····
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 31-55: Unexpected if as the only statement in an else block.
(no-lonely-if)
[error] 32-32: Replace ················ with ········
(prettier/prettier)
[error] 33-33: Replace ···················· with ··········
(prettier/prettier)
[error] 34-34: Delete ········
(prettier/prettier)
[error] 35-35: Replace ················ with ········
(prettier/prettier)
[error] 36-36: Delete ········
(prettier/prettier)
[error] 37-37: Replace ················ with ········
(prettier/prettier)
[error] 38-38: Replace ················ with ········
(prettier/prettier)
[error] 39-39: Replace ············ with ······
(prettier/prettier)
[error] 40-40: Delete ········
(prettier/prettier)
[error] 41-41: Replace ················ with ········
(prettier/prettier)
[error] 42-42: Delete ········
(prettier/prettier)
[error] 43-43: Delete ········
(prettier/prettier)
[error] 44-44: Replace ················ with ········
(prettier/prettier)
[error] 45-45: Delete ··········
(prettier/prettier)
[error] 46-46: Delete ········
(prettier/prettier)
[error] 47-47: Replace ············ with ······
(prettier/prettier)
[error] 48-48: Replace ················ with ········
(prettier/prettier)
[error] 49-49: Replace ················ with ········
(prettier/prettier)
[error] 50-50: Replace ················ with ········
(prettier/prettier)
[error] 51-51: Replace ················ with ········
(prettier/prettier)
[error] 52-52: Replace ················ with ········
(prettier/prettier)
[error] 53-53: Delete ········
(prettier/prettier)
[error] 54-54: Replace ················ with ········
(prettier/prettier)
[error] 55-55: Delete ······
(prettier/prettier)
[error] 56-56: Replace ········ with ····
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 57-57: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Replace ···· with ··
(prettier/prettier)
[error] 60-60: Delete ··
(prettier/prettier)
[error] 61-61: Delete ····
(prettier/prettier)
[error] 62-62: Delete ··
(prettier/prettier)
[error] 64-64: Delete ··
(prettier/prettier)
[error] 65-65: Delete ····
(prettier/prettier)
[error] 66-66: Replace ············ with ······
(prettier/prettier)
[error] 67-67: Delete ········
(prettier/prettier)
[error] 68-68: Replace ················ with ········
(prettier/prettier)
[error] 69-69: Delete ········
(prettier/prettier)
[error] 70-70: Replace ················ with ········
(prettier/prettier)
[error] 71-71: Delete ········
(prettier/prettier)
[error] 72-72: Delete ········
(prettier/prettier)
[error] 73-73: Delete ········
(prettier/prettier)
[error] 74-74: Delete ········
(prettier/prettier)
[error] 75-75: Replace ············ with ······
(prettier/prettier)
[error] 76-76: Delete ······
(prettier/prettier)
[error] 77-77: Replace ················ with ········
(prettier/prettier)
[error] 78-78: Delete ··········
(prettier/prettier)
[error] 79-79: Replace ························ with ············
(prettier/prettier)
[error] 80-80: Delete ············
(prettier/prettier)
[error] 81-81: Delete ············
(prettier/prettier)
[error] 82-82: Replace ························ with ············
(prettier/prettier)
[error] 83-83: Delete ··········
(prettier/prettier)
[error] 84-84: Delete ············
(prettier/prettier)
[error] 85-85: Replace ···················· with ··········
(prettier/prettier)
[error] 86-86: Replace ················ with ········
(prettier/prettier)
[error] 87-87: Replace ············ with ······
(prettier/prettier)
[error] 88-88: Delete ······
(prettier/prettier)
[error] 89-89: Delete ········
(prettier/prettier)
[error] 90-90: Replace ················ with ········
(prettier/prettier)
[error] 91-91: Delete ········
(prettier/prettier)
[error] 92-92: Replace ················ with ········
(prettier/prettier)
[error] 93-93: Delete ········
(prettier/prettier)
[error] 94-94: Replace ················ with ········
(prettier/prettier)
[error] 95-95: Delete ········
(prettier/prettier)
[error] 96-96: Replace ················ with ········
(prettier/prettier)
[error] 97-97: Delete ······
(prettier/prettier)
[error] 98-98: Replace ········ with ····
(prettier/prettier)
[error] 99-99: Delete ··
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
pages/blog/index.tsx
[error] 1-1: Import sorting issues detected. Run autofix to sort imports.
[error] 2-2: Unused import: 'useRef' is defined but never used
[error] 27-99: Multiple formatting issues: incorrect indentation, missing blank lines, and trailing spaces
components/navigation/BlogPagination.tsx
[error] 1-3: Multiple issues with imports: duplicate React imports and incorrect import ordering
[warning] 65-95: Multiple Tailwind CSS issues: invalid classname order and non-usage of available shorthand properties (e.g., 'size-4' instead of 'w-4, h-4')
[warning] 16-16: Missing JSDoc comment
🔇 Additional comments (2)
components/helpers/applyFilter.ts (1)
140-142: LGTM! Good separation of pagination from filtering logic.The addition of the page property check ensures that pagination state is preserved during filtering operations.
components/navigation/BlogPagination.tsx (1)
1-23: 🛠️ Refactor suggestionClean up imports and improve type safety.
- Remove unused
useRefimport- Add type safety for the page query parameter
-import React, { useState, useEffect } from 'react'; -import { useRouter } from 'next/router'; +import { useRouter } from 'next/router'; +import React, { useState, useEffect } from 'react'; +type PageQuery = { + page?: string; +};Likely invalid or redundant comment.
🧰 Tools
🪛 eslint
[error] 1-7: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 3-3: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 7-7:
@/types/components/buttons/ButtonPropsTypeimport should occur before import of../icons/ArrowRight(import/order)
[error] 9-10: Delete
⏎(prettier/prettier)
[error] 9-10: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 11-11: Delete
··(prettier/prettier)
[error] 12-12: Replace
····with··(prettier/prettier)
[error] 13-13: Delete
··(prettier/prettier)
[error] 17-17: Delete
··(prettier/prettier)
[error] 18-18: Delete
··(prettier/prettier)
[error] 19-19: Delete
··(prettier/prettier)
[error] 19-19: Missing radix parameter.
(radix)
[error] 20-20: Replace
····with··(prettier/prettier)
[error] 21-21: Delete
··(prettier/prettier)
[error] 22-22: Replace
····with··(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 1-3: Multiple issues with imports: duplicate React imports and incorrect import ordering
[warning] 16-16: Missing JSDoc comment
| const currentPage: number = parseInt(page as string); | ||
| const totalPages: number = Math.ceil(totalBlogs / blogsPerPage); |
There was a problem hiding this comment.
Add input validation for currentPage.
The parseInt call is missing a radix parameter and there's no validation for NaN values.
- const currentPage: number = parseInt(page as string);
+ const currentPage: number = Number.isNaN(parseInt(page as string, 10)) ? 1 : parseInt(page as string, 10);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currentPage: number = parseInt(page as string); | |
| const totalPages: number = Math.ceil(totalBlogs / blogsPerPage); | |
| const currentPage: number = Number.isNaN(parseInt(page as string, 10)) ? 1 : parseInt(page as string, 10); | |
| const totalPages: number = Math.ceil(totalBlogs / blogsPerPage); |
🧰 Tools
🪛 eslint
[error] 19-19: Delete ··
(prettier/prettier)
[error] 19-19: Missing radix parameter.
(radix)
[error] 20-20: Replace ···· with ··
(prettier/prettier)
| <div className='flex justify-center items-center p-4 gap-2 mt-8'> | ||
| <Button | ||
| className={`${currentPage === 1 && 'opacity-50 cursor-not-allowed'} w-[120px] h-[35px] py-2 px-4 rounded-l-md`} | ||
| bgClassName='bg-white' | ||
| textClassName='text-[#212525] font-inter text-[14px] font-normal' | ||
| text='Previous' | ||
| disabled={currentPage === 1} | ||
| onClick={() => paginate(currentPage - 1)} | ||
| icon={<IconArrowLeft className='w-4 h-4 inline-block' />} | ||
| iconPosition={ButtonIconPosition.LEFT} | ||
| /> | ||
| <div className='flex justify-center gap-3 w-[35vw]'> | ||
| {pageNumbers.map((number, index) => ( | ||
| <button | ||
| key={index} | ||
| className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B'}`} | ||
| onClick={() => typeof number === 'number' && paginate(number)} | ||
| disabled={number === '...'} | ||
| > | ||
| {number} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| <Button | ||
| className={`${currentPage === totalPages && 'opacity-50 cursor-not-allowed'} w-[120px] h-[35px] py-2 px-4 rounded-l-md`} | ||
| bgClassName='bg-white' | ||
| textClassName='text-[#212525] font-inter text-[14px] font-normal' | ||
| text='Next' | ||
| disabled={currentPage === totalPages} | ||
| onClick={() => paginate(currentPage + 1)} | ||
| icon={<IconArrowRight className='w-4 h-4 inline-block' />} | ||
| iconPosition={ButtonIconPosition.RIGHT} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize Tailwind classes and improve accessibility.
- The pagination buttons need ARIA labels for screen readers
- Tailwind classes can be optimized
- <div className='flex justify-center items-center p-4 gap-2 mt-8'>
+ <nav aria-label="Blog pagination" className='flex justify-center items-center p-4 gap-2 mt-8'>
<Button
- className={`${currentPage === 1 && 'opacity-50 cursor-not-allowed'} w-[120px] h-[35px] py-2 px-4 rounded-l-md`}
+ className={`${currentPage === 1 ? 'opacity-50 cursor-not-allowed' : ''} size-[120px] py-2 px-4 rounded-l-md`}
+ aria-label="Previous page"
// ... rest of the props
/>
<div className='flex justify-center gap-3 w-[35vw]'>
{pageNumbers.map((number, index) => (
<button
key={index}
+ aria-label={`${typeof number === 'number' ? `Go to page ${number}` : 'More pages'}`}
+ aria-current={number === currentPage ? 'page' : undefined}
// ... rest of the props
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className='flex justify-center items-center p-4 gap-2 mt-8'> | |
| <Button | |
| className={`${currentPage === 1 && 'opacity-50 cursor-not-allowed'} w-[120px] h-[35px] py-2 px-4 rounded-l-md`} | |
| bgClassName='bg-white' | |
| textClassName='text-[#212525] font-inter text-[14px] font-normal' | |
| text='Previous' | |
| disabled={currentPage === 1} | |
| onClick={() => paginate(currentPage - 1)} | |
| icon={<IconArrowLeft className='w-4 h-4 inline-block' />} | |
| iconPosition={ButtonIconPosition.LEFT} | |
| /> | |
| <div className='flex justify-center gap-3 w-[35vw]'> | |
| {pageNumbers.map((number, index) => ( | |
| <button | |
| key={index} | |
| className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B'}`} | |
| onClick={() => typeof number === 'number' && paginate(number)} | |
| disabled={number === '...'} | |
| > | |
| {number} | |
| </button> | |
| ))} | |
| </div> | |
| <Button | |
| className={`${currentPage === totalPages && 'opacity-50 cursor-not-allowed'} w-[120px] h-[35px] py-2 px-4 rounded-l-md`} | |
| bgClassName='bg-white' | |
| textClassName='text-[#212525] font-inter text-[14px] font-normal' | |
| text='Next' | |
| disabled={currentPage === totalPages} | |
| onClick={() => paginate(currentPage + 1)} | |
| icon={<IconArrowRight className='w-4 h-4 inline-block' />} | |
| iconPosition={ButtonIconPosition.RIGHT} | |
| /> | |
| </div> | |
| <nav aria-label="Blog pagination" className='flex justify-center items-center p-4 gap-2 mt-8'> | |
| <Button | |
| className={`${currentPage === 1 ? 'opacity-50 cursor-not-allowed' : ''} size-[120px] py-2 px-4 rounded-l-md`} | |
| aria-label="Previous page" | |
| bgClassName='bg-white' | |
| textClassName='text-[#212525] font-inter text-[14px] font-normal' | |
| text='Previous' | |
| disabled={currentPage === 1} | |
| onClick={() => paginate(currentPage - 1)} | |
| icon={<IconArrowLeft className='w-4 h-4 inline-block' />} | |
| iconPosition={ButtonIconPosition.LEFT} | |
| /> | |
| <div className='flex justify-center gap-3 w-[35vw]'> | |
| {pageNumbers.map((number, index) => ( | |
| <button | |
| key={index} | |
| aria-label={`${typeof number === 'number' ? `Go to page ${number}` : 'More pages'}`} | |
| aria-current={number === currentPage ? 'page' : undefined} | |
| className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B'}`} | |
| onClick={() => typeof number === 'number' && paginate(number)} | |
| disabled={number === '...'} | |
| > | |
| {number} | |
| </button> | |
| ))} | |
| </div> | |
| <Button | |
| className={`${currentPage === totalPages && 'opacity-50 cursor-not-allowed'} w-[120px] h-[35px] py-2 px-4 rounded-l-md`} | |
| bgClassName='bg-white' | |
| textClassName='text-[#212525] font-inter text-[14px] font-normal' | |
| text='Next' | |
| disabled={currentPage === totalPages} | |
| onClick={() => paginate(currentPage + 1)} | |
| icon={<IconArrowRight className='w-4 h-4 inline-block' />} | |
| iconPosition={ButtonIconPosition.RIGHT} | |
| /> | |
| </nav> |
🧰 Tools
🪛 eslint
[error] 65-65: Delete ····
(prettier/prettier)
[error] 66-66: Replace ············ with ······
(prettier/prettier)
[error] 67-67: Delete ········
(prettier/prettier)
[error] 68-68: Replace ················ with ········
(prettier/prettier)
[error] 69-69: Delete ········
(prettier/prettier)
[error] 70-70: Replace ················ with ········
(prettier/prettier)
[error] 71-71: Delete ········
(prettier/prettier)
[error] 72-72: Delete ········
(prettier/prettier)
[error] 73-73: Delete ········
(prettier/prettier)
[error] 74-74: Delete ········
(prettier/prettier)
[error] 75-75: Replace ············ with ······
(prettier/prettier)
[error] 76-76: Delete ······
(prettier/prettier)
[error] 77-77: Replace ················ with ········
(prettier/prettier)
[error] 78-78: Delete ··········
(prettier/prettier)
[error] 79-79: Replace ························ with ············
(prettier/prettier)
[error] 80-80: Delete ············
(prettier/prettier)
[error] 81-81: Delete ············
(prettier/prettier)
[error] 82-82: Replace ························ with ············
(prettier/prettier)
[error] 83-83: Delete ··········
(prettier/prettier)
[error] 84-84: Delete ············
(prettier/prettier)
[error] 85-85: Replace ···················· with ··········
(prettier/prettier)
[error] 86-86: Replace ················ with ········
(prettier/prettier)
[error] 87-87: Replace ············ with ······
(prettier/prettier)
[error] 88-88: Delete ······
(prettier/prettier)
[error] 89-89: Delete ········
(prettier/prettier)
[error] 90-90: Replace ················ with ········
(prettier/prettier)
[error] 91-91: Delete ········
(prettier/prettier)
[error] 92-92: Replace ················ with ········
(prettier/prettier)
[error] 93-93: Delete ········
(prettier/prettier)
[error] 94-94: Replace ················ with ········
(prettier/prettier)
[error] 95-95: Delete ········
(prettier/prettier)
[error] 96-96: Replace ················ with ········
(prettier/prettier)
[error] 97-97: Delete ······
(prettier/prettier)
[error] 98-98: Replace ········ with ····
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[warning] 65-95: Multiple Tailwind CSS issues: invalid classname order and non-usage of available shorthand properties (e.g., 'size-4' instead of 'w-4, h-4')
pages/blog/index.tsx
Outdated
| const blogsPerPage = 9; | ||
|
|
||
| const [currentPosts, setCurrentPosts] = useState<IBlogPost[]>([]); | ||
|
|
||
| const paginate = (pageNumber: number) => { | ||
| const { query } = router; | ||
| const newQuery = { | ||
| ...query, | ||
| page: pageNumber | ||
| }; | ||
| router.push( | ||
| { | ||
| pathname: router.pathname, | ||
| query: newQuery | ||
| }, | ||
| undefined, | ||
| { | ||
| shallow: true | ||
| } | ||
| ); | ||
| }; | ||
| useEffect(() => { | ||
| if(router.isReady){ | ||
| const pageFromQuery = parseInt(router.query.page as string); | ||
| if(!pageFromQuery){ | ||
| router.replace( | ||
| { | ||
| pathname: router.pathname, | ||
| query: { ...router.query, page: '1' }, | ||
| }, | ||
| undefined, | ||
| { shallow: true } | ||
| ); | ||
| } | ||
| const indexOfLastPost = pageFromQuery * blogsPerPage; | ||
| const indexOfFirstPost = indexOfLastPost - blogsPerPage; | ||
| setCurrentPosts(posts.slice(indexOfFirstPost, indexOfLastPost)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve pagination state management and performance.
- The posts slicing logic runs on every query change
- Missing error handling for invalid page numbers
- The pagination state could be managed more efficiently
Consider using a custom hook to manage pagination state:
function usePagination<T>(items: T[], itemsPerPage: number) {
const [currentPage, setCurrentPage] = useState(1);
const maxPage = Math.ceil(items.length / itemsPerPage);
const currentItems = useMemo(() => {
const start = (currentPage - 1) * itemsPerPage;
return items.slice(start, start + itemsPerPage);
}, [items, currentPage, itemsPerPage]);
return {
currentPage,
setCurrentPage,
currentItems,
maxPage
};
}🧰 Tools
🪛 eslint
[error] 75-91: Expected blank line after variable declarations.
(newline-after-var)
[error] 77-80: Expected blank line after variable declarations.
(newline-after-var)
[error] 81-90: Expected blank line before this statement.
(padding-line-between-statements)
[error] 86-86: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 86-86: Delete ·
(prettier/prettier)
[error] 93-93: Expected space(s) after "if".
(keyword-spacing)
[error] 93-93: Replace (router.isReady) with ·(router.isReady)·
(prettier/prettier)
[error] 93-108: Missing space before opening brace.
(space-before-blocks)
[error] 94-94: Expected blank line after variable declarations.
(newline-after-var)
[error] 94-94: Missing radix parameter.
(radix)
[error] 95-104: Expected blank line before this statement.
(padding-line-between-statements)
[error] 95-95: Expected space(s) after "if".
(keyword-spacing)
[error] 95-95: Replace (!pageFromQuery) with ·(!pageFromQuery)·
(prettier/prettier)
[error] 95-104: Missing space before opening brace.
(space-before-blocks)
[error] 99-99: Delete ,
(prettier/prettier)
[error] 99-99: Unexpected trailing comma.
(comma-dangle)
[error] 106-106: Expected blank line after variable declarations.
(newline-after-var)
[error] 107-107: Expected blank line before this statement.
(padding-line-between-statements)
pages/blog/index.tsx
Outdated
| @@ -1,5 +1,5 @@ | |||
| import { useRouter } from 'next/router'; | |||
| import React, { useContext, useEffect, useState } from 'react'; | |||
| import React, { useContext, useEffect, useState, useRef } from 'react'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused useRef import.
The useRef hook is imported but never used in the component.
-import React, { useContext, useEffect, useState, useRef } from 'react';
+import React, { useContext, useEffect, useState } from 'react';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React, { useContext, useEffect, useState, useRef } from 'react'; | |
| import React, { useContext, useEffect, useState } from 'react'; |
🧰 Tools
🪛 eslint
[error] 2-2: 'useRef' is defined but never used.
(unused-imports/no-unused-imports)
[error] 2-2: 'useRef' is defined but never used.
(no-unused-vars)
🪛 GitHub Actions: PR testing - if Node project
[error] 2-2: Unused import: 'useRef' is defined but never used
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/navigation/BlogPagination.tsx (1)
63-73: Optimize Tailwind classes for the Previous button.Use Tailwind's size utility for consistent dimensions.
-className={`${currentPage === 1 ? 'opacity-50 cursor-not-allowed' : ''} w-[120px] h-[35px] py-2 px-4 rounded-l-md`} +className={`${currentPage === 1 ? 'opacity-50 cursor-not-allowed' : ''} size-[120px] py-2 px-4 rounded-l-md`}🧰 Tools
🪛 eslint
[error] 63-63: Replace
············with······(prettier/prettier)
[error] 64-64: Delete
········(prettier/prettier)
[error] 65-65: Replace
················aria-label="Previous·page"with········aria-label='Previous·page'(prettier/prettier)
[error] 65-65: Unexpected usage of doublequote.
(jsx-quotes)
[error] 66-66: Replace
················with········(prettier/prettier)
[error] 67-67: Replace
················with········(prettier/prettier)
[error] 68-68: Delete
········(prettier/prettier)
[error] 69-69: Replace
················with········(prettier/prettier)
[error] 70-70: Delete
········(prettier/prettier)
[error] 71-71: Replace
················with········(prettier/prettier)
[error] 72-72: Replace
················with········(prettier/prettier)
[error] 73-73: Delete
······(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/navigation/BlogPagination.tsx(1 hunks)pages/blog/index.tsx(6 hunks)
🧰 Additional context used
🪛 eslint
components/navigation/BlogPagination.tsx
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 2-2: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 6-6: @/types/components/buttons/ButtonPropsType import should occur before import of ../icons/ArrowRight
(import/order)
[error] 8-9: Delete ⏎
(prettier/prettier)
[error] 8-9: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 10-10: Delete ··
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 13-13: Delete ··
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 18-18: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 22-22: Replace ········ with ····
(prettier/prettier)
[error] 22-22: Expected blank line after variable declarations.
(newline-after-var)
[error] 22-22: 'pageNumbers' is already declared in the upper scope on line 19 column 12.
(@typescript-eslint/no-shadow)
[error] 23-23: Delete ····
(prettier/prettier)
[error] 23-53: Expected blank line before this statement.
(padding-line-between-statements)
[error] 24-24: Replace ············ with ······
(prettier/prettier)
[error] 25-25: Replace ················ with ········
(prettier/prettier)
[error] 26-26: Delete ······
(prettier/prettier)
[error] 27-27: Replace ········ with ····
(prettier/prettier)
[error] 28-28: Delete ······
(prettier/prettier)
[error] 28-52: Unexpected if as the only statement in an else block.
(no-lonely-if)
[error] 29-29: Replace ················ with ········
(prettier/prettier)
[error] 30-30: Delete ··········
(prettier/prettier)
[error] 31-31: Replace ················ with ········
(prettier/prettier)
[error] 32-32: Delete ········
(prettier/prettier)
[error] 33-33: Replace ················ with ········
(prettier/prettier)
[error] 34-34: Replace ················ with ········
(prettier/prettier)
[error] 35-35: Replace ················ with ········
(prettier/prettier)
[error] 36-36: Delete ······
(prettier/prettier)
[error] 37-37: Replace ················ with ········
(prettier/prettier)
[error] 38-38: Replace ················ with ········
(prettier/prettier)
[error] 39-39: Replace ················ with ········
(prettier/prettier)
[error] 40-40: Replace ················ with ········
(prettier/prettier)
[error] 41-41: Delete ········
(prettier/prettier)
[error] 42-42: Replace ···················· with ··········
(prettier/prettier)
[error] 43-43: Replace ················ with ········
(prettier/prettier)
[error] 44-44: Delete ······
(prettier/prettier)
[error] 45-45: Replace ················ with ········
(prettier/prettier)
[error] 46-46: Replace ················ with ········
(prettier/prettier)
[error] 47-47: Replace ················ with ········
(prettier/prettier)
[error] 48-48: Replace ················ with ········
(prettier/prettier)
[error] 49-49: Replace ················ with ········
(prettier/prettier)
[error] 50-50: Replace ················ with ········
(prettier/prettier)
[error] 51-51: Delete ········
(prettier/prettier)
[error] 52-52: Delete ······
(prettier/prettier)
[error] 53-53: Replace ········ with ····
(prettier/prettier)
[error] 54-54: Delete ····
(prettier/prettier)
[error] 54-54: Expected blank line before this statement.
(padding-line-between-statements)
[error] 55-55: Delete ··
(prettier/prettier)
[error] 57-57: Delete ··
(prettier/prettier)
[error] 58-58: Delete ····
(prettier/prettier)
[error] 59-59: Replace ···· with ··
(prettier/prettier)
[error] 61-61: Delete ··
(prettier/prettier)
[error] 62-62: Replace ········<nav·aria-label="Blog·pagination" with ····<nav·aria-label='Blog·pagination'
(prettier/prettier)
[error] 62-62: Unexpected usage of doublequote.
(jsx-quotes)
[error] 63-63: Replace ············ with ······
(prettier/prettier)
[error] 64-64: Delete ········
(prettier/prettier)
[error] 65-65: Replace ················aria-label="Previous·page" with ········aria-label='Previous·page'
(prettier/prettier)
[error] 65-65: Unexpected usage of doublequote.
(jsx-quotes)
[error] 66-66: Replace ················ with ········
(prettier/prettier)
[error] 67-67: Replace ················ with ········
(prettier/prettier)
[error] 68-68: Delete ········
(prettier/prettier)
[error] 69-69: Replace ················ with ········
(prettier/prettier)
[error] 70-70: Delete ········
(prettier/prettier)
[error] 71-71: Replace ················ with ········
(prettier/prettier)
[error] 72-72: Replace ················ with ········
(prettier/prettier)
[error] 73-73: Delete ······
(prettier/prettier)
[error] 74-74: Delete ······
(prettier/prettier)
[error] 75-75: Delete ········
(prettier/prettier)
[error] 76-76: Delete ··········
(prettier/prettier)
[error] 77-77: Replace ························ with ············
(prettier/prettier)
[error] 78-78: Replace ························ with ············
(prettier/prettier)
[error] 79-79: Delete ············
(prettier/prettier)
[error] 80-80: Replace ························ with ············
(prettier/prettier)
[error] 81-81: Replace ························ with ············
(prettier/prettier)
[error] 82-82: Delete ············
(prettier/prettier)
[error] 83-83: Replace ···················· with ··········
(prettier/prettier)
[error] 84-84: Replace ························ with ············
(prettier/prettier)
[error] 85-85: Replace ···················· with ··········
(prettier/prettier)
[error] 86-86: Replace ················ with ········
(prettier/prettier)
[error] 87-87: Delete ······
(prettier/prettier)
[error] 88-88: Replace ············ with ······
(prettier/prettier)
[error] 89-89: Replace ················ with ········
(prettier/prettier)
[error] 90-90: Delete ········
(prettier/prettier)
[error] 91-91: Replace ················ with ········
(prettier/prettier)
[error] 92-92: Delete ········
(prettier/prettier)
[error] 93-93: Delete ········
(prettier/prettier)
[error] 94-94: Replace ················ with ········
(prettier/prettier)
[error] 95-95: Delete ········
(prettier/prettier)
[error] 96-96: Replace ················ with ········
(prettier/prettier)
[error] 97-97: Delete ······
(prettier/prettier)
[error] 98-98: Replace ········ with ····
(prettier/prettier)
[error] 99-99: Delete ··
(prettier/prettier)
pages/blog/index.tsx
[error] 1-16: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 27-27: Insert ··
(prettier/prettier)
[error] 28-28: Insert ··
(prettier/prettier)
[error] 30-30: Insert ··
(prettier/prettier)
[error] 31-31: Insert ··
(prettier/prettier)
[error] 33-33: Insert ··
(prettier/prettier)
[error] 34-34: Insert ··
(prettier/prettier)
[error] 55-55: Expected blank line after variable declarations.
(newline-after-var)
[error] 56-65: Expected blank line before this statement.
(padding-line-between-statements)
[error] 59-59: Delete ,
(prettier/prettier)
[error] 59-59: Unexpected trailing comma.
(comma-dangle)
[error] 63-63: Delete ,
(prettier/prettier)
[error] 63-63: Unexpected trailing comma.
(comma-dangle)
[error] 73-73: Missing radix parameter.
(radix)
[error] 77-77: Expected blank line after variable declarations.
(newline-after-var)
[error] 78-78: Expected blank line before this statement.
(padding-line-between-statements)
[error] 83-86: Expected blank line after variable declarations.
(newline-after-var)
[error] 87-96: Expected blank line before this statement.
(padding-line-between-statements)
🪛 GitHub Actions: PR testing - if Node project
components/navigation/BlogPagination.tsx
[error] 1-2: Multiple imports of 'react' detected and import sorting issues
[error] 22-22: Variable 'pageNumbers' is already declared in the upper scope
[warning] 62-96: Multiple Tailwind CSS issues: invalid classnames order and non-shorthand usage (w-4/h-4 could be size-4)
[warning] 16-16: Missing JSDoc comment
pages/blog/index.tsx
[error] 1-1: Import sorting issues detected - requires autofix
[error] 73-73: Missing radix parameter in parseInt/parseFloat operation
🔇 Additional comments (6)
components/navigation/BlogPagination.tsx (2)
9-14: LGTM! Well-defined interface.The
BlogPaginationPropsinterface clearly defines the required props with appropriate types.🧰 Tools
🪛 eslint
[error] 10-10: Delete
··(prettier/prettier)
[error] 11-11: Delete
··(prettier/prettier)
[error] 12-12: Delete
··(prettier/prettier)
[error] 13-13: Delete
··(prettier/prettier)
57-59: LGTM! Proper effect dependencies.The useEffect hook correctly updates page numbers when relevant props change.
🧰 Tools
🪛 eslint
[error] 57-57: Delete
··(prettier/prettier)
[error] 58-58: Delete
····(prettier/prettier)
[error] 59-59: Replace
····with··(prettier/prettier)
pages/blog/index.tsx (4)
71-73: Add input validation for currentPage.The
parseIntcall is missing a radix parameter and there's no validation for NaN values.-const currentPage = parseInt(router.query.page as string) || 1; +const currentPage = Number.isNaN(parseInt(router.query.page as string, 10)) ? 1 : parseInt(router.query.page as string, 10);🧰 Tools
🪛 eslint
[error] 73-73: Missing radix parameter.
(radix)
🪛 GitHub Actions: PR testing - if Node project
[error] 73-73: Missing radix parameter in parseInt/parseFloat operation
75-79: LGTM! Efficient posts pagination.Good use of useMemo for optimizing the posts slice calculation with proper dependencies.
🧰 Tools
🪛 eslint
[error] 77-77: Expected blank line after variable declarations.
(newline-after-var)
[error] 78-78: Expected blank line before this statement.
(padding-line-between-statements)
81-97: LGTM! Clean pagination navigation.The paginate function correctly updates the URL while preserving other query parameters.
🧰 Tools
🪛 eslint
[error] 83-86: Expected blank line after variable declarations.
(newline-after-var)
[error] 87-96: Expected blank line before this statement.
(padding-line-between-statements)
180-185: LGTM! Proper pagination component integration.The BlogPagination component is correctly integrated with all required props.
| {pageNumbers.map((number, index) => ( | ||
| <button | ||
| key={index} | ||
| className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B'}`} |
There was a problem hiding this comment.
Fix className syntax error.
There's a missing closing bracket in the className string.
-className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B'}`}
+className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B]'}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B'}`} | |
| className={`w-[40px] h-[40px] ${number === currentPage ? 'border rounded bg-[#6200EE] text-white' : 'text-[#6B6B6B]'}`} |
🧰 Tools
🪛 eslint
[error] 78-78: Replace ························ with ············
(prettier/prettier)
| import React from 'react'; | ||
| import { useState, useEffect } from 'react'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate React imports.
Combine the React imports to avoid duplication.
-import React from 'react';
-import { useState, useEffect } from 'react';
+import React, { useState, useEffect } from 'react';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React from 'react'; | |
| import { useState, useEffect } from 'react'; | |
| import React, { useState, useEffect } from 'react'; |
🧰 Tools
🪛 eslint
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 2-2: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
🪛 GitHub Actions: PR testing - if Node project
[error] 1-2: Multiple imports of 'react' detected and import sorting issues
| const calculatePageNumbers = () => { | ||
| const pageNumbers: (number | string)[] = []; | ||
| if (totalPages <= pagesToShow) { | ||
| for (let i = 1; i <= totalPages; i++) { | ||
| pageNumbers.push(i); | ||
| } | ||
| } else { | ||
| if (currentPage <= 2) { | ||
| for (let i = 1; i <= 3; i++) { | ||
| pageNumbers.push(i); | ||
| } | ||
| pageNumbers.push('...'); | ||
| pageNumbers.push(totalPages - 2); | ||
| pageNumbers.push(totalPages - 1); | ||
| pageNumbers.push(totalPages); | ||
| } else if (currentPage >= totalPages - 1) { | ||
| pageNumbers.push(1); | ||
| pageNumbers.push(2); | ||
| pageNumbers.push(3); | ||
| pageNumbers.push('...'); | ||
| for (let i = totalPages - 2; i <= totalPages; i++) { | ||
| pageNumbers.push(i); | ||
| } | ||
| } else { | ||
| pageNumbers.push(1); | ||
| pageNumbers.push('...'); | ||
| pageNumbers.push(currentPage - 1); | ||
| pageNumbers.push(currentPage); | ||
| pageNumbers.push(currentPage + 1); | ||
| pageNumbers.push('...'); | ||
| pageNumbers.push(totalPages); | ||
| } | ||
| } | ||
| return pageNumbers; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve pagination calculation robustness.
- The
pageNumbersvariable shadows the state variable. - Add validation for edge cases.
const calculatePageNumbers = () => {
- const pageNumbers: (number | string)[] = [];
+ const numbers: (number | string)[] = [];
+ if (totalPages < 1) return [];
if (totalPages <= pagesToShow) {
for (let i = 1; i <= totalPages; i++) {
- pageNumbers.push(i);
+ numbers.push(i);
}
} else {
// ... rest of the logic, replace pageNumbers with numbers
}
- return pageNumbers;
+ return numbers;
};Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 eslint
[error] 21-21: Delete ··
(prettier/prettier)
[error] 22-22: Replace ········ with ····
(prettier/prettier)
[error] 22-22: Expected blank line after variable declarations.
(newline-after-var)
[error] 22-22: 'pageNumbers' is already declared in the upper scope on line 19 column 12.
(@typescript-eslint/no-shadow)
[error] 23-23: Delete ····
(prettier/prettier)
[error] 23-53: Expected blank line before this statement.
(padding-line-between-statements)
[error] 24-24: Replace ············ with ······
(prettier/prettier)
[error] 25-25: Replace ················ with ········
(prettier/prettier)
[error] 26-26: Delete ······
(prettier/prettier)
[error] 27-27: Replace ········ with ····
(prettier/prettier)
[error] 28-28: Delete ······
(prettier/prettier)
[error] 28-52: Unexpected if as the only statement in an else block.
(no-lonely-if)
[error] 29-29: Replace ················ with ········
(prettier/prettier)
[error] 30-30: Delete ··········
(prettier/prettier)
[error] 31-31: Replace ················ with ········
(prettier/prettier)
[error] 32-32: Delete ········
(prettier/prettier)
[error] 33-33: Replace ················ with ········
(prettier/prettier)
[error] 34-34: Replace ················ with ········
(prettier/prettier)
[error] 35-35: Replace ················ with ········
(prettier/prettier)
[error] 36-36: Delete ······
(prettier/prettier)
[error] 37-37: Replace ················ with ········
(prettier/prettier)
[error] 38-38: Replace ················ with ········
(prettier/prettier)
[error] 39-39: Replace ················ with ········
(prettier/prettier)
[error] 40-40: Replace ················ with ········
(prettier/prettier)
[error] 41-41: Delete ········
(prettier/prettier)
[error] 42-42: Replace ···················· with ··········
(prettier/prettier)
[error] 43-43: Replace ················ with ········
(prettier/prettier)
[error] 44-44: Delete ······
(prettier/prettier)
[error] 45-45: Replace ················ with ········
(prettier/prettier)
[error] 46-46: Replace ················ with ········
(prettier/prettier)
[error] 47-47: Replace ················ with ········
(prettier/prettier)
[error] 48-48: Replace ················ with ········
(prettier/prettier)
[error] 49-49: Replace ················ with ········
(prettier/prettier)
[error] 50-50: Replace ················ with ········
(prettier/prettier)
[error] 51-51: Delete ········
(prettier/prettier)
[error] 52-52: Delete ······
(prettier/prettier)
[error] 53-53: Replace ········ with ····
(prettier/prettier)
[error] 54-54: Delete ····
(prettier/prettier)
[error] 54-54: Expected blank line before this statement.
(padding-line-between-statements)
[error] 55-55: Delete ··
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 22-22: Variable 'pageNumbers' is already declared in the upper scope
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/navigation/BlogPagination.tsx (3)
1-8: Group related imports together.Consider organizing imports into groups: external dependencies, types, and internal components.
import React, { useEffect, useState } from 'react'; - import { ButtonIconPosition } from '@/types/components/buttons/ButtonPropsType'; - import Button from '../buttons/Button'; import IconArrowLeft from '../icons/ArrowLeft'; import IconArrowRight from '../icons/ArrowRight';
21-56: Extract magic numbers into named constants.Improve code maintainability by making the pagination logic more configurable and self-documenting.
+ // Constants for pagination configuration + const VISIBLE_PAGES_NEAR_CURRENT = 1; // Number of pages to show before and after current page + const VISIBLE_PAGES_AT_EDGES = 3; // Number of pages to show at the start/end + const ELLIPSIS = '...'; + const calculatePageNumbers = () => { const numbers: (number | string)[] = []; if (totalPages < 1) return []; if (totalPages <= pagesToShow) { for (let i = 1; i <= totalPages; i++) { numbers.push(i); } - } else if (currentPage <= 2) { - for (let i = 1; i <= 3; i++) { + } else if (currentPage <= VISIBLE_PAGES_AT_EDGES) { + for (let i = 1; i <= VISIBLE_PAGES_AT_EDGES; i++) { numbers.push(i); } - numbers.push('...'); + numbers.push(ELLIPSIS); numbers.push(totalPages - 2); numbers.push(totalPages - 1); numbers.push(totalPages); } else if (currentPage >= totalPages - 1) {
90-90: Fix Tailwind class order in Next button.For consistency with the Previous button, reorder the classes and use the
sizeshorthand.- className={`${currentPage === totalPages && 'cursor-not-allowed opacity-50'} h-[35px] w-[120px] rounded-l-md px-4 py-2`} + className={`${currentPage === totalPages ? 'cursor-not-allowed opacity-50' : ''} size-[120px] rounded-l-md px-4 py-2`}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/navigation/BlogPagination.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR testing - if Node project
components/navigation/BlogPagination.tsx
[warning] 16-16: Missing JSDoc comment
🔇 Additional comments (1)
components/navigation/BlogPagination.tsx (1)
58-60: LGTM!The effect hook correctly updates the page numbers when relevant props change.
| interface BlogPaginationProps { | ||
| blogsPerPage: number; | ||
| totalBlogs: number; | ||
| paginate: (pageNumber: number) => void; | ||
| currentPage: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSDoc documentation for the interface.
Add documentation to improve code maintainability and address the pipeline warning.
+/**
+ * Props for the BlogPagination component
+ * @property {number} blogsPerPage - Number of blogs to display per page
+ * @property {number} totalBlogs - Total number of blogs
+ * @property {function} paginate - Callback function to handle page changes
+ * @property {number} currentPage - Current active page number
+ */
interface BlogPaginationProps {
blogsPerPage: number;
totalBlogs: number;
paginate: (pageNumber: number) => void;
currentPage: number;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface BlogPaginationProps { | |
| blogsPerPage: number; | |
| totalBlogs: number; | |
| paginate: (pageNumber: number) => void; | |
| currentPage: number; | |
| } | |
| /** | |
| * Props for the BlogPagination component | |
| * @property {number} blogsPerPage - Number of blogs to display per page | |
| * @property {number} totalBlogs - Total number of blogs | |
| * @property {function} paginate - Callback function to handle page changes | |
| * @property {number} currentPage - Current active page number | |
| */ | |
| interface BlogPaginationProps { | |
| blogsPerPage: number; | |
| totalBlogs: number; | |
| paginate: (pageNumber: number) => void; | |
| currentPage: number; | |
| } |
| export default function BlogPagination({ blogsPerPage, totalBlogs, paginate, currentPage }: BlogPaginationProps) { | ||
| const totalPages: number = Math.ceil(totalBlogs / blogsPerPage); | ||
| const pagesToShow: number = 6; | ||
| const [pageNumbers, setPageNumbers] = useState<(number | string)[]>([]); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSDoc documentation for the component.
Add documentation to improve code maintainability and address the pipeline warning.
+/**
+ * A pagination component for blog posts that displays page numbers and navigation buttons
+ * @param {BlogPaginationProps} props - The props for the component
+ * @returns {JSX.Element} A navigation element with pagination controls
+ */
export default function BlogPagination({ blogsPerPage, totalBlogs, paginate, currentPage }: BlogPaginationProps) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function BlogPagination({ blogsPerPage, totalBlogs, paginate, currentPage }: BlogPaginationProps) { | |
| const totalPages: number = Math.ceil(totalBlogs / blogsPerPage); | |
| const pagesToShow: number = 6; | |
| const [pageNumbers, setPageNumbers] = useState<(number | string)[]>([]); | |
| /** | |
| * A pagination component for blog posts that displays page numbers and navigation buttons | |
| * @param {BlogPaginationProps} props - The props for the component | |
| * @returns {JSX.Element} A navigation element with pagination controls | |
| */ | |
| export default function BlogPagination({ blogsPerPage, totalBlogs, paginate, currentPage }: BlogPaginationProps) { | |
| const totalPages: number = Math.ceil(totalBlogs / blogsPerPage); | |
| const pagesToShow: number = 6; | |
| const [pageNumbers, setPageNumbers] = useState<(number | string)[]>([]); | |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[warning] 16-16: Missing JSDoc comment
|
@akshatnema @Mayaleeeee |
Description
Added pagination feature on blog page.
Related issue(s)
Fixes #3570
Demo
Blogs.Page.-.Made.with.Clipchamp.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Improvements