Skip to content

Pricefix2 #283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AshrafAbir merged 3 commits into from
Feb 22, 2026
Merged

Pricefix2 #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
014f3a3
up
AshrafAbir Feb 19, 2026
5898bee
up
AshrafAbir Feb 19, 2026
77be642
Merge branch 'main' into pricefix2
AshrafAbir Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions src/app/store/[slug]/components/product-details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
"use client";

import { useState, useMemo } from "react";
import { useState } from "react";
import { VariantSelector, type Variant } from "./variant-selector";
import { AddToCartButton } from "./add-to-cart-button";
import { PriceDisplay } from "./price-display";
import { StockBadge } from "./stock-badge";
import { Truck } from "lucide-react";
import { DiscountType, type DiscountInfo } from "@/lib/discount-utils";
import { DiscountType, type DiscountInfo, getEffectiveDiscount } from "@/lib/discount-utils";

interface ProductDetailsProps {
product: {
Expand Down Expand Up @@ -41,34 +41,45 @@ export function ProductDetails({
storeSlug,
onImageChange,
}: ProductDetailsProps) {
const defaultVariant = useMemo(
() => variants.find((v) => v.isDefault) || variants[0] || null,
[variants]
);

const [selectedVariant, setSelectedVariant] = useState<Variant | null>(defaultVariant);
// No variant pre-selected: show product.price initially, variant price only after user selection
const [selectedVariant, setSelectedVariant] = useState<Variant | null>(null);

// Calculate current price and inventory based on variant selection
// Show product default price until user explicitly picks a variant
const currentPrice = selectedVariant?.price ?? product.price;
const currentCompareAtPrice = selectedVariant?.compareAtPrice ?? product.compareAtPrice;
const currentInventory = selectedVariant?.inventoryQty ?? product.inventoryQty;
const currentInventory = selectedVariant ? selectedVariant.inventoryQty : product.inventoryQty;

// Build discount info for price display
// Build discount info for product and variant
const productDiscount: DiscountInfo = {
discountType: product.discountType ?? null,
discountValue: product.discountValue ?? null,
discountStartDate: product.discountStartDate ? new Date(product.discountStartDate) : null,
discountEndDate: product.discountEndDate ? new Date(product.discountEndDate) : null,
};

const variantDiscount: DiscountInfo | null = selectedVariant ? {
discountType: selectedVariant.discountType ?? null,
discountValue: selectedVariant.discountValue ?? null,
discountStartDate: selectedVariant.discountStartDate ? new Date(selectedVariant.discountStartDate) : null,
discountEndDate: selectedVariant.discountEndDate ? new Date(selectedVariant.discountEndDate) : null,
} : null;

// Use variant discount if active, otherwise fall back to product discount
const effectiveDiscount = getEffectiveDiscount(productDiscount, variantDiscount);

Comment on lines +60 to +69
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When no variant is selected (selectedVariant is null), the discount calculation falls back to productDiscount, but currentPrice also falls back to product.price. This creates a potential issue: if a variant has a different price than the product, the discount might be calculated incorrectly when no variant is selected.

For example, if the product has price 100 with 10% discount, but all variants have price 200 with no discount, showing "90" (discounted product price) when no variant is selected could be misleading if users can only purchase variants, not the base product.

Verify that the pricing logic aligns with the business requirements - should the product's base price and discount be shown when variants exist but none is selected?

Suggested change
const variantDiscount: DiscountInfo | null = selectedVariant ? {
discountType: selectedVariant.discountType ?? null,
discountValue: selectedVariant.discountValue ?? null,
discountStartDate: selectedVariant.discountStartDate ? new Date(selectedVariant.discountStartDate) : null,
discountEndDate: selectedVariant.discountEndDate ? new Date(selectedVariant.discountEndDate) : null,
} : null;
// Use variant discount if active, otherwise fall back to product discount
const effectiveDiscount = getEffectiveDiscount(productDiscount, variantDiscount);
const variantDiscount: DiscountInfo | null = selectedVariant
? {
discountType: selectedVariant.discountType ?? null,
discountValue: selectedVariant.discountValue ?? null,
discountStartDate: selectedVariant.discountStartDate ? new Date(selectedVariant.discountStartDate) : null,
discountEndDate: selectedVariant.discountEndDate ? new Date(selectedVariant.discountEndDate) : null,
}
: null;
// When variants exist and none is selected, do not show a product-level discount,
// to avoid advertising a price that may not be available on any purchasable variant.
const baseDiscountForDisplay: DiscountInfo =
variants.length > 0 && !selectedVariant
? {
discountType: null,
discountValue: null,
discountStartDate: null,
discountEndDate: null,
}
: productDiscount;
// Use variant discount if active, otherwise fall back to the base discount for display
const effectiveDiscount = getEffectiveDiscount(baseDiscountForDisplay, variantDiscount);

Copilot uses AI. Check for mistakes.
// Check if product has courier pricing
const hasCourierPricing = (product.courierPriceInsideDhaka ?? 0) > 0 || (product.courierPriceOutsideDhaka ?? 0) > 0;

const handleVariantChange = (variant: Variant) => {
setSelectedVariant(variant);
// Update image gallery if variant has an image
if (variant.image && onImageChange) {
onImageChange(variant.image);
// Toggle: if already selected, deselect it (go back to default product price)
if (selectedVariant?.id === variant.id) {
setSelectedVariant(null);
} else {
setSelectedVariant(variant);
// Update image gallery if variant has an image
if (variant.image && onImageChange) {
onImageChange(variant.image);
}
Comment on lines +74 to +82
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The toggle behavior introduced in handleVariantChange creates a confusing user experience. Clicking a selected variant now deselects it and returns to showing the product's default price, but this pattern is unexpected for product variant selection. In typical e-commerce flows, users expect variant selection to be persistent and not toggleable - they should switch between variants rather than toggle them on/off.

This behavior can be particularly confusing because:

  1. After deselecting, users won't know which variant will be added to the cart
  2. The image gallery update logic doesn't handle deselection (no reset to default product image)
  3. It's inconsistent with standard e-commerce UX patterns

Consider either removing the toggle behavior entirely or defaulting to the first/default variant instead of null.

Suggested change
// Toggle: if already selected, deselect it (go back to default product price)
if (selectedVariant?.id === variant.id) {
setSelectedVariant(null);
} else {
setSelectedVariant(variant);
// Update image gallery if variant has an image
if (variant.image && onImageChange) {
onImageChange(variant.image);
}
setSelectedVariant(variant);
// Update image gallery if variant has an image
if (variant.image && onImageChange) {
onImageChange(variant.image);

Copilot uses AI. Check for mistakes.
Comment on lines +76 to +82
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a variant is deselected (toggled off), the image gallery is not reset to the default product image. The current logic only handles updating the image when a variant is selected, but when setSelectedVariant(null) is called, the image remains on the last selected variant's image rather than reverting to the product's default image.

To fix this, add an else branch that resets the image when deselecting, or call onImageChange with the product's default thumbnail when setting the variant to null.

Copilot uses AI. Check for mistakes.
}
};

Expand All @@ -79,7 +90,7 @@ export function ProductDetails({
<PriceDisplay
price={currentPrice}
compareAtPrice={currentCompareAtPrice}
discount={productDiscount}
discount={effectiveDiscount}
size="lg"
/>
</div>
Expand Down Expand Up @@ -124,6 +135,7 @@ export function ProductDetails({
{variants.length >= 1 && (
<VariantSelector
variants={variants}
selectedVariantId={selectedVariant?.id ?? null}
onVariantChange={handleVariantChange}
/>
)}
Expand Down
21 changes: 12 additions & 9 deletions src/app/store/[slug]/components/variant-selector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"use client";

import { useState } from "react";
import { Label } from "@/components/ui/label";
import { Button } from "@/components/ui/button";
import { cn } from "@/lib/utils";
Expand All @@ -25,6 +24,7 @@ export interface Variant {

interface VariantSelectorProps {
variants: Variant[];
selectedVariantId?: string | null;
onVariantChange?: (variant: Variant) => void;
className?: string;
}
Expand All @@ -36,14 +36,17 @@ interface VariantSelectorProps {
*/
export function VariantSelector({
variants,
selectedVariantId,
onVariantChange,
className,
}: VariantSelectorProps) {
const defaultVariant = variants.find((v) => v.isDefault) || variants[0] || null;
const [selectedVariant, setSelectedVariant] = useState<Variant | null>(defaultVariant);
// Use external selectedVariantId if provided, otherwise manage locally
const selectedVariant = selectedVariantId
? variants.find((v) => v.id === selectedVariantId) ?? null
: null;

// Early return if no variants
if (variants.length === 0 || !selectedVariant) {
if (variants.length === 0) {
return null;
}

Expand All @@ -69,7 +72,6 @@ export function VariantSelector({
});

const handleVariantSelect = (variant: Variant) => {
setSelectedVariant(variant);
onVariantChange?.(variant);
};

Expand All @@ -80,12 +82,12 @@ export function VariantSelector({
<div className="space-y-2">
<Label>
Variant:{" "}
<span className="font-semibold">{selectedVariant.name}</span>
<span className="font-semibold">{selectedVariant ? selectedVariant.name : "Select a variant"}</span>
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variant selector now displays "Select a variant" when no variant is selected, which is good UX. However, there's a mismatch with the new behavior in product-details.tsx where no variant is pre-selected by default.

Consider the user flow: when a product page loads with variants, users see "Select a variant" in the selector, but the price displayed above is the product's default price (potentially with discount). If variants are required for purchase, this could be confusing. Consider either:

  1. Pre-selecting the default variant (reverting to the old behavior)
  2. Showing a message like "Price varies by variant - please select" when no variant is selected
  3. Ensuring the AddToCartButton is disabled when variants exist but none is selected
Suggested change
<span className="font-semibold">{selectedVariant ? selectedVariant.name : "Select a variant"}</span>
<span className="font-semibold">
{selectedVariant
? selectedVariant.name
: "Price varies by variant\u00a0-\u00a0please select"}
</span>

Copilot uses AI. Check for mistakes.
</Label>

<div className="flex flex-wrap gap-2">
{variants.map((variant) => {
const isSelected = selectedVariant.id === variant.id;
const isSelected = selectedVariant?.id === variant.id;
const isOutOfStock = variant.inventoryQty === 0;

return (
Expand Down Expand Up @@ -133,9 +135,10 @@ export function VariantSelector({
{optionName}:{" "}
<span className="font-semibold">
{(() => {
if (!selectedVariant) return "Select";
try {
const opts = JSON.parse(selectedVariant.options);
return opts[optionName];
return opts[optionName] || "";
} catch {
return "";
}
Expand All @@ -157,7 +160,7 @@ export function VariantSelector({

if (!matchingVariant) return null;

const isSelected = selectedVariant.id === matchingVariant.id;
const isSelected = selectedVariant?.id === matchingVariant.id;
const isOutOfStock = matchingVariant.inventoryQty === 0;

return (
Expand Down
Loading