feat(products): Server-rendered Product Details Page with gallery, variant swatches, size picker, and recommendations #9
Conversation
…riants, size picker, and collapsibles; link product cards to dynamic route; add lucide-react Co-Authored-By: Adrian | JS Mastery <contact@jsmastery.pro>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughAdds lucide-react dependency. Introduces a product detail page with mock data and UI: gallery, color swatches, size picker, collapsible sections, and recommendations. Implements a variant selection Zustand store. Updates the products list to link to individual product pages. Re-exports new components from the components index. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Next.js Router as Router
participant ProductDetailPage as Page [/products/[id]]
participant ProductGallery as Gallery
participant ColorSwatches as Swatches
participant SizePicker as Sizes
participant VariantStore as useVariantStore
User->>Router: Navigate to /products/123
Router->>Page: Render with params {id}
Page->>Page: Load MOCK_PRODUCTS[id], compute discount
Page->>Gallery: props {productId, variants}
Page->>Swatches: props {productId, variants}
Page->>Sizes: Render size options
Note over Gallery,Swatches: Initial variant resolved via VariantStore.getSelected(fallback)
User->>Swatches: Click color swatch (index i)
Swatches->>VariantStore: setSelected(productId, i)
VariantStore-->>Gallery: selection updated
Gallery->>Gallery: Reset active image, show images for variant i
User->>Gallery: Arrow keys / controls to navigate images
Gallery->>Gallery: Update activeIndex (wraparound)
User->>Sizes: Toggle a size
Sizes->>Sizes: Track selected size locally
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
Co-Authored-By: Adrian | JS Mastery <contact@jsmastery.pro>
…e picker; extract ColorSwatches with shared variant store Co-Authored-By: Adrian | JS Mastery <contact@jsmastery.pro>
…ynamic ssr:false) Co-Authored-By: Adrian | JS Mastery <contact@jsmastery.pro>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
src/app/(root)/products/page.tsx (1)
63-69: Normalize price formatting to a consistent, currency-formatted string.Currently the single-price path passes a number while the range path passes a formatted string, which can yield inconsistent UI and potential type mismatches with Card.price.
Apply this diff to format single prices consistently:
- const price = - p.minPrice !== null && p.maxPrice !== null && p.minPrice !== p.maxPrice - ? `$${p.minPrice.toFixed(2)} - $${p.maxPrice.toFixed(2)}` - : p.minPrice !== null - ? p.minPrice - : undefined; + const price = + p.minPrice !== null && p.maxPrice !== null && p.minPrice !== p.maxPrice + ? `$${p.minPrice.toFixed(2)} - $${p.maxPrice.toFixed(2)}` + : p.minPrice !== null + ? `$${p.minPrice.toFixed(2)}` + : undefined;src/store/variant.ts (1)
5-21: Optional: add a clearSelected helper to avoid unbounded map growth.In long sessions, selectedByProduct can grow. A small helper enables clearing an entry or resetting all.
Apply this diff to extend the API:
type State = { selectedByProduct: Record<string, number>; setSelected: (productId: string, index: number) => void; getSelected: (productId: string, fallback?: number) => number; + clearSelected: (productId?: string) => void; }; export const useVariantStore = create<State>((set, get) => ({ selectedByProduct: {}, setSelected: (productId, index) => set((s) => ({ selectedByProduct: { ...s.selectedByProduct, [productId]: index }, })), getSelected: (productId, fallback = 0) => { const map = get().selectedByProduct; return map[productId] ?? fallback; }, + clearSelected: (productId) => + set((s) => { + if (!productId) return { selectedByProduct: {} }; + const { [productId]: _, ...rest } = s.selectedByProduct; + return { selectedByProduct: rest }; + }), }));src/components/SizePicker.tsx (3)
18-21: Prevent unintended form submissions: set button type="button".If this component is used inside a form, the “Size Guide” button defaults to type="submit".
Apply this diff:
- <button className="text-caption text-dark-700 underline-offset-2 hover:underline focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500]"> + <button + type="button" + className="text-caption text-dark-700 underline-offset-2 hover:underline focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500]" + >
27-36: Also set type="button" on size toggles.Avoids accidental submits when embedded in forms.
Apply this diff:
- <button + <button + type="button" key={s} onClick={() => setSelected(isActive ? null : s)}
23-39: Optional: prefer radiogroup semantics over toggle buttons for single selection.ARIA radios communicate mutual exclusivity better than multiple aria-pressed toggles.
You could:
- Wrap the grid with role="radiogroup" and an aria-label or aria-labelledby.
- Change buttons to role="radio", use aria-checked, and manage selection on Space/Enter.
- Optionally roving tabindex for better keyboard UX.
If you want, I can provide a small refactor diff.src/components/CollapsibleSection.tsx (2)
25-38: Make the trigger a true button and connect it to its content for accessibility.Add type="button" to avoid unintended form submits and wire aria-controls/ids for a robust disclosure pattern.
Apply this diff:
-import { useState } from "react"; +import { useId, useState } from "react"; @@ export default function CollapsibleSection({ @@ }: CollapsibleSectionProps) { - const [open, setOpen] = useState(defaultOpen); + const [open, setOpen] = useState(defaultOpen); + const contentId = useId(); @@ - <button - onClick={() => setOpen((o) => !o)} - className="flex w-full items-center justify-between gap-4 py-5 text-left focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500]" - aria-expanded={open} - > + <button + type="button" + onClick={() => setOpen((o) => !o)} + className="flex w-full items-center justify-between gap-4 py-5 text-left focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500]" + aria-expanded={open} + aria-controls={contentId} + > @@ - {open && ( - <div className="pb-6"> + {open && ( + <div id={contentId} role="region" className="pb-6"> {children ? <div className="text-body text-dark-700">{children}</div> : null} </div> )}
31-37: rightMeta inside the button should remain non-interactive.Because rightMeta is rendered inside the button, interactive elements (links/buttons) would be invalid and inaccessible. Either document this constraint or render rightMeta outside the button.
I can adjust the markup to place rightMeta adjacent to the button if you plan to pass interactive nodes.
src/components/index.ts (1)
6-8: Export ColorSwatches via the barrel for consistencyThe PDP imports ColorSwatches directly from its path, while the rest come from the components barrel. Re-exporting ColorSwatches keeps the public API consistent.
Apply this diff:
export { default as SocialProviders } from "./SocialProviders"; export { default as ProductGallery } from "./ProductGallery"; export { default as SizePicker } from "./SizePicker"; export { default as CollapsibleSection } from "./CollapsibleSection"; +export { default as ColorSwatches } from "./ColorSwatches";src/components/ColorSwatches.tsx (1)
24-36: ARIA semantics: buttons inside a listboxUsing role="listbox" with role="option" typically assumes an active descendant pattern (aria-activedescendant) rather than clickable buttons. Two options:
- Drop listbox/option roles and rely on native button semantics with aria-pressed, or
- Implement a roving tabindex with aria-activedescendant.
Minimal tweak if you choose the first option:
-<div className={`flex flex-wrap gap-3 ${className}`} role="listbox" aria-label="Choose color"> +<div className={`flex flex-wrap gap-3 ${className}`} aria-label="Choose color"> ... - aria-selected={isActive} - role="option" + aria-pressed={isActive}src/app/(root)/products/[id]/page.tsx (4)
2-5: Use the barrel export for ColorSwatches (after adding it to index.ts)For consistency with other components, import ColorSwatches from "@/components" after you re-export it.
-import { Card, CollapsibleSection, ProductGallery, SizePicker } from "@/components"; +import { Card, CollapsibleSection, ProductGallery, SizePicker, ColorSwatches } from "@/components"; -import ColorSwatches from "@/components/ColorSwatches";
80-82: Fix Next.js page props typing; no Promise for paramsApp Router passes params synchronously. Typing as Promise and awaiting it is unnecessary and misleading. This also makes TS types diverge from Next’s conventions.
-export default async function ProductDetailPage({ params }: { params: Promise<{ id: string }> }) { - const { id } = await params; +export default async function ProductDetailPage({ params }: { params: { id: string } }) { + const { id } = params;
105-117: Only render compare-at price when there’s an actual discountCurrently, the struck-through compareAt price shows even when it’s not greater than price. Gate the entire block on discount to avoid confusing UI.
- <div className="flex items-center gap-3"> - <p className="text-lead text-dark-900">${product.price.toFixed(2)}</p> - {product.compareAt && ( - <> - <span className="text-body text-dark-700 line-through">${product.compareAt.toFixed(2)}</span> - {discount !== null && ( - <span className="rounded-full border border-light-300 px-2 py-1 text-caption text-[--color-green]"> - {discount}% off - </span> - )} - </> - )} - </div> + <div className="flex items-center gap-3"> + <p className="text-lead text-dark-900">${product.price.toFixed(2)}</p> + {discount !== null && ( + <> + <span className="text-body text-dark-700 line-through">${product.compareAt!.toFixed(2)}</span> + <span className="rounded-full border border-light-300 px-2 py-1 text-caption text-[--color-green]"> + {discount}% off + </span> + </> + )} + </div>
80-84: Optional: use notFound() for unknown product IDsInstead of falling back to the first product, consider returning 404 for unknown ids to match user expectations.
If desired:
-import { Card, CollapsibleSection, ProductGallery, SizePicker, ColorSwatches } from "@/components"; +import { Card, CollapsibleSection, ProductGallery, SizePicker, ColorSwatches } from "@/components"; +import { notFound } from "next/navigation"; ... - const product = MOCK_PRODUCTS[id] ?? Object.values(MOCK_PRODUCTS)[0]; + const product = MOCK_PRODUCTS[id]; + if (!product) return notFound();src/components/ProductGallery.tsx (3)
35-41: Use a single source of truth for selected index; respect bounded fallbackAlign with the store API (getSelected) and keep the bounded fallback you already compute. This reduces coupling to the store’s internal shape and ensures the default index is valid for the filtered variants.
- const variantIndex = - useVariantStore( - (s) => s.selectedByProduct[productId] ?? Math.min(initialVariantIndex, Math.max(validVariants.length - 1, 0)) - ); + const fallbackIndex = Math.min(initialVariantIndex, Math.max(validVariants.length - 1, 0)); + const variantIndex = useVariantStore( + (s) => (typeof s.getSelected === "function" ? s.getSelected(productId, fallbackIndex) : s.selectedByProduct?.[productId] ?? fallbackIndex) + );
72-79: Add type="button" to clickable controlsPrevents accidental form submission if this component is ever used inside a form.
<button key={`${src}-${i}`} aria-label={`Thumbnail ${i + 1}`} onClick={() => setActiveIndex(i)} + type="button" className={`relative h-16 w-16 flex-shrink-0 overflow-hidden rounded-lg ring-1 ring-light-300 focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500] ${i === activeIndex ? "ring-[--color-dark-500]" : ""}`} > ... <button aria-label="Previous image" onClick={() => go(-1)} + type="button" className="rounded-full bg-light-100/80 p-2 ring-1 ring-light-300 transition hover:bg-light-100 focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500]" > ... <button aria-label="Next image" onClick={() => go(1)} + type="button" className="rounded-full bg-light-100/80 p-2 ring-1 ring-light-300 transition hover:bg-light-100 focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500]" >Also applies to: 96-109
56-66: Optional: Simplify keyboard handling by scoping to the main containerInstead of a window keydown listener with contains checks, attach onKeyDown to the main container and make it focusable. This reduces global listeners and makes intent explicit.
Sketch:
<div ref={mainRef} tabIndex={0} onKeyDown={(e) => { if (e.key === "ArrowLeft") { e.preventDefault(); go(-1); } if (e.key === "ArrowRight") { e.preventDefault(); go(1); } }} className="order-1 relative w-full h-[500px] ..." >
📜 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 ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.json(1 hunks)src/app/(root)/products/[id]/page.tsx(1 hunks)src/app/(root)/products/page.tsx(1 hunks)src/components/CollapsibleSection.tsx(1 hunks)src/components/ColorSwatches.tsx(1 hunks)src/components/ProductGallery.tsx(1 hunks)src/components/SizePicker.tsx(1 hunks)src/components/index.ts(1 hunks)src/store/variant.ts(1 hunks)
🔇 Additional comments (4)
package.json (1)
26-28: lucide-react addition looks correct; ensure tree-shaking by keeping named imports.The dependency aligns with usage in new components. Using named imports from lucide-react will keep bundles lean via tree-shaking.
src/app/(root)/products/page.tsx (2)
70-77: LGTM: Card now links to product detail pages.Adding href={
/products/${p.id}} is the expected navigation for PDP.
70-77: No action needed: CardProps already support number for price and string for hrefVerified in src/components/Card.tsx:
price?: string | numberhref?: stringPassing a numeric
priceand the newhrefprop aligns with the existing type definitions.src/store/variant.ts (1)
11-21: Simple, clear Zustand store for per-product variant selection.API is minimal and typed; client-only directive prevents SSR import errors. Good use of functional set.
| export default function ColorSwatches({ productId, variants, className = "" }: ColorSwatchesProps) { | ||
| const setSelected = useVariantStore((s) => s.setSelected); | ||
| const selected = useVariantStore((s) => s.getSelected(productId, 0)); | ||
|
|
||
| return ( | ||
| <div className={`flex flex-wrap gap-3 ${className}`} role="listbox" aria-label="Choose color"> | ||
| {variants.map((v, i) => { | ||
| const src = firstValidImage(v.images); | ||
| if (!src) return null; | ||
| const isActive = selected === i; | ||
| return ( | ||
| <button | ||
| key={`${v.color}-${i}`} | ||
| onClick={() => setSelected(productId, i)} | ||
| aria-label={`Color ${v.color}`} | ||
| aria-selected={isActive} | ||
| role="option" | ||
| className={`relative h-[72px] w-[120px] overflow-hidden rounded-lg ring-1 ring-light-300 transition focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500] ${ | ||
| isActive ? "ring-[--color-dark-500]" : "hover:ring-dark-500" | ||
| }`} | ||
| > | ||
| <Image src={src} alt={v.color} fill sizes="120px" className="object-cover" /> | ||
| {isActive && ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Variant index desync with ProductGallery (filtering mismatch) — align on the same filtered index set
ProductGallery filters out variants without valid images and uses indices into that filtered list. ColorSwatches currently uses indices from the unfiltered variants array (skipping null-rendered items), which can desynchronize the selected index across components. Result: clicking a swatch may show a different variant in the gallery.
Apply this diff to:
- Use the same filtered set (only variants with a valid image).
- Set/select indices based on the filtered list.
- Add type="button" to avoid accidental form submission.
"use client";
-import Image from "next/image";
+import Image from "next/image";
+import { useMemo } from "react";
import { Check } from "lucide-react";
import { useVariantStore } from "@/store/variant";
type Variant = { color: string; images: string[] };
export interface ColorSwatchesProps {
productId: string;
variants: Variant[];
className?: string;
}
function firstValidImage(images: string[]) {
return images.find((s) => typeof s === "string" && s.trim().length > 0);
}
export default function ColorSwatches({ productId, variants, className = "" }: ColorSwatchesProps) {
const setSelected = useVariantStore((s) => s.setSelected);
- const selected = useVariantStore((s) => s.getSelected(productId, 0));
+ const selected = useVariantStore((s) => s.getSelected(productId, 0));
+
+ // Use the same filtering rule as the gallery: only variants with a valid image.
+ const valid = useMemo(
+ () =>
+ variants
+ .map((v) => ({ v, src: firstValidImage(v.images) }))
+ .filter((e): e is { v: Variant; src: string } => Boolean(e.src)),
+ [variants]
+ );
return (
- <div className={`flex flex-wrap gap-3 ${className}`} role="listbox" aria-label="Choose color">
- {variants.map((v, i) => {
- const src = firstValidImage(v.images);
- if (!src) return null;
- const isActive = selected === i;
+ <div className={`flex flex-wrap gap-3 ${className}`} role="listbox" aria-label="Choose color">
+ {valid.map(({ v, src }, idx) => {
+ const isActive = selected === idx;
return (
<button
- key={`${v.color}-${i}`}
- onClick={() => setSelected(productId, i)}
+ key={`${v.color}-${idx}`}
+ type="button"
+ onClick={() => setSelected(productId, idx)}
aria-label={`Color ${v.color}`}
aria-selected={isActive}
role="option"
className={`relative h-[72px] w-[120px] overflow-hidden rounded-lg ring-1 ring-light-300 transition focus:outline-none focus-visible:ring-2 focus-visible:ring-[--color-dark-500] ${
isActive ? "ring-[--color-dark-500]" : "hover:ring-dark-500"
}`}
>
- <Image src={src} alt={v.color} fill sizes="120px" className="object-cover" />
+ <Image src={src} alt={v.color} fill sizes="120px" className="object-cover" />
{isActive && (
<span className="absolute right-1 top-1 rounded-full bg-light-100 p-1">
<Check className="h-4 w-4 text-dark-900" />
</span>
)}
</button>
);
})}
</div>
);
}Also applies to: 25-49, 3-6
🤖 Prompt for AI Agents
In src/components/ColorSwatches.tsx around lines 19-41, the component maps over
the original variants array and skips variants without valid images, causing
index desync with ProductGallery; instead compute a filteredVariants =
variants.filter(v => firstValidImage(v.images)) (and a corresponding src per
item), map over filteredVariants so indices align, call setSelected(productId,
i) and use the same index i everywhere, and ensure the button elements include
type="button" to prevent accidental form submission.
Server-rendered Product Details Page (PDP)
Implements a fully server-rendered Product Details Page at
/products/[id]with interactive client components for gallery, color swatches, size picker, and collapsible sections. Matches the provided design, is responsive, and uses Tailwind with globals.css tokens.Link to Devin run: https://app.devin.ai/sessions/08f144f15fca4a38bd7eabe5914fca0f
Requested by: Adrian | JS Mastery (@adrianhajdin)
Key points:
src/app/(root)/products/[id]/page.tsxcomponents/ProductGallery.tsx(gallery, thumbnails, arrow keys, swatches w/ Check icon, ImageOff fallback)components/SizePicker.tsx(keyboard-accessible size selection)components/CollapsibleSection.tsx(accordions)Cardand links to/products/[id]/public/shoes/*vianext/imageScreenshots:
Summary by CodeRabbit