feat:product carousel component #489
Conversation
WalkthroughIntroduces a new ProductCarousel component to the UI library with full TypeScript typing and Storybook documentation. The component renders a carousel of product cards with optional header, description, and action. Minor .gitignore update adds agent-os to the IDEs section. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/components/ProductCarousel/ProductCarousel.tsx (1)
45-61: Consider making the badge limit configurable.Line 53 hardcodes a limit of 2 badges with
.slice(0, 2). While this matches the ProductCard layout constraints, consider adding a prop likemaxBadges?: numberto ProductCarouselProps to make this configurable for different use cases.🔎 Proposed enhancement
Add to ProductCarouselProps in
ProductCarousel.types.ts:export interface ProductCarouselProps { products: ProductSummaryItem[]; title?: string; description?: Models.RichText.RichText; action?: React.ReactNode; LinkComponent: FrontendModels.Link.LinkComponent; carouselConfig?: Partial<CarouselProps>; detailsLabel: string; carouselClassName?: string; + maxBadges?: number; }Then update the component:
export const ProductCarousel: React.FC<ProductCarouselProps> = ({ products, title, description, action, LinkComponent, carouselConfig, detailsLabel, carouselClassName, + maxBadges = 2, }) => { // ... <ProductCard title={product.name} description={product.description} price={product.price} image={product.image} - tags={product.badges?.slice(0, 2)} + tags={product.badges?.slice(0, maxBadges)} link={{ label: detailsLabel, url: product.link, }} LinkComponent={LinkComponent} />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignorepackages/ui/src/components/ProductCarousel/ProductCarousel.stories.tsxpackages/ui/src/components/ProductCarousel/ProductCarousel.tsxpackages/ui/src/components/ProductCarousel/ProductCarousel.types.tspackages/ui/src/components/ProductCarousel/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/ui/src/components/ProductCarousel/ProductCarousel.stories.tsx (2)
packages/ui/src/components/ProductCarousel/ProductCarousel.tsx (1)
ProductCarousel(15-88)packages/ui/src/components/ProductCarousel/ProductCarousel.types.ts (1)
ProductSummaryItem(19-27)
packages/ui/src/components/ProductCarousel/ProductCarousel.tsx (4)
packages/ui/src/components/ProductCarousel/ProductCarousel.types.ts (1)
ProductCarouselProps(8-17)packages/ui/src/components/RichText/RichText.tsx (1)
RichText(48-252)packages/ui/src/components/Cards/ProductCard/ProductCard.tsx (1)
ProductCard(16-94)packages/ui/src/lib/utils.ts (1)
cn(5-7)
🔇 Additional comments (9)
.gitignore (1)
36-36: LGTM!The addition of
agent-osto the IDEs section follows the existing pattern and is appropriate.packages/ui/src/components/ProductCarousel/ProductCarousel.types.ts (1)
1-32: LGTM! Well-structured type definitions.The type definitions are comprehensive and follow framework conventions correctly. The required vs. optional field choices are appropriate, and the use of framework Models types ensures consistency across the codebase.
packages/ui/src/components/ProductCarousel/index.ts (1)
1-2: LGTM!The re-exports follow best practices with proper
export typesyntax for type-only exports.packages/ui/src/components/ProductCarousel/ProductCarousel.stories.tsx (3)
12-16: MockLinkComponent is appropriate for Storybook stories.The simplified link component is sufficient for demonstration purposes in Storybook.
32-141: LGTM! Well-structured sample data.The sample product data is comprehensive and demonstrates various product configurations with images, prices, descriptions, and badge combinations.
143-175: Stories demonstrate component usage effectively.The three stories (Default, WithDescription, WithAction) provide good coverage of the component's capabilities. The
console.logon line 168 is acceptable for Storybook demonstration purposes.packages/ui/src/components/ProductCarousel/ProductCarousel.tsx (3)
25-27: LGTM! Proper guard clause.The early return for empty products array prevents unnecessary rendering.
32-42: LGTM! Well-structured header section.The conditional rendering and responsive layout for the header section (title, description, action) is well implemented.
62-85: Verify the carouselConfig spread order is intentional.The
{...carouselConfig}spread at line 84 allows consumers to override all the hardcoded Carousel props (slidesPerView, breakpoints, showNavigation, showPagination, className). While this provides flexibility, it could lead to unexpected behavior if important defaults are accidentally overridden.Consider whether this level of override is intended, or if certain props should be protected from override.
Alternative approach if some defaults should be protected:
🔎 Suggested pattern for protected defaults
<Carousel + {...carouselConfig} slides={products.map((product) => ( // ... ))} slidesPerView={1} spaceBetween={16} showNavigation={true} showPagination={true} className={cn( '[&_.swiper-slide]:h-auto [&_.swiper-pagination]:bottom-0 [&_.swiper-pagination-bullet]:bg-primary [&_.swiper-pagination-bullet-active]:bg-primary [&_.swiper-pagination-bullet]:opacity-100 [&_.swiper-pagination-bullet]:w-2.5 [&_.swiper-pagination-bullet]:h-2.5', carouselClassName, + carouselConfig?.className, )} breakpoints={{ slidesPerView: 1, spaceBetween: 16, }, slidesPerView: 2, spaceBetween: 20, }, slidesPerView: 3, spaceBetween: 24, }, + ...carouselConfig?.breakpoints, }} - {...carouselConfig} />This pattern spreads
carouselConfigfirst, then applies component defaults that can't be overridden, while still allowing selective override of className and breakpoints through explicit merging.
|
it has already been merged into: #488 |
What does this PR do?
Related Ticket(s)
Key Changes
How to test
Media (Loom or gif)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.