-
-
Notifications
You must be signed in to change notification settings - Fork 997
feat: add pagination to Blog index page fixes #4499 #4526
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 project configuration. |
WalkthroughClient-side pagination was added to the blog index: new Changes
Sequence DiagramsequenceDiagram
actor User
participant BlogPage as Blog Page
participant FilterComponent as Filter Component
participant State as Pagination State
participant Renderer as Render Engine
User->>FilterComponent: Apply filter
FilterComponent->>BlogPage: onFilter(filteredPosts)
BlogPage->>State: set currentPage = 1<br/>set posts = filteredPosts
State->>State: compute currentPosts and totalPages
State->>Renderer: trigger re-render
Renderer->>User: render currentPosts and Prev/Next controls (Page X of Y)
User->>BlogPage: Click Next / Previous
BlogPage->>State: update currentPage
State->>Renderer: trigger re-render
Renderer->>User: render updated currentPosts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4526--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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/blog/index.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T11:32:30.226Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T11:17:32.246Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:80-87
Timestamp: 2024-10-11T11:17:32.246Z
Learning: In the `BlogPostItem` component, the parent `Link` wraps the entire content, so inner `Link` components around the title and excerpt are unnecessary.
Applied to files:
pages/blog/index.tsx
🧬 Code graph analysis (1)
pages/blog/index.tsx (5)
components/typography/Heading.tsx (1)
Heading(30-85)components/typography/Paragraph.tsx (1)
Paragraph(28-56)components/typography/TextLink.tsx (1)
TextLink(21-42)components/Loader.tsx (1)
Loader(28-41)components/layout/GenericLayout.tsx (1)
GenericLayout(25-51)
🪛 GitHub Actions: PR testing - if Node project
pages/blog/index.tsx
[error] 17-17: prettier/prettier: Delete ⏎⏎
[error] 17-17: no-multiple-empty-lines: More than 1 blank line not allowed.
[warning] 19-19: require-jsdoc: Missing JSDoc comment.
[error] 27-27: prettier/prettier: Insert spaces for indentation.
[error] 28-28: prettier/prettier: Insert spaces for indentation.
[error] 30-30: prettier/prettier: Insert spaces for indentation.
[error] 31-31: prettier/prettier: Insert spaces for indentation.
[error] 33-33: prettier/prettier: Insert spaces for indentation.
[error] 34-34: prettier/prettier: Insert spaces for indentation.
[error] 54-54: prettier/prettier: Replace multi-line object notation with single-line when possible.
[error] 75-75: prettier/prettier: Incorrect line break around className/id attributes.
[error] 83-83: prettier/prettier: Delete extra newline.
[error] 92-92: prettier/prettier: Replace multi-line props with single-line form.
[error] 97-97: prettier/prettier: Replace multi-line attribute blocks with single line.
[error] 104-104: prettier/prettier: Replace multi-line attribute blocks with single line.
[error] 119-119: prettier/prettier: Delete extra newline.
[error] 121-121: no-multiple-empty-lines: More than 1 blank line not allowed.
[error] 144-144: prettier/prettier: Replace multi-line text with single-line text.
[error] 162-162: prettier/prettier: Replace multi-line expression with single line.
[error] 176-176: prettier/prettier: Replace multi-line setCurrentPage call with single line.
[error] 193-193: prettier/prettier: Replace multi-line props with single line.
[error] 205-205: prettier/prettier: Insert missing newline at end of file.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lighthouse CI
- Render blog posts during SSR (removed isClient guard) - Use stable unique key (slug/id/title) for BlogPostItem mapping - Removed redundant loader
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
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)
2-2: Remove the unuseduseEffectimport.The
useEffectimport is no longer used after removing the client-side rendering logic.Apply this diff:
-import React, { useContext, useEffect, useState } from 'react'; +import React, { useContext, useState } from 'react';
🧹 Nitpick comments (1)
pages/blog/index.tsx (1)
16-16: Consider adding a JSDoc comment.While optional, a brief JSDoc comment describing the blog index page and its pagination behavior would improve maintainability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/blog/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-11T11:17:32.246Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:80-87
Timestamp: 2024-10-11T11:17:32.246Z
Learning: In the `BlogPostItem` component, the parent `Link` wraps the entire content, so inner `Link` components around the title and excerpt are unnecessary.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T11:32:30.226Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T07:38:35.745Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T10:46:58.882Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T07:27:53.362Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:27:53.362Z
Learning: Using anchor tags for author names in 'FeaturedBlogPost' component leads to hydration issues on the website.
Applied to files:
pages/blog/index.tsx
🧬 Code graph analysis (1)
pages/blog/index.tsx (4)
components/layout/GenericLayout.tsx (1)
GenericLayout(25-51)components/typography/Heading.tsx (1)
Heading(30-85)components/typography/Paragraph.tsx (1)
Paragraph(28-56)components/typography/TextLink.tsx (1)
TextLink(21-42)
🪛 GitHub Actions: PR testing - if Node project
pages/blog/index.tsx
[error] 2-2: useEffect is defined but never used. (no-unused-vars)
[error] 2-2: useEffect is defined but never used. (no-unused-vars) — ESLint: @typescript-eslint/no-unused-vars
[warning] 16-16: Missing JSDoc comment. (require-jsdoc)
[error] 24-31: Prettier formatting issues detected in this block. Automatic fixes may be required (Insert spaces, quote style, line breaks).
[error] 122-122: Line exceeds maximum length of 120. Consider breaking into multiple lines. (max-len)
[error] 122-122: JSX quotes style violation. Use single quotes. (jsx-quotes)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
🔇 Additional comments (5)
pages/blog/index.tsx (5)
20-33: LGTM: Sorting logic correctly prioritizes featured posts.The sorting implementation correctly places featured posts first and then orders by date descending. This preserves the existing behavior.
35-42: LGTM: Pagination calculation is correct.The pagination logic correctly calculates indices and slices the posts array. Using
Math.ceilfortotalPagesensures partial pages are properly counted.
44-57: LGTM: Filter integration with pagination is well-implemented.Resetting
currentPageto 1 when filters change is the correct behavior, preventing users from landing on an empty page when filtered results span fewer pages.
142-146: Good improvement: Using stable post properties for keys.The key now uses
post.slug || post.id || post.title, which is a significant improvement over the array index and provides stability across pagination changes. While there's a theoretical edge case if all three properties are non-unique, this is unlikely in practice and represents a solid solution.
150-178: LGTM: Pagination controls are well-implemented.The Previous/Next buttons correctly use
Math.maxandMath.minto prevent out-of-bounds navigation, and the disabled states provide clear visual feedback. Conditional rendering (only whentotalPages > 1) ensures a clean UX for single-page results.
pages/blog/index.tsx
Outdated
| type='button' | ||
| className='bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0' | ||
| type="button" | ||
| className="bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0" |
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.
Fix formatting issues flagged by the linter.
This line has multiple formatting issues: it exceeds the 120-character limit and uses double quotes instead of single quotes in JSX attributes.
Apply this diff to fix the formatting:
- className="bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0"
+ className='bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0'Alternatively, break the className into multiple lines for better readability:
className='bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2
- font-semibold tracking-heading text-gray-800 shadow-none transition-all
- duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0'
+ font-semibold tracking-heading text-gray-800 shadow-none
+ transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0'📝 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="bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0" | |
| className='bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0' |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 122-122: Line exceeds maximum length of 120. Consider breaking into multiple lines. (max-len)
[error] 122-122: JSX quotes style violation. Use single quotes. (jsx-quotes)
🤖 Prompt for AI Agents
In pages/blog/index.tsx around line 122, the JSX attribute uses double quotes
and the line exceeds the 120-character limit; change the attribute to use single
quotes and break the className string across multiple lines (or split into
template literal concatenation) so no output line exceeds 120 chars and each JSX
attribute uses single quotes; ensure indentation remains consistent and the
resulting lines preserve the same class order and spacing.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4526 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 780 780
Branches 144 144
=========================================
Hits 780 780 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 (1)
pages/blog/index.tsx (1)
106-108: Fix the className formatting as previously flagged.This formatting issue was flagged in the previous review and remains unresolved. The
classNameshould use single quotes per project conventions.Apply this diff:
- className='bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 - font-semibold tracking-heading text-gray-800 shadow-none transition-all - duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0' + className='bg-none text-md mt-1 rounded-md border border-gray-200 px-4 py-2 font-semibold tracking-heading text-gray-800 shadow-none transition-all duration-500 ease-in-out hover:text-gray-700 md:mt-0 md:py-0'
🧹 Nitpick comments (1)
pages/blog/index.tsx (1)
131-153: LGTM: Pagination logic is sound.The Previous/Next handlers correctly use
Math.max/Math.minto prevent out-of-bounds navigation, and the disabled states are properly managed.Optional: Enhance accessibility for pagination controls.
Consider adding ARIA attributes to improve screen reader support:
<button onClick={() => setCurrentPage((prev) => Math.max(prev - 1, 1))} disabled={currentPage === 1} + aria-label='Go to previous page' className='rounded-md border border-gray-300 px-3 py-1 text-gray-700 hover:bg-gray-100 disabled:opacity-50' > Previous </button> - <span className='text-gray-700'> + <span className='text-gray-700' aria-live='polite' aria-atomic='true'> Page {currentPage} of {totalPages} </span> <button onClick={() => setCurrentPage((prev) => Math.min(prev + 1, totalPages))} disabled={currentPage === totalPages} + aria-label='Go to next page' className='rounded-md border border-gray-300 px-3 py-1 text-gray-700 hover:bg-gray-100 disabled:opacity-50' > Next </button>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/blog/index.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-11T11:32:30.226Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T07:38:35.745Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T10:46:58.882Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T11:17:32.246Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:80-87
Timestamp: 2024-10-11T11:17:32.246Z
Learning: In the `BlogPostItem` component, the parent `Link` wraps the entire content, so inner `Link` components around the title and excerpt are unnecessary.
Applied to files:
pages/blog/index.tsx
📚 Learning: 2024-10-11T07:27:53.362Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:27:53.362Z
Learning: Using anchor tags for author names in 'FeaturedBlogPost' component leads to hydration issues on the website.
Applied to files:
pages/blog/index.tsx
🧬 Code graph analysis (1)
pages/blog/index.tsx (1)
components/layout/GenericLayout.tsx (1)
GenericLayout(25-51)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (5)
pages/blog/index.tsx (5)
2-2: LGTM: Import cleanup is correct.Removing
useEffectaligns with the removal of client-detection logic.
20-33: LGTM: Sorting logic is correct.Featured posts are prioritized, followed by reverse chronological order. The implementation correctly handles the date comparison.
35-42: LGTM: Pagination calculations are correct.The index calculations and
Math.ceilfor total pages are standard and accurate for client-side pagination.
44-47: LGTM: Filter callback correctly resets pagination.Resetting
currentPageto 1 when filters change is the expected behavior and prevents edge cases where the user might be stranded on a non-existent page.
118-129: LGTM: Blog list rendering is correct.The empty state and paginated posts rendering are properly implemented. The stable key with fallback pattern (
post.slug || post.id || post.title) ensures React can efficiently track items across pagination changes.
|
Hii @sambhavgupta0705 |
|
Hi @princerajpoot20, PR link: #4526 |
Description
fixes-> #4499
postsPerPageconstant).Test results
Related issue(s)
Screen.Recording.2025-11-13.at.10.29.02.AM.mov
Summary by CodeRabbit
New Features
Style