feat: add Pagination component and related exports - #79
Conversation
📝 WalkthroughWalkthroughAdded a complete pagination UI component system ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (2)
src/components/ui/pagination.tsx (1)
68-100: Consider adding disabled state support for first/last page scenarios.The components work well for navigation, but users may want to disable
PaginationPreviouson the first page andPaginationNexton the last page. Currently, this would require consumers to conditionally render or apply custom styling.Consider adding a
disabledprop that setsaria-disabled="true"and applies disabled styling. This is optional since consumers can work around it.💡 Optional enhancement for disabled state
type PaginationPreviousProps = React.ComponentProps<typeof PaginationLink> & { disabled?: boolean; }; function PaginationPrevious({ className, disabled, ...props }: PaginationPreviousProps) { return ( <PaginationLink aria-label="Go to previous page" aria-disabled={disabled || undefined} size="default" className={cn( "gap-1 px-2.5 sm:pl-2.5", disabled && "pointer-events-none opacity-50", className )} {...props} > <ChevronLeftIcon /> <span className="hidden sm:block">Previous</span> </PaginationLink> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/pagination.tsx` around lines 68 - 100, Add optional disabled support to PaginationPrevious and PaginationNext: extend their props from React.ComponentProps<typeof PaginationLink> with a disabled?: boolean, pass aria-disabled={disabled || undefined} into PaginationLink, and conditionally include disabled styling classes (e.g., "pointer-events-none opacity-50") in the cn(...) call so the buttons are visually and programmatically disabled without consumers having to special-case rendering; update both functions (PaginationPrevious and PaginationNext) to accept the new prop and forward ...props as before.src/components/ui/Pagination.stories.tsx (1)
142-166: The[&_span]:hiddenselector is fragile.This CSS selector relies on the internal structure of
PaginationPrevious/PaginationNextcomponents. If the component implementation changes (e.g., span becomes a different element), this story will break silently.Consider documenting this pattern or adding a dedicated
iconOnlyprop to the Previous/Next components if this is a common use case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Pagination.stories.tsx` around lines 142 - 166, The story uses a fragile descendant selector "[&_span]:hidden" on PaginationPrevious and PaginationNext which depends on their internal markup; update the components to support an explicit icon-only mode and change the story to use that instead: add an iconOnly (or hideLabel) prop to PaginationPrevious and PaginationNext (and propagate it through PaginationItem/PaginationContent if needed), implement conditional rendering or a dedicated class inside those components to hide the label without depending on a span tag, and update this IconsOnly story to pass iconOnly to PaginationPrevious and PaginationNext; also add a short comment/doc note in the story or component docs explaining the new prop for future maintainers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/ui/Pagination.stories.tsx`:
- Around line 142-166: The story uses a fragile descendant selector
"[&_span]:hidden" on PaginationPrevious and PaginationNext which depends on
their internal markup; update the components to support an explicit icon-only
mode and change the story to use that instead: add an iconOnly (or hideLabel)
prop to PaginationPrevious and PaginationNext (and propagate it through
PaginationItem/PaginationContent if needed), implement conditional rendering or
a dedicated class inside those components to hide the label without depending on
a span tag, and update this IconsOnly story to pass iconOnly to
PaginationPrevious and PaginationNext; also add a short comment/doc note in the
story or component docs explaining the new prop for future maintainers.
In `@src/components/ui/pagination.tsx`:
- Around line 68-100: Add optional disabled support to PaginationPrevious and
PaginationNext: extend their props from React.ComponentProps<typeof
PaginationLink> with a disabled?: boolean, pass aria-disabled={disabled ||
undefined} into PaginationLink, and conditionally include disabled styling
classes (e.g., "pointer-events-none opacity-50") in the cn(...) call so the
buttons are visually and programmatically disabled without consumers having to
special-case rendering; update both functions (PaginationPrevious and
PaginationNext) to accept the new prop and forward ...props as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53a28752-36d7-4910-a891-56a417b4f474
📒 Files selected for processing (5)
src/components/ui/Pagination.stories.tsxsrc/components/ui/index.tssrc/components/ui/pagination.tsxsrc/components/ui/smart-multi-select.tsxsrc/index.ts
Summary by CodeRabbit
New Features
Documentation