feat: add pagination feature in blog page#5157
feat: add pagination feature in blog page#5157pryxnsu wants to merge 12 commits intoasyncapi:masterfrom
Conversation
❌ Deploy Preview for asyncapi-website failed.Built without sensitive environment variables
|
📝 WalkthroughWalkthroughAdds client-side pagination (hook + components) and integrates it into the blog index; introduces Next/Previous icon components; adjusts button icon spacing; and updates filter logic to ignore the page query when applying filters. Changes
Sequence DiagramsequenceDiagram
participant User
participant BlogPage as Blog Page
participant usePagination as usePagination Hook
participant Router
participant Pagination as Pagination Component
User->>BlogPage: Open blog page
BlogPage->>usePagination: init with posts, itemsPerPage
usePagination->>BlogPage: return currentPage, setCurrentPage, currentItems, maxPage
BlogPage->>Router: read router.query.page
alt valid page in query
BlogPage->>usePagination: setCurrentPage(pageFromQuery)
else
BlogPage->>usePagination: setCurrentPage(1)
end
BlogPage->>BlogPage: render currentItems
BlogPage->>Pagination: render(totalPages, currentPage, onPageChange)
User->>Pagination: click page/Next/Previous
Pagination->>BlogPage: onPageChange(newPage)
BlogPage->>usePagination: setCurrentPage(newPage)
BlogPage->>Router: router.push/replace updated page (shallow)
Router-->>BlogPage: router.query updated
BlogPage->>BlogPage: rerender with new currentItems
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/blog/index.tsx (1)
91-91:⚠️ Potential issue | 🟡 Minor"Clear filters" button appears when the URL only contains
?page=N(no actual filters).
showClearFiltersistruewheneverrouter.queryhas any key, includingpage. Users will see a "Clear filters" button even though no filter is active — just a page number. Consider excluding thepagekey:Suggested fix
- const showClearFilters = Object.keys(router.query).length > 0; + const showClearFilters = Object.keys(router.query).filter((k) => k !== 'page').length > 0;
🤖 Fix all issues with AI agents
In `@components/buttons/Button.tsx`:
- Around line 79-87: The icon margin classes were removed causing icons to
collide with text; update the Button component so the icon spans (the ones with
data-testid 'Button-icon-left' and 'Button-icon-right' and the equivalent in the
Link branch) conditionally include the margin class when text is not empty: add
"mr-2" to the left-icon span only when text is present, and "ml-2" to the
right-icon span only when text is present, using the existing ButtonIconPosition
and text prop to decide; apply the same conditional margin logic in both the
normal button branch and the Link branch so components like GithubButton,
YoutubeButton, SlackButton, SubscribeButton, GoogleCalendarButton and
ICSFileButton regain spacing.
In `@components/helpers/usePagination.ts`:
- Around line 14-29: The usePagination hook leaves currentPage stale when items
shrink; update usePagination to import and use useEffect to clamp currentPage to
the recalculated maxPage: inside usePagination (function name usePagination,
state currentPage, setter setCurrentPage, and value maxPage) add an effect that
runs when maxPage changes and if currentPage > maxPage calls
setCurrentPage(Math.max(1, maxPage)) so currentItems never becomes an empty
slice; also add useEffect to the React import list.
In `@components/pagination/Pagination.tsx`:
- Around line 79-98: The container uses role='list' but its children (rendered
by getPageNumber() mapping: PaginationItem and the ellipsis span) lack
role='listitem'; update the mapping so each mapped item is wrapped in an element
with role='listitem' (e.g., wrap both the PaginationItem branch and the ellipsis
span branch in a wrapper with role='listitem'), ensuring PaginationItem still
receives pageNumber/isActive/onPageChange props and aria-label remains;
alternatively remove role='list' from the container if you prefer not to add
wrappers.
- Around line 65-71: The className strings in the Pagination component use the
invalid Tailwind class "hover:bg-gray-white"; update both occurrences inside the
component's className conditional (where currentPage === 1) to a valid color
like "hover:bg-white" or "hover:bg-gray-50" so the hover state is applied;
search for the "hover:bg-gray-white" token in
components/pagination/Pagination.tsx and replace it in the two places (the
currentPage === 1 branch around the first pagination button and the similar
branch later) with the chosen valid Tailwind class.
🧹 Nitpick comments (4)
pages/blog/index.tsx (3)
58-69:handlePageChangeis called inside the effect but missing from the dependency array.
handlePageChangecapturesrouterandsetCurrentPagein its closure and is invoked on line 66, but it's not listed as a dependency. This will trigger anexhaustive-depslint warning.Either wrap
handlePageChangeinuseCallbackand add it to the deps, or — per this repository's convention — add aneslint-disablecomment explaining the omission.Based on learnings, in the asyncapi/website repository, when you intentionally omit dependencies in React hooks, add an eslint-disable comment with an explanatory note.
Minimal fix — suppress with explanation
+ // eslint-disable-next-line react-hooks/exhaustive-deps -- handlePageChange is stable in behavior; only router.query.page and maxPage drive this effect }, [router.query.page, maxPage, currentPage]);
151-168: UseArray.lengthinstead ofObject.keys(…).lengthon arrays.
postsandcurrentItemsare arrays.Object.keys(array)works but returns stringified indices — it's semantically misleading and marginally slower. Use.lengthdirectly.Proposed diff
- {Object.keys(posts).length === 0 && ( + {posts.length === 0 && ( ... )} - {Object.keys(posts).length > 0 && isClient && ( + {posts.length > 0 && isClient && ( <ul className='mx-auto mt-12 grid max-w-lg gap-5 lg:max-w-none lg:grid-cols-3'> {currentItems.map((post, index) => ( <BlogPostItem key={index} post={post} /> ))} </ul> )} - {Object.keys(currentItems).length > 0 && !isClient && ( + {currentItems.length > 0 && !isClient && (
43-56: Page change doesn't scroll to top.After clicking a page number the viewport stays at the pagination bar near the bottom, forcing users to scroll up. Consider adding
window.scrollTo(0, 0)(or scrolling to#main-content) insidehandlePageChange.Suggested addition
const handlePageChange = (page: number) => { setCurrentPage(page); + window.scrollTo({ top: 0, behavior: 'smooth' }); const currentFilters = { ...router.query, page: page.toString() };components/pagination/PaginationItem.tsx (1)
26-36: Spread order:{...buttonProps}afterclassNameallows accidental override.If a consumer passes
classNamevia the rest props, it will silently replace the component's own styling. Consider merging or reordering. Not a problem with current usages, just defensive.
| <span className='inline-block' data-testid='Button-icon-left'> | ||
| {icon} | ||
| </span> | ||
| )} | ||
| <span className='inline-block'>{text}</span> | ||
| {icon && iconPosition === ButtonIconPosition.RIGHT && ( | ||
| <span className='ml-2 inline-block' data-testid='Button-icon-right'> | ||
| <span className='inline-block' data-testid='Button-icon-right'> | ||
| {icon} | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the Button.tsx file
find . -name "Button.tsx" -type fRepository: asyncapi/website
Length of output: 125
🏁 Script executed:
# Check the content around lines 79-87 to see the actual code
rg -n "inline-block" components/buttons/Button.tsx -A2 -B2 | head -40Repository: asyncapi/website
Length of output: 859
🏁 Script executed:
# Search for Button usages with both icon and text props to assess impact
rg -n "<Button" --type=tsx --type=jsx -A3 | grep -E "icon=|text=" | head -80Repository: asyncapi/website
Length of output: 87
🏁 Script executed:
# Specifically look for pagination button usage
rg -n "text=''" --type=tsx --type=jsx -B3 -A3 | grep -E "(Button|Pagination)" | head -40Repository: asyncapi/website
Length of output: 87
🏁 Script executed:
# Check git log to see if mr-2/ml-2 were removed from Button.tsx
git log -p --oneline -- components/buttons/Button.tsx | grep -E "(mr-2|ml-2)" -B3 -A3 | head -40Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Search for Button component usages more broadly
rg -n "Button" components/ -A4 | grep -E "(icon=|text=)" | head -100Repository: asyncapi/website
Length of output: 2884
🏁 Script executed:
# Look for imports of Button component to find all usages
rg -n "from.*Button" --type=ts --type=js -A5 2>/dev/null | head -60Repository: asyncapi/website
Length of output: 4375
🏁 Script executed:
# More direct search: find Button usages in JSX without type restrictions
rg -n "<Button" components/ -A5 | head -120Repository: asyncapi/website
Length of output: 8033
🏁 Script executed:
# Check pagination component to see how Button is used there
cat -n components/pagination/Pagination.tsx | sed -n '60,75p'Repository: asyncapi/website
Length of output: 787
🏁 Script executed:
# Check the specific Button usages that have both text and icon
cat -n components/layout/DocsLayout.tsx | sed -n '101,107p'Repository: asyncapi/website
Length of output: 568
🏁 Script executed:
# Check community Header that uses Button with icon
cat -n components/community/Header.tsx | sed -n '47,53p'Repository: asyncapi/website
Length of output: 418
🏁 Script executed:
# Let's check git diff to see what actually changed in the Button component
git diff HEAD~1 -- components/buttons/Button.tsx 2>/dev/null | head -80Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check if there are any recent changes or staging
git status -- components/buttons/Button.tsxRepository: asyncapi/website
Length of output: 125
🏁 Script executed:
# Try to see the previous version of the file
git show HEAD:components/buttons/Button.tsx 2>/dev/null | grep -n "inline-block" -A1 -B1 | head -40Repository: asyncapi/website
Length of output: 765
🏁 Script executed:
# Look for any mr-2 or ml-2 in the current file
rg -n "mr-2|ml-2" components/buttons/Button.tsxRepository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check all Button usages that have both non-empty text AND icon
rg -n "<Button" components/ -A8 | grep -B8 "icon=" | grep -A8 "text='" | grep -v "text=''" | head -100Repository: asyncapi/website
Length of output: 1131
🏁 Script executed:
# Let's check the git log for the Button.tsx file to see commit history
git log --oneline components/buttons/Button.tsx | head -20Repository: asyncapi/website
Length of output: 110
🏁 Script executed:
# Look at a specific commit to see if mr-2/ml-2 was removed
git log -p components/buttons/Button.tsx | grep -E "(mr-2|ml-2)" -B5 -A5 | head -60Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check the complete Button.tsx file to see full implementation
cat -n components/buttons/Button.tsx | head -120Repository: asyncapi/website
Length of output: 4400
🏁 Script executed:
# Look at the parent commit to confirm Button.tsx state there
git show HEAD~1:components/buttons/Button.tsx 2>/dev/null | grep -n "inline-block" -A1 -B1 | head -40Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Search across entire codebase for mr-2 or ml-2 in button contexts
rg -n "mr-2|ml-2" components/ --type=tsx --type=jsxRepository: asyncapi/website
Length of output: 87
🏁 Script executed:
# Check if IconRocket, IconGoogleCalendar, etc. have built-in margins
cat -n components/icons/Rocket.tsxRepository: asyncapi/website
Length of output: 2364
🏁 Script executed:
# Let's check if there's a master branch or other branches that might have the full history
git branch -aRepository: asyncapi/website
Length of output: 163
🏁 Script executed:
# Try to get more git history
git log --oneline --all | head -10Repository: asyncapi/website
Length of output: 209
🏁 Script executed:
# Check if previous version had mr-2/ml-2 by looking at git blame
git blame components/buttons/Button.tsx | grep -E "inline-block"Repository: asyncapi/website
Length of output: 827
🏁 Script executed:
# Let's verify that buttons with both icon and text actually work by checking rendered tests
find . -name "*.test.tsx" -o -name "*.spec.tsx" | xargs grep -l "Button" | head -3Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check if there are any snapshot tests or visual tests that would show spacing
find . -name "*.snap" | xargs grep -l "Button" 2>/dev/null | head -3Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check the master branch to see if it has mr-2/ml-2
git show master:components/buttons/Button.tsx 2>/dev/null | grep -n "inline-block" -A1 -B1Repository: asyncapi/website
Length of output: 785
🏁 Script executed:
# Let's check if there's a different Button file (dashboard)
cat -n components/dashboard/Button.tsx | head -120Repository: asyncapi/website
Length of output: 894
🏁 Script executed:
# Search for any margin classes in icon-related spans
rg -n "icon" components/buttons/ -A2 -B2 | grep -E "(className|mr|ml|gap)" | head -40Repository: asyncapi/website
Length of output: 3215
Removing icon margins breaks every other Button with both icon and text.
Confirmed: the mr-2 / ml-2 classes were removed from the icon spans (lines 79, 85, 101, 103), eliminating spacing between icons and labels globally. This breaks multiple existing Buttons with both icon and text — including GithubButton, YoutubeButton, SlackButton, SubscribeButton, GoogleCalendarButton, ICSFileButton, and others. The pagination buttons with text='' remain unaffected since they have no visible text label.
Apply the conditional margin approach:
Proposed fix — conditionally apply margin when text is present
{icon && iconPosition === ButtonIconPosition.LEFT && (
- <span className='inline-block' data-testid='Button-icon-left'>
+ <span className={`inline-block${text ? ' mr-2' : ''}`} data-testid='Button-icon-left'>
{icon}
</span>
)}
<span className='inline-block'>{text}</span>
{icon && iconPosition === ButtonIconPosition.RIGHT && (
- <span className='inline-block' data-testid='Button-icon-right'>
+ <span className={`inline-block${text ? ' ml-2' : ''}`} data-testid='Button-icon-right'>
{icon}
</span>
)}Apply the same pattern in the <Link> branch (lines 101–103).
📝 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.
| <span className='inline-block' data-testid='Button-icon-left'> | |
| {icon} | |
| </span> | |
| )} | |
| <span className='inline-block'>{text}</span> | |
| {icon && iconPosition === ButtonIconPosition.RIGHT && ( | |
| <span className='ml-2 inline-block' data-testid='Button-icon-right'> | |
| <span className='inline-block' data-testid='Button-icon-right'> | |
| {icon} | |
| </span> | |
| <span className={`inline-block${text ? ' mr-2' : ''}`} data-testid='Button-icon-left'> | |
| {icon} | |
| </span> | |
| )} | |
| <span className='inline-block'>{text}</span> | |
| {icon && iconPosition === ButtonIconPosition.RIGHT && ( | |
| <span className={`inline-block${text ? ' ml-2' : ''}`} data-testid='Button-icon-right'> | |
| {icon} | |
| </span> |
🤖 Prompt for AI Agents
In `@components/buttons/Button.tsx` around lines 79 - 87, The icon margin classes
were removed causing icons to collide with text; update the Button component so
the icon spans (the ones with data-testid 'Button-icon-left' and
'Button-icon-right' and the equivalent in the Link branch) conditionally include
the margin class when text is not empty: add "mr-2" to the left-icon span only
when text is present, and "ml-2" to the right-icon span only when text is
present, using the existing ButtonIconPosition and text prop to decide; apply
the same conditional margin logic in both the normal button branch and the Link
branch so components like GithubButton, YoutubeButton, SlackButton,
SubscribeButton, GoogleCalendarButton and ICSFileButton regain spacing.
components/helpers/usePagination.ts
Outdated
| export 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 | ||
| }; |
There was a problem hiding this comment.
currentPage becomes stale when items shrinks (e.g., after filtering).
When the user is on page 5 and applies a filter that reduces items to fewer than 5 pages, maxPage recalculates but currentPage remains 5. currentItems will be an empty slice, showing a blank page with no way to recover. The useEffect in blog/index.tsx won't fire either because router.query.page hasn't changed.
Clamp currentPage inside the hook when maxPage changes:
Proposed fix
export function usePagination<T>(items: T[], itemsPerPage: number) {
const [currentPage, setCurrentPage] = useState(1);
const maxPage = Math.ceil(items.length / itemsPerPage);
+ // Reset to page 1 when the item list shrinks below the current page
+ useEffect(() => {
+ if (currentPage > maxPage && maxPage > 0) {
+ setCurrentPage(1);
+ }
+ }, [maxPage]);
+
const currentItems = useMemo(() => {You'll need to add useEffect to the import 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.
| export 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 | |
| }; | |
| export function usePagination<T>(items: T[], itemsPerPage: number) { | |
| const [currentPage, setCurrentPage] = useState(1); | |
| const maxPage = Math.ceil(items.length / itemsPerPage); | |
| // Reset to page 1 when the item list shrinks below the current page | |
| useEffect(() => { | |
| if (currentPage > maxPage && maxPage > 0) { | |
| setCurrentPage(1); | |
| } | |
| }, [maxPage]); | |
| const currentItems = useMemo(() => { | |
| const start = (currentPage - 1) * itemsPerPage; | |
| return items.slice(start, start + itemsPerPage); | |
| }, [items, currentPage, itemsPerPage]); | |
| return { | |
| currentPage, | |
| setCurrentPage, | |
| currentItems, | |
| maxPage | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@components/helpers/usePagination.ts` around lines 14 - 29, The usePagination
hook leaves currentPage stale when items shrink; update usePagination to import
and use useEffect to clamp currentPage to the recalculated maxPage: inside
usePagination (function name usePagination, state currentPage, setter
setCurrentPage, and value maxPage) add an effect that runs when maxPage changes
and if currentPage > maxPage calls setCurrentPage(Math.max(1, maxPage)) so
currentItems never becomes an empty slice; also add useEffect to the React
import list.
| className={`font-normal flex h-[34px] items-center justify-center rounded bg-white px-3 py-[7px] text-sm | ||
| leading-[17px] | ||
| tracking-[-0.01em] ${ | ||
| currentPage === 1 | ||
| ? 'hover:bg-gray-white cursor-not-allowed text-gray-300' | ||
| : 'text-[#141717] hover:bg-gray-50' | ||
| }`} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display Tailwind configuration files
fd 'tailwind\.config\.(js|ts|cjs|mjs)' --type f -x cat {} \;
# Search for gray-white usage in the codebase
echo "=== Searching for gray-white usage ==="
rg 'gray-white' -nRepository: asyncapi/website
Length of output: 6052
Replace hover:bg-gray-white with a valid Tailwind color class.
The hover:bg-gray-white class is not defined in the project's Tailwind configuration. The custom gray palette includes shades 50–900, but no "gray-white" variant. Standard Tailwind also doesn't include this shade. The class will be silently ignored, leaving the hover state unstyled. Use hover:bg-white or hover:bg-gray-50 instead.
This applies to lines 69 and 109 in components/pagination/Pagination.tsx.
🤖 Prompt for AI Agents
In `@components/pagination/Pagination.tsx` around lines 65 - 71, The className
strings in the Pagination component use the invalid Tailwind class
"hover:bg-gray-white"; update both occurrences inside the component's className
conditional (where currentPage === 1) to a valid color like "hover:bg-white" or
"hover:bg-gray-50" so the hover state is applied; search for the
"hover:bg-gray-white" token in components/pagination/Pagination.tsx and replace
it in the two places (the currentPage === 1 branch around the first pagination
button and the similar branch later) with the chosen valid Tailwind class.
| <div className='flex gap-2' role='list'> | ||
| {getPageNumber().map((page) => | ||
| typeof page === 'number' ? ( | ||
| <PaginationItem | ||
| key={page} | ||
| pageNumber={page} | ||
| isActive={page === currentPage} | ||
| onPageChange={handlePageChange} | ||
| aria-label={`Go to page ${page}`} | ||
| /> | ||
| ) : ( | ||
| <span | ||
| key={page} | ||
| className='font-inter flex size-10 items-center justify-center text-sm font-semibold text-[#6B6B6B]' | ||
| aria-hidden='true' | ||
| > | ||
| ... | ||
| </span> | ||
| ) | ||
| )} |
There was a problem hiding this comment.
role='list' without role='listitem' children is an ARIA violation.
The <div role='list'> wraps <button> and <span> elements directly. ARIA requires role='list' children to be role='listitem'. Screen readers may ignore or misreport the list structure.
Either wrap each child in <div role='listitem'> or remove role='list' entirely (the page numbers are already self-explanatory with their aria-labels).
Option A — add listitem wrappers
<div className='flex gap-2' role='list'>
{getPageNumber().map((page) =>
typeof page === 'number' ? (
- <PaginationItem
- key={page}
- ...
- />
+ <div role='listitem' key={page}>
+ <PaginationItem
+ pageNumber={page}
+ isActive={page === currentPage}
+ onPageChange={handlePageChange}
+ aria-label={`Go to page ${page}`}
+ />
+ </div>
) : (
- <span key={page} ...>
- ...
- </span>
+ <div role='listitem' key={page}>
+ <span
+ className='font-inter flex size-10 items-center justify-center text-sm font-semibold text-[`#6B6B6B`]'
+ aria-hidden='true'
+ >
+ ...
+ </span>
+ </div>
)
)}
</div>📝 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 gap-2' role='list'> | |
| {getPageNumber().map((page) => | |
| typeof page === 'number' ? ( | |
| <PaginationItem | |
| key={page} | |
| pageNumber={page} | |
| isActive={page === currentPage} | |
| onPageChange={handlePageChange} | |
| aria-label={`Go to page ${page}`} | |
| /> | |
| ) : ( | |
| <span | |
| key={page} | |
| className='font-inter flex size-10 items-center justify-center text-sm font-semibold text-[#6B6B6B]' | |
| aria-hidden='true' | |
| > | |
| ... | |
| </span> | |
| ) | |
| )} | |
| <div className='flex gap-2' role='list'> | |
| {getPageNumber().map((page) => | |
| typeof page === 'number' ? ( | |
| <div role='listitem' key={page}> | |
| <PaginationItem | |
| pageNumber={page} | |
| isActive={page === currentPage} | |
| onPageChange={handlePageChange} | |
| aria-label={`Go to page ${page}`} | |
| /> | |
| </div> | |
| ) : ( | |
| <div role='listitem' key={page}> | |
| <span | |
| className='font-inter flex size-10 items-center justify-center text-sm font-semibold text-[`#6B6B6B`]' | |
| aria-hidden='true' | |
| > | |
| ... | |
| </span> | |
| </div> | |
| ) | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
In `@components/pagination/Pagination.tsx` around lines 79 - 98, The container
uses role='list' but its children (rendered by getPageNumber() mapping:
PaginationItem and the ellipsis span) lack role='listitem'; update the mapping
so each mapped item is wrapped in an element with role='listitem' (e.g., wrap
both the PaginationItem branch and the ellipsis span branch in a wrapper with
role='listitem'), ensuring PaginationItem still receives
pageNumber/isActive/onPageChange props and aria-label remains; alternatively
remove role='list' from the container if you prefer not to add wrappers.
…ynamically and update the function to render page number on pagination component
f9bd78e to
0636531
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5157 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 796 796
Branches 146 146
=========================================
Hits 796 796 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/pagination/PaginationItem.tsx`:
- Around line 3-5: Remove the stray eslint directive that targets a blank line
inside the PaginationItemProps declaration: delete the "//
eslint-disable-next-line prettier/prettier" line so the interface declaration is
clean (inspect the PaginationItemProps interface in PaginationItem.tsx and
remove that unnecessary comment).
🧹 Nitpick comments (7)
components/helpers/usePagination.ts (1)
28-38: Uncontrolled mode:setCurrentPageaccepts any number without bounds checking.In uncontrolled mode,
setInternalPage(nextPage)stores the raw value. AlthoughsafePageclamps it on the next render socurrentItemsand the returnedcurrentPageare always valid, the internal state can drift out of bounds. This is a minor inconsistency — consider clamping before setting:Optional improvement
- setInternalPage(nextPage); + setInternalPage(Math.min(Math.max(nextPage, 1), maxPage || 1));components/pagination/PaginationItem.tsx (2)
26-36: Addtype="button"to prevent accidental form submission.HTML
<button>elements default totype="submit". If this component is ever rendered inside a<form>, clicking it would submit the form. Adding an explicittype="button"before the spread ensures safe defaults while still allowing overrides viabuttonProps.Proposed fix
<button + type='button' onClick={() => onPageChange(pageNumber)} className={`font-inter font-normal relative flex size-8 items-center
28-31: Hardcoded hex colors bypass the Tailwind theme.
bg-[#6200EE]andtext-[#141717]are magic values. If the project defines these as theme tokens (e.g.,bg-primary-500,text-gray-900), prefer using those for consistency and easier future theming.pages/blog/index.tsx (4)
59-71: Synchronous mutation insideuseMemo— works but is fragile.
onFilterApplyis designed to deliver results via a callback. Here you exploit the fact that it calls the callback synchronously to capture the result into a locallet. This works today, but ifonFilterApplyever becomes async (e.g., debounced or deferred), this will silently return stale/empty data.A brief inline comment explaining the reliance on synchronous invocation would guard against future refactoring surprises.
72-77:totalPagesduplicates whatmaxPagecomputes inside the hook.
totalPages(line 72) andmaxPage(returned byusePagination) always produce the same value.totalPagesis needed before calling the hook to derivequeryPage, so it can't be eliminated entirely, but renaming or adding a short comment would clarify why two identical computations exist.
103-122: Filters change without resetting the page — potential UX confusion.When a user is on, say, page 3 and applies a filter that still yields ≥3 pages, the URL retains
?page=3. Most pagination UIs reset to page 1 on a data-source change (filter applied/removed) so users see the most relevant results first. The out-of-bounds case is handled (page is clamped), but the in-bounds case silently keeps a stale page number.Consider adding a
useEffectthat resets to page 1 wheneverfilterschanges (andcurrentPage > 1):Proposed approach
+ useEffect(() => { + if (!router.isReady) return; + if (Object.keys(filters).length === 0) return; + if (currentPage <= 1) return; + + const nextQuery = { ...router.query }; + delete nextQuery.page; + router.replace({ pathname: router.pathname, query: nextQuery }, undefined, { shallow: true, scroll: false }); + }, [filters]);
215-226: Loader condition relies oncurrentItems.length > 0but should mirror the data-present check.Line 215 gates on
filteredPosts.length > 0 && isClientto show posts. Line 222 gates oncurrentItems.length > 0 && !isClientto show the loader. IffilteredPostsis non-empty butcurrentItemshappens to be empty (theoretically shouldn't with current clamping, but defensive code should be symmetric), the loader would not display. For consistency, usefilteredPosts.length > 0 && !isClient:Proposed fix
- {currentItems.length > 0 && !isClient && ( + {filteredPosts.length > 0 && !isClient && (
| export interface PaginationItemProps { | ||
| // eslint-disable-next-line prettier/prettier | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Stray eslint-disable-next-line — applies to a blank line and has no effect.
The directive on line 4 targets the next line, which is empty. Remove it.
Proposed fix
export interface PaginationItemProps {
- // eslint-disable-next-line prettier/prettier
-
/** The page number to display */📝 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 interface PaginationItemProps { | |
| // eslint-disable-next-line prettier/prettier | |
| export interface PaginationItemProps { | |
| /** The page number to display */ |
🤖 Prompt for AI Agents
In `@components/pagination/PaginationItem.tsx` around lines 3 - 5, Remove the
stray eslint directive that targets a blank line inside the PaginationItemProps
declaration: delete the "// eslint-disable-next-line prettier/prettier" line so
the interface declaration is clean (inspect the PaginationItemProps interface in
PaginationItem.tsx and remove that unnecessary comment).
|
@anshgoyalevil I've resolved the conflicts and updated the PR. Please take a look |
|
@pryxnsu you are raising PR for an issue which has been closed . |



Description
Adds pagination functionality to the Blog page.
demo_web.mp4
Related issue(s)
Fixes #3570
Summary by CodeRabbit
New Features
Bug Fixes
Style