Use Shadcn - #50
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a shadcn/Radix-based design system and UI primitives, introduces ProductImage/CategoryImage, tokenizes Tailwind theme, migrates many icons to lucide-react, removes SVGR handling, updates deps/config, and replaces numerous custom UI pieces with the new primitives; also changes customer register API to accept a params object. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/products/MediaGallery.tsx (2)
77-85:⚠️ Potential issue | 🟡 MinorSet
type="button"on the gallery controls.These buttons currently default to
submit. IfMediaGalleryis ever rendered inside a product form, clicking thumbnails or lightbox controls will submit that form unexpectedly.Possible fix
<button + type="button" key={image.id} onClick={() => setSelectedIndex(index)} className={`relative w-20 h-20 flex-shrink-0 rounded-xl overflow-hidden border-2 transition-colors bg-gray-100 ${ @@ <button + type="button" className="absolute top-4 right-4 text-white p-2 hover:bg-white/10 rounded-lg transition-colors" onClick={() => setIsZoomed(false)} aria-label="Close lightbox" @@ <button + type="button" className="absolute left-4 top-1/2 -translate-y-1/2 text-white p-2 hover:bg-white/10 rounded-lg transition-colors" onClick={(e) => { @@ <button + type="button" className="absolute right-4 top-1/2 -translate-y-1/2 text-white p-2 hover:bg-white/10 rounded-lg transition-colors" onClick={(e) => {Also applies to: 111-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/MediaGallery.tsx` around lines 77 - 85, The gallery buttons in MediaGallery are missing an explicit type and default to "submit", causing unintended form submissions; update every button element in MediaGallery (e.g., the thumbnail buttons that call setSelectedIndex and the lightbox control buttons referenced around the other button block) to include type="button" so clicks don't submit surrounding forms.
41-66:⚠️ Potential issue | 🟠 MajorMake the zoom trigger keyboard-accessible.
This is still a clickable
div, so keyboard users can't open the lightbox. Please use a realbuttonhere, or add equivalent button semantics and keyboard handling.Possible fix
- <div - className="relative aspect-square bg-gray-100 rounded-xl overflow-hidden cursor-zoom-in" + <button + type="button" + className="relative aspect-square bg-gray-100 rounded-xl overflow-hidden cursor-zoom-in focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring" onClick={() => mainImageUrl && setIsZoomed(true)} + aria-label={`Zoom image of ${selectedImage?.alt || productName}`} > {mainImageUrl ? ( <Image src={mainImageUrl} alt={selectedImage?.alt || productName} @@ {mainImageUrl && ( <div className="absolute bottom-4 right-4 bg-white/80 backdrop-blur-sm px-3 py-1.5 rounded-lg text-sm text-gray-600 flex items-center gap-1.5"> <ZoomIn className="w-4 h-4" /> Click to zoom </div> )} - </div> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/MediaGallery.tsx` around lines 41 - 66, The zoom trigger is a clickable div that isn’t keyboard-accessible; update the element that currently uses onClick={() => mainImageUrl && setIsZoomed(true)} (and the visual hint containing <ZoomIn /> / "Click to zoom") to be a real <button> or to add full button semantics: render a <button> (or element with role="button") that is focusable (tabIndex=0), has an accessible label (aria-label like "Open image zoom"), and handles activation via onClick and onKeyDown (treat Enter/Space the same as click) to call setIsZoomed(true) only when mainImageUrl is present; ensure styling and className are preserved and that the element is keyboard and screen-reader friendly.package.json (1)
40-40:⚠️ Potential issue | 🟡 MinorRemove
@svgr/webpackas it's unused. The package is not referenced in next.config.ts or anywhere else in the codebase, confirming it's an orphaned dependency from the previous icon system. Remove it from package.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 40, Remove the unused dependency "@svgr/webpack" from package.json and update the lockfile by running your package manager install (e.g., npm install or yarn install) so the dependency is removed from package-lock.json / yarn.lock; search for any remaining references to "@svgr/webpack" to confirm it's not used before committing the change.src/components/products/filters/MobileFilterDrawer.tsx (1)
42-46:⚠️ Potential issue | 🟠 MajorThis effect can clobber staged edits while the drawer is open.
If
activeFilterschanges from the parent while the sheet is open, this immediately overwritesstagedFiltersand discards the user's in-progress selections. Please switch to a reset-on-open/keyed-child pattern instead of syncing props back into local state withuseEffect. As per coding guidelines "Avoid using useEffect to reset state when props change; use component key prop to reset state or compute initial state from props".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/filters/MobileFilterDrawer.tsx` around lines 42 - 46, The useEffect in MobileFilterDrawer that sets stagedFilters when activeFilters changes can overwrite user edits while the sheet is open; remove that effect and stop syncing props into local state. Instead, implement a reset-on-open or keyed-child pattern: initialize stagedFilters from activeFilters only when the drawer is created (use initial state) and ensure the drawer is re-mounted when you want to reset by giving the MobileFilterDrawer (or its internal sheet child) a key derived from activeFilters (e.g., a filtersVersion or serialized activeFilters) or only reset stagedFilters when opening the drawer from closed-to-open; update references to isOpen, setStagedFilters, stagedFilters and activeFilters accordingly and remove the useEffect that currently syncs activeFilters into stagedFilters.
🧹 Nitpick comments (10)
src/components/products/ProductCard.tsx (1)
4-4: Consider using a more direct import alias.The alias
ImageIcon as ImagePlaceholderworks, but lucide-react provides anImageOfficon that might be more semantically appropriate for a "no image" placeholder scenario. Alternatively, keepingImageIconas-is without aliasing would be clearer.💡 Optional: Use ImageOff for better semantics
-import { ImageIcon as ImagePlaceholder } from "lucide-react"; +import { ImageOff as ImagePlaceholder } from "lucide-react";Or simply use without alias:
-import { ImageIcon as ImagePlaceholder } from "lucide-react"; +import { ImageIcon } from "lucide-react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductCard.tsx` at line 4, The import currently aliases ImageIcon to ImagePlaceholder; change this to a clearer import by either importing ImageIcon directly (remove the alias and update all usages of ImagePlaceholder to ImageIcon) or switch to lucide-react's ImageOff (import ImageOff and replace all ImagePlaceholder usages with ImageOff) so the placeholder icon name better matches semantics; update the JSX where ImagePlaceholder is used accordingly.src/app/[country]/[locale]/(storefront)/taxonomies/page.tsx (1)
22-22: Consider addinggenerateMetadatafor SEO.This page lacks a
generateMetadatafunction. While not directly related to the icon migration changes, the coding guidelines require dynamic SEO metadata generation for pages.📝 Example implementation
import type { Metadata } from "next"; export async function generateMetadata({ params }: CategoriesPageProps): Promise<Metadata> { const { locale } = await params; return { title: "Categories", description: "Browse all product categories", // Add locale-specific metadata as needed }; }As per coding guidelines: "Use generateMetadata function to dynamically generate SEO metadata for pages based on route params"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/taxonomies/page.tsx at line 22, Add a generateMetadata export to this page to provide dynamic SEO metadata based on route params: implement an exported async function named generateMetadata that accepts the same params type as CategoriesPage (CategoriesPageProps) and returns a Metadata object (title, description, and any locale-specific fields); ensure it extracts locale from params and returns appropriate strings so the page has dynamic SEO metadata alongside the existing export default async function CategoriesPage.src/components/ui/radio-group.tsx (1)
8-19: Consider adding explicit return types per coding guidelines.The
RadioGroupandRadioGroupItemfunctions lack explicit return types. While TypeScript can infer the return type, the coding guidelines specify explicit return types for functions.✨ Proposed fix to add explicit return types
function RadioGroup({ className, ...props -}: React.ComponentProps<typeof RadioGroupPrimitive.Root>) { +}: React.ComponentProps<typeof RadioGroupPrimitive.Root>): React.JSX.Element { return ( <RadioGroupPrimitive.Root data-slot="radio-group" className={cn("grid w-full gap-2", className)} {...props} /> ); } function RadioGroupItem({ className, ...props -}: React.ComponentProps<typeof RadioGroupPrimitive.Item>) { +}: React.ComponentProps<typeof RadioGroupPrimitive.Item>): React.JSX.Element { return (Also applies to: 21-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/radio-group.tsx` around lines 8 - 19, The RadioGroup and RadioGroupItem functions lack explicit return types; update their signatures to include an explicit JSX return type (e.g., add ": JSX.Element" or "React.ReactElement" to the function declarations for RadioGroup and RadioGroupItem) so the components conform to the project's TypeScript style guide and make the return type explicit.src/lib/utils.ts (1)
4-6: Add explicit return type annotation.Per coding guidelines, functions should have explicit return types. The
cnfunction should declare its return type.Suggested fix
-export function cn(...inputs: ClassValue[]) { +export function cn(...inputs: ClassValue[]): string { return twMerge(clsx(inputs)); }As per coding guidelines: "Use strict TypeScript type checking; define explicit return types for functions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils.ts` around lines 4 - 6, The cn function lacks an explicit return type—add a return type annotation to its signature (e.g., declare cn(...inputs: ClassValue[]): string) so the function explicitly returns a string produced by clsx and twMerge; update the cn function declaration to include ": string" while keeping the existing parameters and body referencing ClassValue, clsx, and twMerge.src/components/ui/textarea.tsx (1)
5-16: Add explicit return type annotation.Per coding guidelines, functions should have explicit return types.
Suggested fix
-function Textarea({ className, ...props }: React.ComponentProps<"textarea">) { +function Textarea({ className, ...props }: React.ComponentProps<"textarea">): React.JSX.Element { return (As per coding guidelines: "Use strict TypeScript type checking; define explicit return types for functions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/textarea.tsx` around lines 5 - 16, Add an explicit return type to the Textarea component: update the function signature for Textarea to include a return type such as JSX.Element or React.ReactElement (e.g., function Textarea(...): JSX.Element) so it conforms to the project's strict TypeScript rules; locate the Textarea function and annotate its signature accordingly while leaving props and implementation unchanged.src/app/[country]/[locale]/(checkout)/layout.tsx (1)
19-21: Consider addingpriorityprop for above-the-fold logo.The header logo is visible immediately on page load. Adding
priority={true}will preload the image and improve Largest Contentful Paint (LCP).Suggested improvement
- <Image src="/spree.png" alt="Spree Store" width={90} height={32} /> + <Image src="/spree.png" alt="Spree Store" width={90} height={32} priority />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/layout.tsx around lines 19 - 21, The header logo Image used inside the Link (the JSX with Link and Image) should be marked as a high-priority above-the-fold asset; update the Image element referenced here to include the Next.js priority prop (e.g., add priority={true} or priority) alongside the existing src, alt, width, and height so the logo is preloaded to improve LCP.src/app/[country]/[locale]/(storefront)/account/gift-cards/page.tsx (1)
162-173: Consider adding generateMetadata for SEO.Per coding guidelines, page files should use
generateMetadatato dynamically generate SEO metadata. This could be added to improve discoverability of the gift cards page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/gift-cards/page.tsx around lines 162 - 173, Add an exported generateMetadata function alongside the existing GiftCardsPage component that returns an object with title and description (and other SEO fields as needed) so the page has dynamic metadata; you can call getGiftCards or use route params to construct a descriptive title like "Gift Cards" and a helpful description, then export async function generateMetadata(...) { return { title: '...', description: '...' } }; keep the client-side GiftCardsPage and its useEffect/getGiftCards usage unchanged and ensure generateMetadata is exported at module scope.src/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsx (1)
24-284: Consider addinggenerateMetadatafor SEO.This dynamic route page lacks a
generateMetadataexport. While order confirmation pages may have limited SEO value, adding basic metadata improves consistency and can help with page titles in browser tabs/history.💡 Example metadata export
import type { Metadata } from "next"; export async function generateMetadata(): Promise<Metadata> { return { title: "Order Confirmation", robots: { index: false, follow: false }, // Prevent indexing of order pages }; }As per coding guidelines: "Use generateMetadata function to dynamically generate SEO metadata for pages based on route params."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx around lines 24 - 284, Add a generateMetadata export to this page to supply basic SEO info: import type { Metadata } from "next" and implement export async function generateMetadata(): Promise<Metadata> that returns at least a title like "Order Confirmation" and robots: { index: false, follow: false } (optionally use params/order data to include order number or customer name); place this alongside the OrderPlacedPage export so Next picks it up for the dynamic route.src/components/layout/Header.tsx (1)
24-24: Consider addingpriorityprop for the logo image.The logo is above the fold in the sticky header and likely contributes to LCP. Adding
priorityensures it's preloaded.💡 Suggested change
- <Image src="/spree.png" alt="Spree Store" width={90} height={32} /> + <Image src="/spree.png" alt="Spree Store" width={90} height={32} priority />As per coding guidelines: "Use Next.js Image component with optimization props (priority, placeholder, blurDataURL) instead of HTML img tags."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Header.tsx` at line 24, The logo Image in Header.tsx is missing the Next.js performance props; update the Image component (the JSX element rendering <Image src="/spree.png" alt="Spree Store" width={90} height={32} />) to include priority to preload it for LCP (and optionally add placeholder and blurDataURL if a low-quality placeholder is available); ensure you only add the priority prop (priority={true} or just priority) and, if adding placeholder/blurDataURL, provide a valid blurDataURL string.src/components/products/filters/AvailabilityDropdownContent.tsx (1)
2-5: Use radio items for single-select availability filter.The
selectedprop holds a singleAvailabilityStatus, but lines 26-44 render checkbox items, which announce a multi-select control. The actual behavior toggles between one selection and none (mutually exclusive). ReplaceDropdownMenuCheckboxItemwithDropdownMenuRadioGroupandDropdownMenuRadioItemto match the single-select semantics. SeeSortDropdownContent.tsxfor the same pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/filters/AvailabilityDropdownContent.tsx` around lines 2 - 5, The AvailabilityDropdownContent renders checkbox items while its selected prop is a single AvailabilityStatus; change the control to a radio group to match single-select semantics by replacing DropdownMenuCheckboxItem usages with a DropdownMenuRadioGroup that wraps DropdownMenuRadioItem entries, mirroring the pattern used in SortDropdownContent.tsx; update imports to bring in DropdownMenuRadioGroup and DropdownMenuRadioItem and ensure selection logic in AvailabilityDropdownContent (the selected prop and onSelect handler) maps to the radio group's value/onValueChange so a single option is chosen or cleared correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[country]/[locale]/(storefront)/account/page.tsx:
- Around line 85-95: The email and password inputs lost browser autofill
metadata; restore name and autoComplete props on the Input components (the one
with id="email" using value={email} and setEmail, and the other input using
id="password" and setPassword) so password managers can match credentials—add
name="email" autoComplete="email" to the email Input and name="current-password"
autoComplete="current-password" (or "new-password" if for signup) to the
password Input.
In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx:
- Around line 110-136: The per-item controls render identical buttons and allow
a no-op decrement; update the Button usages for the increment, decrement and
remove actions to include unique aria-labels (e.g., `aria-label={`Decrease
quantity for ${item.id}`}`, `aria-label={`Increase quantity for ${item.id}`}`,
`aria-label={`Remove item ${item.id}`}`) so each control is self-describing, and
set the decrement Button to disabled when `item.quantity === 1` to prevent
dispatching `updateItem(item.id, 1)`; keep calls to `updateItem(item.id,
item.quantity - 1)`, `updateItem(item.id, item.quantity + 1)`, and
`handleRemove(item)` unchanged otherwise.
In `@src/app/globals.css`:
- Line 10: The CSS defines a circular/undefined variable (--font-sans:
var(--font-sans)) while the body uses --font-geist and html is assigned a
Tailwind font class, causing inconsistent fonts; fix by making the variables
consistent: either set --font-sans to reference --font-geist (replace the
current --font-sans declaration with a direct mapping to --font-geist) or update
body to use var(--font-sans) so both html and body use the same variable; locate
and update the declarations for --font-sans, --font-geist, the body rule, and
the html font assignment to ensure a single authoritative font variable is used.
In `@src/components/cart/CartDrawer.tsx`:
- Around line 244-247: In the CartDrawer component update the user-visible
shipping copy: replace the misspelled string "Ccalculated on checkout" (the
second <span> in the div with className "flex justify-between items-center")
with the correct text "Calculated on checkout" so the UI shows the proper
spelling.
- Around line 65-69: The SheetContent is rendering SheetTitle without a
SheetDescription which triggers Radix Dialog accessibility warnings; update the
SheetContent usage in CartDrawer (the component that renders SheetTitle but no
SheetDescription) to explicitly pass aria-describedby={undefined} so the prop is
forwarded to the underlying Dialog.Content and suppresses the warning, i.e., add
the aria-describedby={undefined} attribute to the SheetContent element that
currently has side="right" className="w-full max-w-md flex flex-col p-0 gap-0"
showCloseButton={false}.
In `@src/components/checkout/AddressEditModal.tsx`:
- Around line 141-147: In AddressEditModal.tsx the Cancel Button renders as a
native button inside the form and will default to type="submit"; update the
Cancel Button (the Button with variant="outline" and onClick={onClose}) to
explicitly include type="button" so it does not submit the form when clicked and
only runs onClose.
In `@src/components/checkout/AddressFormFields.tsx`:
- Around line 105-160: Add explicit ids on the rendered controls and set
FieldLabel htmlFor to those ids so labels are programmatically associated: give
the country SelectTrigger an id like `${idPrefix}-country` and set the first
FieldLabel htmlFor to that id; for the state block give the SelectTrigger used
when hasStates an id like `${idPrefix}-state-select` and set the second
FieldLabel htmlFor to that id; for the fallback Input keep its existing id
`${idPrefix}-state` and also set the second FieldLabel htmlFor to that id (and
ensure the loading disabled SelectTrigger also receives an id
`${idPrefix}-state-select`), so FieldLabel, SelectTrigger and Input ids match
and restore accessibility.
In `@src/components/checkout/CouponCode.tsx`:
- Around line 7-8: The coupon input is placeholder-only and lacks an accessible
name; update the CouponCode component so the Field/Input pair provides a
programmatic label (e.g., render a visible or visually-hidden <label> tied to
the Input via htmlFor/id or add aria-label/aria-labelledby on the Input) so
screen readers can identify the control; locate the Field and Input usage in
CouponCode.tsx (both around the import of Field/Input and the input block at
lines ~96-116) and ensure the Input has a stable id and corresponding label or
aria attribute reflecting "Coupon code" (or similar) and remove reliance on
placeholder for accessibility.
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 289-291: The Edit and Back buttons inside the PaymentStep
component are rendered within a form with onSubmit={handleSubmit} and currently
default to type="submit", so update the Button instances that call onBack (the
one with onClick={onBack} and the other Back button in the same PaymentStep) to
explicitly set type="button" to prevent them from submitting the form; locate
the Button elements that use the onBack handler and add type="button" to each.
In `@src/components/layout/CountrySwitcher.tsx`:
- Around line 87-90: Replace the use of onClick on each DropdownMenuItem with
onSelect so keyboard users can activate items; specifically, remove the onClick
prop on the DropdownMenuItem instances (the ones keyed by c.iso) and use
onSelect={() => handleCountrySelect(c)} instead so handleCountrySelect is
invoked for both pointer and keyboard activation.
In `@src/components/navigation/Breadcrumbs.tsx`:
- Line 51: Breadcrumb link styling in Breadcrumbs.tsx currently uses
className="text-primary hover:text-primary" so hover shows no change; update the
class on the breadcrumb link element (the JSX element using
className="text-primary hover:text-primary") to use a distinct hover color such
as className="text-primary-500 hover:text-primary-700" (or your theme's darker
primary variant) so the link visually responds on hover.
In `@src/components/products/filters/FilterDropdown.tsx`:
- Around line 43-45: The aria-haspopup value on the dropdown trigger in
FilterDropdown.tsx is incorrect for a Radix DropdownMenu; update the attribute
from aria-haspopup="listbox" to aria-haspopup="menu" (or remove aria-haspopup
entirely) on the trigger element so it matches the menu semantics used by the
Radix DropdownMenu component (look for the JSX element setting aria-expanded and
aria-haspopup in the FilterDropdown component).
In `@src/components/products/filters/PriceDropdownContent.tsx`:
- Around line 32-45: The filter is using DropdownMenuCheckboxItem which gives
checkbox (multi-select) semantics but the UI is single-select; replace the
checkbox items with a radio group: wrap the list in DropdownMenuRadioGroup and
render each option with DropdownMenuRadioItem instead of
DropdownMenuCheckboxItem, preserving keys (bucket.id), labels (bucket.label) and
selection logic via isSelected and onPriceChange (call onPriceChange(undefined,
undefined) to clear, or onPriceChange(bucket.min, bucket.max) to select). Also
remove onCheckedChange/onSelect handlers incompatible with radio items and
ensure the radio group's value/checked state is driven by the same selected
bucket state.
In `@src/components/ui/checkbox.tsx`:
- Line 16: The Tailwind selectors using `data-checked:` are incorrect for Radix
UI state attributes; locate the class string in the Checkbox component (the
string containing `data-checked:border-primary data-checked:bg-primary
data-checked:text-primary-foreground` in checkbox.tsx) and replace each
`data-checked:` usage with the arbitrary data selector `data-[state=checked]:`
(e.g., `data-[state=checked]:border-primary`, `data-[state=checked]:bg-primary`,
etc.); do the same in the RadioGroup component where `data-checked:` selectors
appear (radio-group.tsx line with those selectors) so styles target Radix’s
`data-state="checked"` correctly.
In `@src/components/ui/dialog.tsx`:
- Around line 38-44: The Tailwind state selectors on DialogPrimitive.Overlay are
using non-existent boolean attributes (data-open/data-closed) so the animation
classes never apply; update the className construction in the
DialogPrimitive.Overlay (and similar uses) to use Radix's data-state attribute
selectors, e.g., replace data-open/data-closed selectors with
data-[state=open]:... and data-[state=closed]:... (so change occurrences in the
cn call that build animation classes for DialogPrimitive.Overlay and any zoom
variants), keeping the existing className and props spread logic intact.
In `@src/components/ui/input-group.tsx`:
- Around line 58-63: The onClick handler in src/components/ui/input-group.tsx
currently focuses only an <input> element, so clicks on addons for
textarea-based groups won't focus the control; update the handler (the onClick
closure) to locate the first focusable form control inside the group instead of
hard-coding "input" — e.g., use a selector that includes "textarea", "select",
contenteditable or role="textbox" (or otherwise detect focusable elements) and
call focus() on the found element; ensure you still skip clicks that originated
inside a button via the existing closest("button") check.
In `@src/components/ui/popover.tsx`:
- Around line 58-64: PopoverTitle is declared with React.ComponentProps<"h2">
but currently returns a <div>, losing heading semantics; update the PopoverTitle
component to render an actual <h2> element (preserving
data-slot="popover-title", className via cn("font-medium", className), and
spreading ...props) so the rendered element matches its typing and provides
correct accessibility/semantic behavior.
---
Outside diff comments:
In `@package.json`:
- Line 40: Remove the unused dependency "@svgr/webpack" from package.json and
update the lockfile by running your package manager install (e.g., npm install
or yarn install) so the dependency is removed from package-lock.json /
yarn.lock; search for any remaining references to "@svgr/webpack" to confirm
it's not used before committing the change.
In `@src/components/products/filters/MobileFilterDrawer.tsx`:
- Around line 42-46: The useEffect in MobileFilterDrawer that sets stagedFilters
when activeFilters changes can overwrite user edits while the sheet is open;
remove that effect and stop syncing props into local state. Instead, implement a
reset-on-open or keyed-child pattern: initialize stagedFilters from
activeFilters only when the drawer is created (use initial state) and ensure the
drawer is re-mounted when you want to reset by giving the MobileFilterDrawer (or
its internal sheet child) a key derived from activeFilters (e.g., a
filtersVersion or serialized activeFilters) or only reset stagedFilters when
opening the drawer from closed-to-open; update references to isOpen,
setStagedFilters, stagedFilters and activeFilters accordingly and remove the
useEffect that currently syncs activeFilters into stagedFilters.
In `@src/components/products/MediaGallery.tsx`:
- Around line 77-85: The gallery buttons in MediaGallery are missing an explicit
type and default to "submit", causing unintended form submissions; update every
button element in MediaGallery (e.g., the thumbnail buttons that call
setSelectedIndex and the lightbox control buttons referenced around the other
button block) to include type="button" so clicks don't submit surrounding forms.
- Around line 41-66: The zoom trigger is a clickable div that isn’t
keyboard-accessible; update the element that currently uses onClick={() =>
mainImageUrl && setIsZoomed(true)} (and the visual hint containing <ZoomIn /> /
"Click to zoom") to be a real <button> or to add full button semantics: render a
<button> (or element with role="button") that is focusable (tabIndex=0), has an
accessible label (aria-label like "Open image zoom"), and handles activation via
onClick and onKeyDown (treat Enter/Space the same as click) to call
setIsZoomed(true) only when mainImageUrl is present; ensure styling and
className are preserved and that the element is keyboard and screen-reader
friendly.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(checkout)/layout.tsx:
- Around line 19-21: The header logo Image used inside the Link (the JSX with
Link and Image) should be marked as a high-priority above-the-fold asset; update
the Image element referenced here to include the Next.js priority prop (e.g.,
add priority={true} or priority) alongside the existing src, alt, width, and
height so the logo is preloaded to improve LCP.
In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx:
- Around line 24-284: Add a generateMetadata export to this page to supply basic
SEO info: import type { Metadata } from "next" and implement export async
function generateMetadata(): Promise<Metadata> that returns at least a title
like "Order Confirmation" and robots: { index: false, follow: false }
(optionally use params/order data to include order number or customer name);
place this alongside the OrderPlacedPage export so Next picks it up for the
dynamic route.
In `@src/app/`[country]/[locale]/(storefront)/account/gift-cards/page.tsx:
- Around line 162-173: Add an exported generateMetadata function alongside the
existing GiftCardsPage component that returns an object with title and
description (and other SEO fields as needed) so the page has dynamic metadata;
you can call getGiftCards or use route params to construct a descriptive title
like "Gift Cards" and a helpful description, then export async function
generateMetadata(...) { return { title: '...', description: '...' } }; keep the
client-side GiftCardsPage and its useEffect/getGiftCards usage unchanged and
ensure generateMetadata is exported at module scope.
In `@src/app/`[country]/[locale]/(storefront)/taxonomies/page.tsx:
- Line 22: Add a generateMetadata export to this page to provide dynamic SEO
metadata based on route params: implement an exported async function named
generateMetadata that accepts the same params type as CategoriesPage
(CategoriesPageProps) and returns a Metadata object (title, description, and any
locale-specific fields); ensure it extracts locale from params and returns
appropriate strings so the page has dynamic SEO metadata alongside the existing
export default async function CategoriesPage.
In `@src/components/layout/Header.tsx`:
- Line 24: The logo Image in Header.tsx is missing the Next.js performance
props; update the Image component (the JSX element rendering <Image
src="/spree.png" alt="Spree Store" width={90} height={32} />) to include
priority to preload it for LCP (and optionally add placeholder and blurDataURL
if a low-quality placeholder is available); ensure you only add the priority
prop (priority={true} or just priority) and, if adding placeholder/blurDataURL,
provide a valid blurDataURL string.
In `@src/components/products/filters/AvailabilityDropdownContent.tsx`:
- Around line 2-5: The AvailabilityDropdownContent renders checkbox items while
its selected prop is a single AvailabilityStatus; change the control to a radio
group to match single-select semantics by replacing DropdownMenuCheckboxItem
usages with a DropdownMenuRadioGroup that wraps DropdownMenuRadioItem entries,
mirroring the pattern used in SortDropdownContent.tsx; update imports to bring
in DropdownMenuRadioGroup and DropdownMenuRadioItem and ensure selection logic
in AvailabilityDropdownContent (the selected prop and onSelect handler) maps to
the radio group's value/onValueChange so a single option is chosen or cleared
correctly.
In `@src/components/products/ProductCard.tsx`:
- Line 4: The import currently aliases ImageIcon to ImagePlaceholder; change
this to a clearer import by either importing ImageIcon directly (remove the
alias and update all usages of ImagePlaceholder to ImageIcon) or switch to
lucide-react's ImageOff (import ImageOff and replace all ImagePlaceholder usages
with ImageOff) so the placeholder icon name better matches semantics; update the
JSX where ImagePlaceholder is used accordingly.
In `@src/components/ui/radio-group.tsx`:
- Around line 8-19: The RadioGroup and RadioGroupItem functions lack explicit
return types; update their signatures to include an explicit JSX return type
(e.g., add ": JSX.Element" or "React.ReactElement" to the function declarations
for RadioGroup and RadioGroupItem) so the components conform to the project's
TypeScript style guide and make the return type explicit.
In `@src/components/ui/textarea.tsx`:
- Around line 5-16: Add an explicit return type to the Textarea component:
update the function signature for Textarea to include a return type such as
JSX.Element or React.ReactElement (e.g., function Textarea(...): JSX.Element) so
it conforms to the project's strict TypeScript rules; locate the Textarea
function and annotate its signature accordingly while leaving props and
implementation unchanged.
In `@src/lib/utils.ts`:
- Around line 4-6: The cn function lacks an explicit return type—add a return
type annotation to its signature (e.g., declare cn(...inputs: ClassValue[]):
string) so the function explicitly returns a string produced by clsx and
twMerge; update the cn function declaration to include ": string" while keeping
the existing parameters and body referencing ClassValue, clsx, and twMerge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 211892d4-e917-4e12-a27b-87363957f651
⛔ Files ignored due to path filters (34)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/spree.pngis excluded by!**/*.pngsrc/components/icons/arrow-left.svgis excluded by!**/*.svgsrc/components/icons/check-circle-solid.svgis excluded by!**/*.svgsrc/components/icons/check-circle.svgis excluded by!**/*.svgsrc/components/icons/check-solid.svgis excluded by!**/*.svgsrc/components/icons/check.svgis excluded by!**/*.svgsrc/components/icons/chevron-down.svgis excluded by!**/*.svgsrc/components/icons/chevron-left.svgis excluded by!**/*.svgsrc/components/icons/chevron-right.svgis excluded by!**/*.svgsrc/components/icons/clipboard-copy.svgis excluded by!**/*.svgsrc/components/icons/close.svgis excluded by!**/*.svgsrc/components/icons/credit-card.svgis excluded by!**/*.svgsrc/components/icons/eye-slash.svgis excluded by!**/*.svgsrc/components/icons/eye.svgis excluded by!**/*.svgsrc/components/icons/filter.svgis excluded by!**/*.svgsrc/components/icons/gift.svgis excluded by!**/*.svgsrc/components/icons/grid.svgis excluded by!**/*.svgsrc/components/icons/home.svgis excluded by!**/*.svgsrc/components/icons/image-placeholder.svgis excluded by!**/*.svgsrc/components/icons/info-circle.svgis excluded by!**/*.svgsrc/components/icons/lightning-bolt.svgis excluded by!**/*.svgsrc/components/icons/lock.svgis excluded by!**/*.svgsrc/components/icons/map-pin.svgis excluded by!**/*.svgsrc/components/icons/minus.svgis excluded by!**/*.svgsrc/components/icons/plus.svgis excluded by!**/*.svgsrc/components/icons/search-plus.svgis excluded by!**/*.svgsrc/components/icons/search.svgis excluded by!**/*.svgsrc/components/icons/shopping-bag.svgis excluded by!**/*.svgsrc/components/icons/sign-out.svgis excluded by!**/*.svgsrc/components/icons/spinner.svgis excluded by!**/*.svgsrc/components/icons/support.svgis excluded by!**/*.svgsrc/components/icons/user.svgis excluded by!**/*.svgsrc/components/icons/x-circle-solid.svgis excluded by!**/*.svg
📒 Files selected for processing (67)
components.jsonnext.config.tspackage.jsonsrc/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/addresses/page.tsxsrc/app/[country]/[locale]/(storefront)/account/credit-cards/page.tsxsrc/app/[country]/[locale]/(storefront)/account/gift-cards/page.tsxsrc/app/[country]/[locale]/(storefront)/account/layout.tsxsrc/app/[country]/[locale]/(storefront)/account/orders/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/orders/page.tsxsrc/app/[country]/[locale]/(storefront)/account/page.tsxsrc/app/[country]/[locale]/(storefront)/account/profile/page.tsxsrc/app/[country]/[locale]/(storefront)/account/register/page.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/app/[country]/[locale]/(storefront)/page.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.tsxsrc/app/[country]/[locale]/(storefront)/taxonomies/page.tsxsrc/app/global-error.tsxsrc/app/globals.csssrc/components/cart/CartDrawer.tsxsrc/components/checkout/AddressEditModal.tsxsrc/components/checkout/AddressFormFields.tsxsrc/components/checkout/AddressSelector.tsxsrc/components/checkout/AddressStep.tsxsrc/components/checkout/CouponCode.tsxsrc/components/checkout/DeliveryStep.tsxsrc/components/checkout/OrderSummary.tsxsrc/components/checkout/PaymentStep.tsxsrc/components/icons/index.tssrc/components/layout/CountrySwitcher.tsxsrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/components/navigation/Breadcrumbs.tsxsrc/components/products/MediaGallery.tsxsrc/components/products/ProductCard.tsxsrc/components/products/ProductCarousel.tsxsrc/components/products/ProductListingLayout.tsxsrc/components/products/VariantPicker.tsxsrc/components/products/filters/AvailabilityDropdownContent.tsxsrc/components/products/filters/FilterChips.tsxsrc/components/products/filters/FilterDropdown.tsxsrc/components/products/filters/MobileFilterDrawer.tsxsrc/components/products/filters/OptionDropdownContent.tsxsrc/components/products/filters/PriceDropdownContent.tsxsrc/components/products/filters/ProductFilters.tsxsrc/components/products/filters/SortDropdownContent.tsxsrc/components/search/SearchBar.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/checkbox.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/field.tsxsrc/components/ui/input-group.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/popover.tsxsrc/components/ui/radio-group.tsxsrc/components/ui/select.tsxsrc/components/ui/separator.tsxsrc/components/ui/sheet.tsxsrc/components/ui/textarea.tsxsrc/lib/utils.tssrc/lib/utils/filters.tsvitest.config.ts
💤 Files with no reviewable changes (2)
- next.config.ts
- src/components/icons/index.ts
- Unify header navigation icons (cart, account) with consistent Button ghost style - Extract reusable QuantityPicker component used across PDP, cart drawer, and cart page - Extract ProductCardSkeleton for DRY skeleton loading across product grid and carousel - Create CategoryImage wrapper to fix Server→Client component serialization - Create CategoryBanner component for taxonomy pages - Create ProductImage component with fallback placeholder icons - Add cursor-pointer to all interactive Button elements - Fix filter dropdown alignment (Color/Size vs Price/Availability) - Fix OrderSummary quantity badge clipping - Fix payment step loading spinner style - Fix quantity picker button gap (rounded-md vs rounded-xl mismatch) - Add disabled state styling for quantity picker minus button - Fix Next.js Image aspect ratio warning on logo - Remove deprecated resolveConditions from turbopack config - Migrate shadcn/ui components to latest patterns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
next.config.ts (1)
4-44:⚠️ Potential issue | 🟡 MinorRemove the stale SVG type declaration in
src/types/svg.d.ts.The SVGR webpack transform has been removed from
next.config.ts, butsrc/types/svg.d.tsstill declaresimport Icon from "./icon.svg"as a React component. While no code currently imports SVGs as components, this stale type declaration should be removed to prevent confusion and keep the type contract aligned with the bundler configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@next.config.ts` around lines 4 - 44, Remove the stale SVG component type declaration that asserts imports like `import Icon from "./icon.svg"` as React components: open the svg type file (the `svg.d.ts` declaration that contains the `import Icon from "./icon.svg"` / module declaration for "*.svg") and delete the entire declaration or remove the module augmentation that exposes SVGs as React components so the types match the removed SVGR transform; ensure no other type-only imports remain that assert SVGs are components.src/components/search/SearchBar.tsx (1)
149-168:⚠️ Potential issue | 🟠 MajorGive the search combobox a programmatic label.
The new
InputGroupkeeps this field placeholder-only.placeholder="Search..."is not a reliable accessible name, so screen-reader users get an unnamed combobox for a primary navigation control.💡 Proposed fix
<InputGroupInput ref={inputRef} type="search" + aria-label="Search products" value={query} onChange={(e) => { setQuery(e.target.value); setIsOpen(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/search/SearchBar.tsx` around lines 149 - 168, The combobox InputGroupInput (ref inputRef) currently relies on placeholder text and lacks a programmatic label; add an accessible name by providing either aria-label="Search" or aria-labelledby that points to a visible label element for the search input, and ensure the InputGroupInput keeps existing ARIA properties (role="combobox", aria-expanded, aria-controls, aria-activedescendant, aria-autocomplete) so screen readers get a proper label for the control.
♻️ Duplicate comments (2)
src/components/products/VariantPicker.tsx (1)
164-167:⚠️ Potential issue | 🟠 MajorMake the variant controls explicit non-submit buttons.
These buttons only change the selected variant. Inside a form,
<button>defaults to submit behavior unlesstype="button"is set, so selecting an option can submit instead of just updating the picker. (developer.mozilla.org)Proposed fix
return ( <button key={value} + type="button" onClick={() => handleOptionSelect(optionType.id, value)} disabled={!isAvailable} title={optionValue?.presentation || value} @@ - <Button + <Button key={value} + type="button" variant={isSelected ? "secondary" : "outline"} onClick={() => handleOptionSelect(optionType.id, value)} disabled={!isAvailable}Also applies to: 203-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/VariantPicker.tsx` around lines 164 - 167, The variant selection buttons inside VariantPicker are missing explicit type attributes so they act as submit buttons when rendered inside a form; update the <button> elements used for option selection (the ones invoking handleOptionSelect(optionType.id, value) and the other similar buttons later in the file) to include type="button" to prevent form submission and keep them as non-submit controls.src/components/ui/quantity-picker.tsx (1)
30-49:⚠️ Potential issue | 🟠 MajorMake the quantity buttons explicit non-submit controls.
This is a reusable input-like control. Inside a form, buttons submit by default unless they opt out explicitly, so increment/decrement can trigger form submission instead of just changing the quantity. (developer.mozilla.org)
Proposed fix
<Button + type="button" variant="ghost" size={buttonSize} className="rounded-l-xl rounded-r-none disabled:opacity-30" @@ <Button + type="button" variant="ghost" size={buttonSize} className="rounded-r-xl rounded-l-none disabled:opacity-30"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/quantity-picker.tsx` around lines 30 - 49, The increment/decrement controls are regular Button components that, when used inside a form, can act as submit buttons by default; update both Button usages (the one with onDecrement and the one with onIncrement) to explicitly opt out of form submission by adding type="button" to each Button element so they behave as non-submit controls while preserving existing props (variant, size, className, disabled, aria-label, and handlers).
🧹 Nitpick comments (7)
src/components/products/ProductGridSkeleton.tsx (1)
1-1: Use the@alias for this import.This new relative import goes against the repo’s TS/TSX import convention. Please switch it to the absolute alias form for consistency with the rest of the codebase.
As per coding guidelines, "Use absolute imports with @ alias (e.g.,
@/components/...,@/lib/...) instead of relative imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductGridSkeleton.tsx` at line 1, The import in ProductGridSkeleton.tsx uses a relative path for ProductCardSkeleton; change it to the repo alias form using the @ prefix (e.g., import { ProductCardSkeleton } from "@/components/products/ProductCardSkeleton") so it follows the project's absolute import convention and matches other TSX imports.src/components/ui/popover.tsx (1)
8-10: Add explicit return types to these shared component wrappers.These exported components currently rely on inference. In a shared UI module, explicit return types make type drift easier to catch and align with the repo TS rule.
Suggested change
function Popover({ ...props -}: React.ComponentProps<typeof PopoverPrimitive.Root>) { +}: React.ComponentProps<typeof PopoverPrimitive.Root>): React.JSX.Element { return <PopoverPrimitive.Root data-slot="popover" {...props} />; } function PopoverTrigger({ ...props -}: React.ComponentProps<typeof PopoverPrimitive.Trigger>) { +}: React.ComponentProps<typeof PopoverPrimitive.Trigger>): React.JSX.Element { return <PopoverPrimitive.Trigger data-slot="popover-trigger" {...props} />; } function PopoverContent({ className, align = "center", sideOffset = 4, ...props -}: React.ComponentProps<typeof PopoverPrimitive.Content>) { +}: React.ComponentProps<typeof PopoverPrimitive.Content>): React.JSX.Element { return ( <PopoverPrimitive.Portal> <PopoverPrimitive.Content data-slot="popover-content" align={align} sideOffset={sideOffset} className={cn( "z-50 flex w-72 origin-(--radix-popover-content-transform-origin) flex-col gap-2.5 rounded-lg bg-popover p-2.5 text-sm text-popover-foreground shadow-md ring-1 ring-foreground/10 outline-hidden duration-100 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95", className, )} {...props} /> </PopoverPrimitive.Portal> ); } function PopoverAnchor({ ...props -}: React.ComponentProps<typeof PopoverPrimitive.Anchor>) { +}: React.ComponentProps<typeof PopoverPrimitive.Anchor>): React.JSX.Element { return <PopoverPrimitive.Anchor data-slot="popover-anchor" {...props} />; } -function PopoverHeader({ className, ...props }: React.ComponentProps<"div">) { +function PopoverHeader({ className, ...props }: React.ComponentProps<"div">): React.JSX.Element { return ( <div data-slot="popover-header" className={cn("flex flex-col gap-0.5 text-sm", className)} {...props} /> ); } -function PopoverTitle({ className, ...props }: React.ComponentProps<"h2">) { +function PopoverTitle({ className, ...props }: React.ComponentProps<"h2">): React.JSX.Element { return ( <h2 data-slot="popover-title" className={cn("font-medium", className)} {...props} /> ); } function PopoverDescription({ className, ...props -}: React.ComponentProps<"p">) { +}: React.ComponentProps<"p">): React.JSX.Element { return ( <p data-slot="popover-description" className={cn("text-muted-foreground", className)} {...props} /> ); }As per coding guidelines,
**/*.ts{,x}:Use strict TypeScript type checking; define explicit return types for functions and avoid 'any' type.Also applies to: 14-16, 20-25, 42-48, 58-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/popover.tsx` around lines 8 - 10, The exported component wrappers (e.g., Popover, PopoverTrigger, PopoverContent, PopoverClose, PopoverPortal) currently rely on inferred return types—update each function signature to include an explicit React return type such as React.ReactElement | null (or JSX.Element | null) while keeping the existing props types (e.g., React.ComponentProps<typeof PopoverPrimitive.Root>); locate the functions by name (Popover, PopoverTrigger, PopoverContent, PopoverClose, PopoverPortal) and change their declarations to include the explicit return type to satisfy the repo TS rule.src/app/globals.css (1)
14-24: Two distinct "primary" color systems may cause visual inconsistency.The numbered primary scale (lines 14-24) uses blue hues (
#0077ff→#001d4d), while the semantic--color-primary(line 49) maps to--primarywhich is defined asoklch(0.205 0 0)— a near-black color.Components using
bg-primary-500will render blue, while components usingbg-primarywill render near-black. If this is intentional (legacy support + new shadcn), consider documenting the distinction. If not, align the color values.Also applies to: 48-49, 98-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/globals.css` around lines 14 - 24, There are two conflicting "primary" systems: the numeric scale (--color-primary-50..-950, e.g., --color-primary-500) is blue while the semantic --primary / --color-primary is oklch(0.205 0 0) (near-black), causing bg-primary-500 vs bg-primary to render differently; to fix, decide which primary you want and align them by either (A) mapping the semantic variable to the numeric scale (set --primary or --color-primary to var(--color-primary-500) / an appropriate step) or (B) update the numeric scale values to match the semantic primary, and if this dual-system is intentional add a clear comment near the declarations explaining the legacy vs shadcn distinction so consumers know when to use --color-primary-500 vs --primary (reference variables: --color-primary-500, --color-primary, --primary and classes/utility usages bg-primary-500, bg-primary).src/components/products/MediaGallery.tsx (1)
36-39: TightenselectImage's TypeScript signature.Use
SetStateAction<number>here and add an explicit: voidreturn type instead of spelling the setter union by hand. That keeps the helper aligned withsetSelectedIndexand matches the repo's TS rule.♻️ Suggested cleanup
-import { useState } from "react"; +import { useState, type SetStateAction } from "react"; @@ - const selectImage = (index: number | ((prev: number) => number)) => { + const selectImage = (index: SetStateAction<number>): void => { setSelectedIndex(index); setMainImageError(false); };As per coding guidelines "Use strict TypeScript type checking; define explicit return types for functions and avoid 'any' type".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/MediaGallery.tsx` around lines 36 - 39, Change the selectImage signature to use React's SetStateAction for consistency with setSelectedIndex and add an explicit void return type: replace the current parameter type union (index: number | ((prev: number) => number)) with (index: SetStateAction<number>) and annotate the function as (): void; keep the body unchanged (setSelectedIndex(index); setMainImageError(false);) so it matches the setter's type and repository TS rules.src/components/ui/quantity-picker.tsx (1)
5-5: Use the repository import alias for shared UI primitives.
src/components/ui/quantity-picker.tsxis importingButtonvia./button, but the repository guideline standardizes on@/...imports for TS/TSX files.Proposed fix
-import { Button } from "./button"; +import { Button } from "@/components/ui/button";As per coding guidelines, "Use absolute imports with @ alias (e.g.,
@/components/...,@/lib/...) instead of relative imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/quantity-picker.tsx` at line 5, Replace the relative import of Button in quantity-picker.tsx (the import statement that references "./button") with the repository alias import — import Button from "@/components/ui/button" — so the component uses the standardized `@/` absolute path for shared UI primitives; update any other relative UI imports in this file to use the `@/` alias as well.src/components/ui/sheet.tsx (1)
9-130: Add explicit return types to the new sheet primitives.
Sheet,SheetTrigger,SheetClose,SheetPortal,SheetOverlay,SheetContent,SheetHeader,SheetFooter,SheetTitle, andSheetDescriptionare all new function declarations without return annotations. Since this is a shared UI primitive module, it is worth making those return types explicit so TypeScript catches accidental non-JSX returns immediately.💡 Example
-function Sheet({ ...props }: React.ComponentProps<typeof SheetPrimitive.Root>) { +function Sheet( + { ...props }: React.ComponentProps<typeof SheetPrimitive.Root>, +): React.JSX.Element { return <SheetPrimitive.Root data-slot="sheet" {...props} />; }As per coding guidelines "Use strict TypeScript type checking; define explicit return types for functions and avoid 'any' type".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sheet.tsx` around lines 9 - 130, All sheet primitive functions lack explicit return types; update each function declaration (Sheet, SheetTrigger, SheetClose, SheetPortal, SheetOverlay, SheetContent, SheetHeader, SheetFooter, SheetTitle, SheetDescription) to include an explicit JSX return type (e.g., React.ReactElement or JSX.Element) on the function signature so TypeScript enforces correct JSX returns; ensure any cases that might return null are typed as React.ReactElement | null and keep existing prop types (e.g., React.ComponentProps<typeof SheetPrimitive.Root>, etc.) unchanged.src/components/products/filters/MobileFilterDrawer.tsx (1)
146-153: Consider full-width buttons for better mobile touch targets.The footer action buttons lack explicit width styling. For a mobile drawer, full-width buttons (
w-full) would provide larger touch targets and visual consistency withCartDrawer's footer buttons.♻️ Suggested improvement
{stagedCount > 0 && ( - <Button variant="ghost" onClick={handleClearAll}> + <Button variant="ghost" onClick={handleClearAll} className="w-full"> Clear all filters ({stagedCount}) </Button> )} - <Button onClick={handleApply}>Show results</Button> + <Button onClick={handleApply} className="w-full">Show results</Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/filters/MobileFilterDrawer.tsx` around lines 146 - 153, The footer buttons in MobileFilterDrawer (the Button elements that use stagedCount, handleClearAll and handleApply) should be full-width to improve mobile touch targets and match CartDrawer; update the two Button components to include the w-full (or equivalent full-width) styling/class and ensure spacing/stacking remains correct (e.g., vertical stack with proper gap) so both Clear all filters ({stagedCount}) and Show results render as full-width mobile-friendly buttons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[country]/[locale]/(checkout)/layout.tsx:
- Around line 19-26: The header currently hard-codes Spree branding in
layout.tsx (the Link/Image block using src="/spree.png" and alt="Spree Store"),
which breaks white-labeling; update the Image to use the runtime-configured
values (e.g., storeLogo or storeLogoPath for src and storeName for the alt text,
with a sensible fallback) while preserving layout props (width/height/priority)
and keep the Link basePath logic intact so the accessible name matches the
configured storeName.
- Around line 28-33: The mobile back link becomes icon-only below `sm`; add
accessibility attributes to the Link and ArrowLeft: set an explicit aria-label
(e.g., "Back to store") on the Link component and mark the ArrowLeft icon as
decorative by adding aria-hidden="true" (or the icon library's equivalent) so
screen readers get the label instead of the raw icon; update the JSX where Link
and ArrowLeft are used to include these attributes.
In `@src/app/`[country]/[locale]/(storefront)/account/page.tsx:
- Around line 102-128: The password toggle button is absolutely positioned over
the Input, causing long text/caret to be obscured; update the Input used in this
component (the Input element with id="password", value={password},
onChange={...}) to include extra right padding so the toggle has reserved space
(e.g., add a padding-right class or inline style large enough for the Button
width), and ensure the absolute Button (the Button with onClick={() =>
setShowPassword(!showPassword)} and aria-label using showPassword) remains
positioned as-is so it overlays the reserved padding area without overlapping
the input text.
In `@src/app/`[country]/[locale]/(storefront)/account/register/page.tsx:
- Around line 94-117: The password-toggle button is absolutely positioned over
the Input so caret/text can sit beneath it; update the Input(s) used for the
password fields (the Input with id="password" and the confirm-password Input) to
reserve space on the right by adding appropriate right padding (e.g., className
or style like pr-10) that matches the Button/icon width, or wrap the Input in a
container that applies padding-end; ensure the icon container (the Button using
setShowPassword and showPassword) width aligns with the padding so text/caret
never renders under the Eye/EyeOff icon.
In `@src/components/products/filters/AvailabilityDropdownContent.tsx`:
- Around line 24-48: The radio group never emits a deselect, so the
onChange(undefined) path is unreachable; add an explicit "clear" radio option
that emits an empty value and map that to undefined. Update the
DropdownMenuRadioGroup (in AvailabilityDropdownContent) to include an extra
DropdownMenuRadioItem (e.g., value "" or "none") rendered before mapping
filter.options so selecting it calls onValueChange with "" and the existing
onValueChange logic will call onChange(undefined); ensure AVAILABILITY_LABELS or
the item label covers this new value and keep isAvailabilityStatus usage intact
(or adjust isAvailabilityStatus to treat the cleared value as non-valid).
In `@src/components/products/filters/MobileFilterDrawer.tsx`:
- Around line 90-95: The SheetContent in MobileFilterDrawer is rendering a
visually hidden SheetTitle without a SheetDescription, which triggers Radix
Dialog accessibility warnings; update the SheetContent component (SheetContent)
to include aria-describedby={undefined} (same pattern used in CartDrawer) so the
dialog does not expect a missing description—apply this prop to the existing
SheetContent element enclosing SheetTitle.
In `@src/components/products/MediaGallery.tsx`:
- Around line 19-21: Clamp the gallery index against the current images prop
when deriving the displayed image instead of assuming selectedIndex is valid:
compute a safeSelectedIndex = Math.max(0, Math.min(selectedIndex, images.length
- 1)) and use that to compute selectedImage so it never becomes undefined;
additionally replace the boolean mainImageError with an error-scoped identifier
(e.g., mainImageErrorUrl) and set it in the image onError handler
(setMainImageErrorUrl(currentImage.url)) and consider the image errored only
when mainImageErrorUrl === selectedImage?.url; alternatively, if you prefer
component-level reset semantics, ensure the component is keyed by
product/variant identity so selectedIndex and error state reset when images
change.
In `@src/components/products/ProductCardSkeleton.tsx`:
- Around line 12-15: ProductCardSkeleton currently only reserves one title row
so loaded cards that clamp to two lines (see ProductCard.tsx name rendering)
cause layout shift; update ProductCardSkeleton to include a second title
placeholder above the price placeholder by adding another element matching the
existing title skeleton (e.g., another <div className="h-4 bg-gray-200 rounded
w-3/4 mb-2" /> or equivalent) so the skeleton's height matches the two-line
clamped title in ProductCard and prevents grid shift.
In `@src/components/ui/button.tsx`:
- Line 12: The default variant string in the Button variant map (the entry
currently set to "bg-primary text-primary-foreground [a]:hover:bg-primary/80")
uses the incorrect selector `[a]:hover:bg-primary/80`; replace that fragment
with `hover:bg-primary/80` so the default variant matches the hover pattern used
by the other variants (outline, secondary, ghost, destructive, link) and works
whether the Button is rendered directly or via asChild.
In `@src/components/ui/dropdown-menu.tsx`:
- Around line 45-46: The CSS selectors for Radix state in DropdownMenu
components are using incorrect data-open / data-closed attributes so open/closed
animations won't apply; update the className strings in DropdownMenuContent,
DropdownMenuSubTrigger, and DropdownMenuSubContent to replace all occurrences of
data-open: with data-[state=open]: and data-closed: with data-[state=closed]: so
the selectors match Radix's data-state="open|closed" attributes and restore the
animations and state-dependent styles.
---
Outside diff comments:
In `@next.config.ts`:
- Around line 4-44: Remove the stale SVG component type declaration that asserts
imports like `import Icon from "./icon.svg"` as React components: open the svg
type file (the `svg.d.ts` declaration that contains the `import Icon from
"./icon.svg"` / module declaration for "*.svg") and delete the entire
declaration or remove the module augmentation that exposes SVGs as React
components so the types match the removed SVGR transform; ensure no other
type-only imports remain that assert SVGs are components.
In `@src/components/search/SearchBar.tsx`:
- Around line 149-168: The combobox InputGroupInput (ref inputRef) currently
relies on placeholder text and lacks a programmatic label; add an accessible
name by providing either aria-label="Search" or aria-labelledby that points to a
visible label element for the search input, and ensure the InputGroupInput keeps
existing ARIA properties (role="combobox", aria-expanded, aria-controls,
aria-activedescendant, aria-autocomplete) so screen readers get a proper label
for the control.
---
Duplicate comments:
In `@src/components/products/VariantPicker.tsx`:
- Around line 164-167: The variant selection buttons inside VariantPicker are
missing explicit type attributes so they act as submit buttons when rendered
inside a form; update the <button> elements used for option selection (the ones
invoking handleOptionSelect(optionType.id, value) and the other similar buttons
later in the file) to include type="button" to prevent form submission and keep
them as non-submit controls.
In `@src/components/ui/quantity-picker.tsx`:
- Around line 30-49: The increment/decrement controls are regular Button
components that, when used inside a form, can act as submit buttons by default;
update both Button usages (the one with onDecrement and the one with
onIncrement) to explicitly opt out of form submission by adding type="button" to
each Button element so they behave as non-submit controls while preserving
existing props (variant, size, className, disabled, aria-label, and handlers).
---
Nitpick comments:
In `@src/app/globals.css`:
- Around line 14-24: There are two conflicting "primary" systems: the numeric
scale (--color-primary-50..-950, e.g., --color-primary-500) is blue while the
semantic --primary / --color-primary is oklch(0.205 0 0) (near-black), causing
bg-primary-500 vs bg-primary to render differently; to fix, decide which primary
you want and align them by either (A) mapping the semantic variable to the
numeric scale (set --primary or --color-primary to var(--color-primary-500) / an
appropriate step) or (B) update the numeric scale values to match the semantic
primary, and if this dual-system is intentional add a clear comment near the
declarations explaining the legacy vs shadcn distinction so consumers know when
to use --color-primary-500 vs --primary (reference variables:
--color-primary-500, --color-primary, --primary and classes/utility usages
bg-primary-500, bg-primary).
In `@src/components/products/filters/MobileFilterDrawer.tsx`:
- Around line 146-153: The footer buttons in MobileFilterDrawer (the Button
elements that use stagedCount, handleClearAll and handleApply) should be
full-width to improve mobile touch targets and match CartDrawer; update the two
Button components to include the w-full (or equivalent full-width) styling/class
and ensure spacing/stacking remains correct (e.g., vertical stack with proper
gap) so both Clear all filters ({stagedCount}) and Show results render as
full-width mobile-friendly buttons.
In `@src/components/products/MediaGallery.tsx`:
- Around line 36-39: Change the selectImage signature to use React's
SetStateAction for consistency with setSelectedIndex and add an explicit void
return type: replace the current parameter type union (index: number | ((prev:
number) => number)) with (index: SetStateAction<number>) and annotate the
function as (): void; keep the body unchanged (setSelectedIndex(index);
setMainImageError(false);) so it matches the setter's type and repository TS
rules.
In `@src/components/products/ProductGridSkeleton.tsx`:
- Line 1: The import in ProductGridSkeleton.tsx uses a relative path for
ProductCardSkeleton; change it to the repo alias form using the @ prefix (e.g.,
import { ProductCardSkeleton } from "@/components/products/ProductCardSkeleton")
so it follows the project's absolute import convention and matches other TSX
imports.
In `@src/components/ui/popover.tsx`:
- Around line 8-10: The exported component wrappers (e.g., Popover,
PopoverTrigger, PopoverContent, PopoverClose, PopoverPortal) currently rely on
inferred return types—update each function signature to include an explicit
React return type such as React.ReactElement | null (or JSX.Element | null)
while keeping the existing props types (e.g., React.ComponentProps<typeof
PopoverPrimitive.Root>); locate the functions by name (Popover, PopoverTrigger,
PopoverContent, PopoverClose, PopoverPortal) and change their declarations to
include the explicit return type to satisfy the repo TS rule.
In `@src/components/ui/quantity-picker.tsx`:
- Line 5: Replace the relative import of Button in quantity-picker.tsx (the
import statement that references "./button") with the repository alias import —
import Button from "@/components/ui/button" — so the component uses the
standardized `@/` absolute path for shared UI primitives; update any other
relative UI imports in this file to use the `@/` alias as well.
In `@src/components/ui/sheet.tsx`:
- Around line 9-130: All sheet primitive functions lack explicit return types;
update each function declaration (Sheet, SheetTrigger, SheetClose, SheetPortal,
SheetOverlay, SheetContent, SheetHeader, SheetFooter, SheetTitle,
SheetDescription) to include an explicit JSX return type (e.g.,
React.ReactElement or JSX.Element) on the function signature so TypeScript
enforces correct JSX returns; ensure any cases that might return null are typed
as React.ReactElement | null and keep existing prop types (e.g.,
React.ComponentProps<typeof SheetPrimitive.Root>, etc.) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd34fef1-4861-403a-b7e2-9965fef64cec
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (46)
next.config.tspackage.jsonsrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/orders/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/page.tsxsrc/app/[country]/[locale]/(storefront)/account/register/page.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.tsxsrc/app/[country]/[locale]/(storefront)/t/[...permalink]/page.tsxsrc/app/[country]/[locale]/(storefront)/taxonomies/page.tsxsrc/app/globals.csssrc/components/cart/CartDrawer.tsxsrc/components/checkout/AddressEditModal.tsxsrc/components/checkout/AddressFormFields.tsxsrc/components/checkout/CouponCode.tsxsrc/components/checkout/OrderSummary.tsxsrc/components/checkout/PaymentStep.tsxsrc/components/layout/CountrySwitcher.tsxsrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/components/navigation/Breadcrumbs.tsxsrc/components/navigation/CategoryBanner.tsxsrc/components/products/MediaGallery.tsxsrc/components/products/ProductCard.tsxsrc/components/products/ProductCardSkeleton.tsxsrc/components/products/ProductCarousel.tsxsrc/components/products/ProductGridSkeleton.tsxsrc/components/products/VariantPicker.tsxsrc/components/products/filters/AvailabilityDropdownContent.tsxsrc/components/products/filters/FilterDropdown.tsxsrc/components/products/filters/MobileFilterDrawer.tsxsrc/components/products/filters/PriceDropdownContent.tsxsrc/components/search/SearchBar.tsxsrc/components/ui/button.tsxsrc/components/ui/category-image.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/input-group.tsxsrc/components/ui/popover.tsxsrc/components/ui/product-image.tsxsrc/components/ui/quantity-picker.tsxsrc/components/ui/radio-group.tsxsrc/components/ui/sheet.tsxsrc/components/ui/textarea.tsxsrc/lib/utils.tssrc/lib/utils/color-map.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/components/products/ProductCarousel.tsx
- src/lib/utils.ts
- src/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.tsx
- src/components/ui/textarea.tsx
- src/components/layout/Footer.tsx
- package.json
- src/components/products/ProductCard.tsx
- src/components/ui/radio-group.tsx
- src/components/layout/Header.tsx
- src/components/ui/input-group.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/products/filters/AvailabilityDropdownContent.tsx (1)
23-47:⚠️ Potential issue | 🟡 MinorAdd an explicit clear option for this radio group.
DropdownMenuRadioGrouponly emits one of its item values, and the current items never emit"", so theonChange(undefined)branch on Lines 25-27 is unreachable in normal use. Once a shopper selects an availability value here, they can't clear it from this menu.💡 Proposed fix
<DropdownMenuRadioGroup value={selected ?? ""} onValueChange={(value) => { if (!value || !isAvailabilityStatus(value)) { onChange(undefined); } else { onChange(value); } }} > + <DropdownMenuRadioItem value=""> + Any availability + </DropdownMenuRadioItem> {filter.options.map((option) => ( <DropdownMenuRadioItem key={option.id} value={option.id} onSelect={(e) => e.preventDefault()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/filters/AvailabilityDropdownContent.tsx` around lines 23 - 47, The radio group never emits an empty value so the onValueChange branch that maps falsy values to onChange(undefined) is unreachable; add an explicit "Clear" radio item inside the same DropdownMenuRadioGroup (e.g., a DropdownMenuRadioItem with value of "" or a reserved CLEAR token) placed before the mapped filter.options so users can clear the selection, ensure its label (e.g., "Clear" or "All availabilities") and count if desired are shown, and keep the existing onValueChange logic that calls onChange(undefined) when the empty/clear value is selected; reference DropdownMenuRadioGroup, DropdownMenuRadioItem, selected, isAvailabilityStatus, onChange, and AVAILABILITY_LABELS to locate where to insert the new item.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/products/filters/AvailabilityDropdownContent.tsx`:
- Around line 23-47: The radio group never emits an empty value so the
onValueChange branch that maps falsy values to onChange(undefined) is
unreachable; add an explicit "Clear" radio item inside the same
DropdownMenuRadioGroup (e.g., a DropdownMenuRadioItem with value of "" or a
reserved CLEAR token) placed before the mapped filter.options so users can clear
the selection, ensure its label (e.g., "Clear" or "All availabilities") and
count if desired are shown, and keep the existing onValueChange logic that calls
onChange(undefined) when the empty/clear value is selected; reference
DropdownMenuRadioGroup, DropdownMenuRadioItem, selected, isAvailabilityStatus,
onChange, and AVAILABILITY_LABELS to locate where to insert the new item.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2fbd20af-5bea-4fdf-b498-dc9713053f48
📒 Files selected for processing (3)
src/components/products/filters/AvailabilityDropdownContent.tsxsrc/components/products/filters/OptionDropdownContent.tsxsrc/components/products/filters/PriceDropdownContent.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/products/filters/PriceDropdownContent.tsx
- src/components/products/filters/OptionDropdownContent.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/data/__tests__/customer.test.ts (1)
72-78: Consider adding a test case for optional fields.The current test always includes
first_nameandlast_name. Consider adding a test case that omits these optional fields to verify the API handles partial params correctly.Example additional test case
it("delegates without optional name fields", async () => { mockRegister.mockResolvedValue(mockUser); const result = await register({ email: "test@example.com", password: "pass", password_confirmation: "pass", }); expect(mockRegister).toHaveBeenCalledWith({ email: "test@example.com", password: "pass", password_confirmation: "pass", }); expect(result).toBe(mockUser); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/__tests__/customer.test.ts` around lines 72 - 78, Add a new test that calls register without the optional first_name and last_name to ensure partial params are accepted: mock the register implementation (mockRegister) to resolve to mockUser, call register with only email, password, and password_confirmation, then assert mockRegister was called with exactly that object and that the returned result equals mockUser; place this alongside the existing test(s) in customer.test.ts so the suite validates both full and minimal payloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[country]/[locale]/(storefront)/account/register/page.tsx:
- Line 17: The component uses useAuth() but only checks isAuthenticated; read
the loading flag from useAuth too and gate both the redirect logic and rendering
of the registration form until loading is false (e.g., return a
placeholder/spinner or null while loading). Update the references around
register/isAuthenticated to also destructure loading from useAuth, and ensure
any useEffect or conditional that redirects when isAuthenticated only runs after
loading === false so you avoid flashing the form or premature redirects.
---
Nitpick comments:
In `@src/lib/data/__tests__/customer.test.ts`:
- Around line 72-78: Add a new test that calls register without the optional
first_name and last_name to ensure partial params are accepted: mock the
register implementation (mockRegister) to resolve to mockUser, call register
with only email, password, and password_confirmation, then assert mockRegister
was called with exactly that object and that the returned result equals
mockUser; place this alongside the existing test(s) in customer.test.ts so the
suite validates both full and minimal payloads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fcd651bc-c1b4-41bb-86a8-43e4fa5b9f7b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/app/[country]/[locale]/(storefront)/account/register/page.tsxsrc/contexts/AuthContext.tsxsrc/lib/data/__tests__/customer.test.tssrc/lib/data/customer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/[country]/[locale]/(storefront)/account/register/page.tsx (1)
128-150:⚠️ Potential issue | 🟡 MinorReserve space for the password toggle inside both inputs.
The icon button is still absolutely positioned on top of the field, but neither input adds matching right padding. Text/caret can render under the toggle.
Proposed fix
<Input + className="pr-10" type={showPassword ? "text" : "password"} id="password" value={password} @@ <Input + className="pr-10" type={showPasswordConfirmation ? "text" : "password"} id="passwordConfirmation" value={passwordConfirmation}Also applies to: 160-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/register/page.tsx around lines 128 - 150, The password visibility toggle Button is absolutely positioned over the Input but the Input fields (password and confirm password) lack matching right padding so text/caret can render underneath; update the two Input usages (the one with id="password" and the confirm-password input around lines 160-186) to add sufficient right padding via their className or style (e.g., a right padding large enough to accommodate the Button width, such as pr-10) so the typed text and caret do not overlap the Button that toggles showPassword (state controlled by showPassword / setShowPassword).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[country]/[locale]/(storefront)/account/register/page.tsx:
- Around line 55-69: The registration flow in the page component calls
register(...) and sets submitting via setSubmitting(true) but does not handle
promise rejections, so setSubmitting(false) may never run; wrap the await
register(...) call in a try/catch/finally block around the existing logic (use
try to await register and check result.success/router.push(basePath +
'/account'), catch to setError with a fallback message when register throws, and
finally always call setSubmitting(false)) and ensure you reference the existing
symbols setSubmitting, register, setError, router.push and basePath so the flow
resets submitting in all failure paths.
---
Duplicate comments:
In `@src/app/`[country]/[locale]/(storefront)/account/register/page.tsx:
- Around line 128-150: The password visibility toggle Button is absolutely
positioned over the Input but the Input fields (password and confirm password)
lack matching right padding so text/caret can render underneath; update the two
Input usages (the one with id="password" and the confirm-password input around
lines 160-186) to add sufficient right padding via their className or style
(e.g., a right padding large enough to accommodate the Button width, such as
pr-10) so the typed text and caret do not overlap the Button that toggles
showPassword (state controlled by showPassword / setShowPassword).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05327f39-c200-41bb-9568-c96944f033c7
📒 Files selected for processing (1)
src/app/[country]/[locale]/(storefront)/account/register/page.tsx
…view
- Fix Button default variant hover (was only applying to <a> elements)
- Add type="button" to VariantPicker, QuantityPicker buttons
- Add aria-describedby={undefined} to MobileFilterDrawer SheetContent
- Add aria-label to SearchBar input and checkout back link
- Add pr-10 to password inputs for toggle button clearance
- Add full-width to MobileFilterDrawer footer buttons
- Gate register page on auth loading state
- Fix register page "Sign in" link hover color
- Delete stale svg.d.ts (SVGR no longer used)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… filter reset - Wrap register() call in try/catch/finally to prevent permanently locked submit button on network errors - Clamp MediaGallery selectedIndex to valid range when images change and scope image error state to URL instead of sticky boolean - Add "Any availability" option to AvailabilityDropdownContent so users can clear the filter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 10, 2026
Merged
Closed
This was referenced Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores