Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces a new Pagination React component with props for alignment, info display, button style, and element type. Adds Storybook stories demonstrating multiple pagination states, a README documenting usage and API, and an index file re-exporting the component. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
src/Components/Pagination/index.ts (1)
1-1: Barrel export looks good; consider re-exporting types (optional).If you decide to expose the data types, re-export them here for consumers.
export * from './Pagination'; +export type { PaginationData, Link } from './Pagination';src/Components/Pagination/README.md (2)
55-57: Use public import path in examples.Show consumers how to import from the package (or the component barrel), not a relative file path.
-import { Pagination } from './Pagination'; +import { Pagination } from '@wedevs/tail-react'; // or 'src/Components/Pagination'
124-133: Accessibility claims vs. implementation details.To truly meet these, document that disabled anchors are marked aria-disabled and removed from tab order (tabIndex -1), or rendered as buttons when interactive without hrefs.
Would you like a follow-up doc patch after the component change below?
src/Components/Pagination/Pagination.tsx (5)
41-42: Don’t hide the info row when there’s only one page.Render the info section even when there is no prev/next; conditionally render only the nav.
- if (!data || (!data.next_page_url && !data.prev_page_url)) return null; + if (!data) return null; + const hasNav = + Boolean(data.next_page_url || data.prev_page_url) || + (Array.isArray(data.links) && data.links.length > 0); ... - return ( + return ( <div className="flex w-full flex-col items-center justify-center md:flex-row md:justify-between"> <Info showInfo={showInfo} /> - <nav + {hasNav && ( + <nav className={`flex w-full flex-wrap items-center justify-center space-x-2 ${getAlignStyle()}`} aria-label="Pagination Navigation" > ... - </nav> + </nav> + )} </div> );Also applies to: 76-79, 80-83, 126-128
52-57: Make page link filtering robust and locale-agnostic.Rely on numeric labels rather than HTML entities/ellipsis strings that can vary.
- const pageLinks = links.filter( - link => - !link.label.includes('«') && - !link.label.includes('»') && - link.label !== '...', - ); + const pageLinks = links.filter(link => /^\d+$/.test(link.label));
66-74: Info component: return null and handle null from/to gracefully.Avoid empty fragments and show 0-based fallbacks.
- const Info = ({ showInfo }: { showInfo: boolean }) => { - if (!showInfo) return <></>; + const Info = ({ showInfo }: { showInfo: boolean }) => { + if (!showInfo) return null; return ( <div className="mb-2 w-full text-center text-sm text-gray-600 sm:mb-0 md:text-left" aria-live="polite"> - Showing {from} to {to} of {total} entries + Showing {from ?? 0} to {to ?? 0} of {total} entries </div> ); };
97-104: Avoid index as key.Use a stable key to prevent reconciliation issues when the link set changes.
- {pageLinks.map((link, index) => ( + {pageLinks.map((link) => ( <Button - key={index} + key={link.url ?? `p-${link.label}`}
84-96: Optional i18n for “Previous”/“Next”.Expose prevLabel/nextLabel props (defaulting to “Previous”/“Next”) to allow localization.
If you’d like, I can draft the minimal prop wiring.
Also applies to: 114-126
src/Components/Pagination/Pagination.stories.tsx (2)
127-139: Let the component do the prev/next filtering; keep stories closer to Laravel payloads.Returning links without prev/next skips exercising your in-component filter. Keep them and let the component filter.
- links: links.filter(link => !link.label.includes('«') && !link.label.includes('»')), + links,
32-42: Annotate the mock with the exported type.After exporting PaginationData/Link, type createMockData’s return for stronger story type-safety.
-const createMockData = (currentPage: number, totalPages: number, showEllipsis = false) => { +import type { PaginationData } from './Pagination'; +const createMockData = ( + currentPage: number, + totalPages: number, + showEllipsis = false +): PaginationData<{ id: number; name: string }> => { ... - return { + return {Also applies to: 122-139
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/Components/Pagination/Pagination.stories.tsx(1 hunks)src/Components/Pagination/Pagination.tsx(1 hunks)src/Components/Pagination/README.md(1 hunks)src/Components/Pagination/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/Components/Pagination/Pagination.tsx (1)
src/Components/Button/Button.tsx (1)
Button(149-149)
src/Components/Pagination/Pagination.stories.tsx (1)
src/Components/Pagination/Pagination.tsx (1)
Pagination(34-129)
🪛 LanguageTool
src/Components/Pagination/README.md
[grammar] ~128-~128: There might be a mistake here.
Context: ... <nav> element with proper ARIA labels - Provides aria-current="page" for the a...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...aria-current="page"for the active page - Includes descriptivearia-label` attrib...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ... aria-label attributes for all buttons - Maintains proper focus management and ke...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...focus management and keyboard navigation - Screen reader friendly with live region ...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...nt variants (primary, secondary, danger) - Multiple button styles (fill, outline, l...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...iple button styles (fill, outline, link) - Consistent disabled states and visual fe...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...tent disabled states and visual feedback - Proper focus and hover states ## Common...
(QB_NEW_EN)
[grammar] ~206-~206: There might be a mistake here.
Context: ...- start: Aligns pagination to the left - center: Centers the pagination controls - `end...
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ...center: Centers the pagination controls - end`: Aligns pagination to the right (defaul...
(QB_NEW_EN)
[grammar] ~212-~212: There might be a mistake here.
Context: ...ill: Solid background buttons (default) - outline: Border-only buttons - link`: Minimal ...
(QB_NEW_EN)
[grammar] ~213-~213: There might be a mistake here.
Context: ...efault) - outline: Border-only buttons - link: Minimal link-style buttons ## Best Pr...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ...hat support: - ES6+ JavaScript features - CSS Grid and Flexbox - Modern React feat...
(QB_NEW_EN)
[grammar] ~235-~235: There might be a mistake here.
Context: ...vaScript features - CSS Grid and Flexbox - Modern React features (hooks, etc.) For...
(QB_NEW_EN)
| interface Pagination<T = Record<string, unknown>> { | ||
| data: T[]; | ||
| links: Link[]; | ||
| current_page: number; | ||
| last_page: number; | ||
| first_page_url?: string; | ||
| last_page_url?: string; | ||
| next_page_url?: string | null; | ||
| prev_page_url?: string | null; | ||
| path?: string; | ||
| per_page: number; | ||
| total: number; | ||
| from: number; | ||
| to: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type naming and Laravel nullability; export types for consumers.
- Rename the Pagination interface to PaginationData to avoid confusion with the component.
- Make from/to nullable to match Laravel responses.
- Optionally export Link and PaginationData.
-interface Link {
+export interface Link {
label: string;
url: string | null;
active: boolean;
page: number | null;
}
-interface Pagination<T = Record<string, unknown>> {
+export interface PaginationData<T = Record<string, unknown>> {
data: T[];
links: Link[];
current_page: number;
last_page: number;
first_page_url?: string;
last_page_url?: string;
next_page_url?: string | null;
prev_page_url?: string | null;
path?: string;
per_page: number;
total: number;
- from: number;
- to: number;
+ from: number | null;
+ to: number | null;
}
-interface PaginationProps<T = Record<string, unknown>> {
- data: Pagination<T>;
+interface PaginationProps<T = Record<string, unknown>> {
+ data: PaginationData<T>;
align?: 'start' | 'center' | 'end';
showInfo?: boolean;
buttonStyle?: 'fill' | 'outline' | 'link';
paginationButtonAs?: 'a' | 'button';
}Also applies to: 26-33
🤖 Prompt for AI Agents
In src/Components/Pagination/Pagination.tsx around lines 10-24 (and also apply
same changes to lines 26-33), rename the interface Pagination to PaginationData
to avoid name collision with the component, change the types of from and to to
allow null (e.g., number | null) to match Laravel nullable fields, and export
the Link type and the new PaginationData interface so other modules can consume
them; update any local references to use the new PaginationData name.
| <Button | ||
| variant="secondary" | ||
| style={buttonStyle} | ||
| size="medium" | ||
| disabled={!prev_page_url} | ||
| as={paginationButtonAs} | ||
| href={prev_page_url || '#'} | ||
| className={!prev_page_url ? 'pointer-events-none' : ''} | ||
| aria-label="Go to previous page" | ||
| > | ||
| Previous | ||
| </Button> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Disabled anchors: prevent keyboard activation and improve a11y.
Anchors with href="#" remain focusable/activatable by keyboard. Use aria-disabled and remove from tab order when no URL; avoid "#" altogether.
<Button
variant="secondary"
style={buttonStyle}
size="medium"
- disabled={!prev_page_url}
- as={paginationButtonAs}
- href={prev_page_url || '#'}
- className={!prev_page_url ? 'pointer-events-none' : ''}
+ disabled={!prev_page_url}
+ as={paginationButtonAs}
+ href={prev_page_url ?? undefined}
+ aria-disabled={!prev_page_url || undefined}
+ tabIndex={!prev_page_url ? -1 : undefined}
+ className={!prev_page_url ? 'pointer-events-none' : ''}
aria-label="Go to previous page"
>
Previous
</Button>
...
<Button
variant="secondary"
style={buttonStyle}
size="medium"
- disabled={!next_page_url}
- as={paginationButtonAs}
- href={next_page_url || '#'}
- className={!next_page_url ? 'pointer-events-none' : ''}
+ disabled={!next_page_url}
+ as={paginationButtonAs}
+ href={next_page_url ?? undefined}
+ aria-disabled={!next_page_url || undefined}
+ tabIndex={!next_page_url ? -1 : undefined}
+ className={!next_page_url ? 'pointer-events-none' : ''}
aria-label="Go to next page"
>
Next
</Button>Also applies to: 114-126
🤖 Prompt for AI Agents
In src/Components/Pagination/Pagination.tsx around lines 84-96 (and similarly
for lines 114-126), the Button renders as an anchor with href="#" when no page
URL which keeps it focusable and keyboard-activatable; remove the placeholder
href when prev_page_url/next_page_url is falsy, add aria-disabled="true" and
tabIndex={-1} to remove it from the tab order, and keep the visual disabled
styling (e.g. pointer-events-none/className) so the element is not
keyboard-activated; ensure when a real URL exists you restore the href, remove
aria-disabled, and set a normal tabIndex.
| interface Pagination<T = Record<string, unknown>> { | ||
| data: T[]; // Array of items for the current page | ||
| links: Link[]; // Array of pagination links | ||
| current_page: number; // Current page number | ||
| last_page: number; // Total number of pages | ||
| first_page_url?: string; // URL for the first page | ||
| last_page_url?: string; // URL for the last page | ||
| next_page_url?: string | null; // URL for the next page | ||
| prev_page_url?: string | null; // URL for the previous page | ||
| path?: string; // Base path for pagination URLs | ||
| per_page: number; // Number of items per page | ||
| total: number; // Total number of items | ||
| from: number; // First item number on current page | ||
| to: number; // Last item number on current page | ||
| } | ||
|
|
||
| interface Link { | ||
| label: string; // Display label for the link | ||
| url: string | null; // URL for the link (null if disabled) | ||
| active: boolean; // Whether this is the current page | ||
| page: number | null; // Page number (null for Previous/Next) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename the interface to avoid value/type name collision and align with Laravel nullability.
Laravel’s from/to can be null on empty results. Also, using the same name as the component is confusing.
-interface Pagination<T = Record<string, unknown>> {
+interface PaginationData<T = Record<string, unknown>> {
data: T[]; // Array of items for the current page
links: Link[]; // Array of pagination links
current_page: number; // Current page number
last_page: number; // Total number of pages
first_page_url?: string; // URL for the first page
last_page_url?: string; // URL for the last page
next_page_url?: string | null; // URL for the next page
prev_page_url?: string | null; // URL for the previous page
path?: string; // Base path for pagination URLs
per_page: number; // Number of items per page
total: number; // Total number of items
- from: number; // First item number on current page
- to: number; // Last item number on current page
+ from: number | null; // First item number on current page (nullable)
+ to: number | null; // Last item number on current page (nullable)
}📝 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 Pagination<T = Record<string, unknown>> { | |
| data: T[]; // Array of items for the current page | |
| links: Link[]; // Array of pagination links | |
| current_page: number; // Current page number | |
| last_page: number; // Total number of pages | |
| first_page_url?: string; // URL for the first page | |
| last_page_url?: string; // URL for the last page | |
| next_page_url?: string | null; // URL for the next page | |
| prev_page_url?: string | null; // URL for the previous page | |
| path?: string; // Base path for pagination URLs | |
| per_page: number; // Number of items per page | |
| total: number; // Total number of items | |
| from: number; // First item number on current page | |
| to: number; // Last item number on current page | |
| } | |
| interface Link { | |
| label: string; // Display label for the link | |
| url: string | null; // URL for the link (null if disabled) | |
| active: boolean; // Whether this is the current page | |
| page: number | null; // Page number (null for Previous/Next) | |
| } | |
| ``` | |
| interface PaginationData<T = Record<string, unknown>> { | |
| data: T[]; // Array of items for the current page | |
| links: Link[]; // Array of pagination links | |
| current_page: number; // Current page number | |
| last_page: number; // Total number of pages | |
| first_page_url?: string; // URL for the first page | |
| last_page_url?: string; // URL for the last page | |
| next_page_url?: string | null; // URL for the next page | |
| prev_page_url?: string | null; // URL for the previous page | |
| path?: string; // Base path for pagination URLs | |
| per_page: number; // Number of items per page | |
| total: number; // Total number of items | |
| from: number | null; // First item number on current page (nullable) | |
| to: number | null; // Last item number on current page (nullable) | |
| } |
🤖 Prompt for AI Agents
In src/Components/Pagination/README.md around lines 28 to 50, the exported
interface name collides with the component name and the from/to fields should be
nullable to match Laravel; rename the interface to something like
PaginatedResponse or PaginationData (update any references/imports accordingly)
and change the types of from and to from number to number | null to reflect
empty-result cases; ensure any usage sites or docs are updated to the new name
and nullable types.
1 participant
This PR adds a component to handle pagination. This component can seamlessly handle pagination of data that is is Laravel's pagination format.
Key Features
Summary by CodeRabbit
New Features
Documentation
Chores