From e9ffa6df4e23d984fe4a380c26be580b7b57ca67 Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 10 Aug 2026 10:47:36 +0200 Subject: [PATCH] feat(quotes/process): add mobile card layout for quote items Replace the cramped horizontal table layout on mobile viewports with responsive vertical cards for each quote item. The new `ProcessItemCard` component provides full-width form fields, properly sized 44px tap targets for all actions, and maintains full parity with the desktop table implementation by reusing the same state handlers and callbacks. Add comprehensive test coverage for the new component, and fix existing mobile overflow issues in the quote process page header and table layout. --- src/app/(app)/quotes/[id]/process/page.tsx | 317 ++++++++---- .../quotes/process-item-card.test.tsx | 322 ++++++++++++ src/components/quotes/process-item-card.tsx | 482 ++++++++++++++++++ 3 files changed, 1031 insertions(+), 90 deletions(-) create mode 100644 src/components/quotes/process-item-card.test.tsx create mode 100644 src/components/quotes/process-item-card.tsx diff --git a/src/app/(app)/quotes/[id]/process/page.tsx b/src/app/(app)/quotes/[id]/process/page.tsx index 1dd54e2..e042255 100644 --- a/src/app/(app)/quotes/[id]/process/page.tsx +++ b/src/app/(app)/quotes/[id]/process/page.tsx @@ -48,6 +48,7 @@ import { ProcessOriginalEmailCard } from "@/components/quotes/process-original-e import { ProcessAddItemForm } from "@/components/quotes/process-add-item-form"; import { ProcessOutboundAttachmentsCard } from "@/components/quotes/process-outbound-attachments-card"; import { ProcessItemAttachments } from "@/components/quotes/process-item-attachments"; +import { ProcessItemCard } from "@/components/quotes/process-item-card"; import type { Quote, QuoteItem, QuoteAttachment, Supplier, Customer } from "@/lib/types"; import { cn } from "@/lib/utils"; import type { LetterheadCode } from "@/lib/letterhead"; @@ -937,12 +938,18 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str {/* Items Table */} -
- + {/* Outer flex wraps to multiple lines on narrow viewports. + Title + total stay on row 1, progress badges wrap to row 2 + on mobile, action buttons drop to row 3. On `md+` everything + fits inline and `md:flex-nowrap` keeps it on one line. + Without the wrap, all 4 action buttons push off-screen on + a 428-px viewport (the original bug). */} +
+ Quote Items ({items.length}) -
+
{/* Per-state counts as a small inline progress strip. Lets the operator spot gaps at a glance without scanning each row. The numbers are click-free — @@ -950,7 +957,7 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str ("Hide completed", "Check pricing", "Rewrite with AI") are the actionable ones. */} {items.length > 0 && ( -
+
) : ( -
- + <> + {/* Desktop table — unchanged from the original + layout. `hidden md:block` keeps it out of the way on + small screens; the cards below take over. */} +
+
{/* Column widths here lock the layout so the @@ -1071,7 +1082,13 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str Qty Markup Attach - Actions + {/* Hidden on mobile (`md:table-cell` restores on + `md+`): the cramped 96-px icon column overflows + the table on a 428-px viewport. The 3 actions + move into the expanded row at the bottom of + each item on mobile, where they get full-width + buttons with proper 44-px tap targets. */} + Actions @@ -1168,7 +1185,7 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str type="button" variant="ghost" size="icon" - className="h-6 w-6 text-muted-foreground hover:text-foreground" + className="h-6 w-6 text-muted-foreground hover:text-foreground shrink-0" disabled={lookingUpId === item.id} onClick={() => void lookupItem(item.id, item.sku || "") @@ -1182,91 +1199,157 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str )} - {/* Inline state indicators — render ONLY - when something's wrong with the row. - Drags too much attention when every - item shows a chip; we want the - abnormal rows to pop. Red border is - reserved for the worst (no - description). Amber for missing - supplier / pricing. */} - {!item.description?.trim() && ( - - no description - - )} - {!item.supplier && ( - - ✕ supplier - - )} - {!(item.cost_price && item.selling_price) && ( - - ✕ priced - - )} - {item.pricing_source === "catalogue" && ( - - - {item.supplier - ? "matched" - : "matched (no supplier)"} - - )} - {item.pricing_source === "manual" && ( - - manual - - )} - {item.pricing_notes && + + {/* State indicators — own row so the + badges don't overflow into the + neighbouring Qty column on a narrow + Description cell. Same visual style as + before (red = worst, amber = missing + supplier / pricing, green = matched, + info = tagged) — just on their own line. */} + {(item.pricing_source === "catalogue" || + item.pricing_source === "manual" || + !item.description?.trim() || + !item.supplier || + !(item.cost_price && item.selling_price) || + (item.pricing_notes && item.pricing_source !== "catalogue" && - item.pricing_source !== "manual" && - (item.pricing_notes - .toLowerCase() - .includes("no match") ? ( + item.pricing_source !== "manual")) && ( +
+ {!item.description?.trim() && ( - - no match + no description + )} + {!item.supplier && ( + + ✕ supplier + + )} + {!(item.cost_price && item.selling_price) && ( + + ✕ priced + + )} + {item.pricing_source === "catalogue" && ( + + + {item.supplier + ? "matched" + : "matched (no supplier)"} + + )} + {item.pricing_source === "manual" && ( + + manual + + )} + {item.pricing_notes && + item.pricing_source !== "catalogue" && + item.pricing_source !== "manual" && + (item.pricing_notes + .toLowerCase() + .includes("no match") ? ( + + + no match + + ) : ( + + tagged + + ))} +
+ )} + {/* Mobile-only action row — always visible + on small screens, never gated behind + the RFQ-details expansion. On `md+` the + same 3 buttons live in the cramped icon + Actions column (`hidden md:table-cell` + — 96 px wide). On mobile this row gets + 44-px tall, full-width buttons so the + operator can save / price / delete any + row without scrolling horizontally or + expanding the RFQ details. */} +
+ + +
-
+ +
+ + {/* Mobile card-per-item layout — vertical cards, one per + item, with full-width fields and labelled action + buttons at 44 px tap targets. Hidden on `md+` so the + desktop table isn't duplicated. The card shares the + same callbacks as the desktop table (single source of + truth for state mutations). */} +
+ {allProcessed && !showCompleted ? ( +
+
+ +

+ All {items.length} items processed +

+ +
+
+ ) : ( +
+ {displayItems.map(({ item, originalIdx }) => { + const isExpanded = expandedRows.has(item.id); + return ( + + ); + })} +
+ )} +
+ )} diff --git a/src/components/quotes/process-item-card.test.tsx b/src/components/quotes/process-item-card.test.tsx new file mode 100644 index 0000000..56fd2de --- /dev/null +++ b/src/components/quotes/process-item-card.test.tsx @@ -0,0 +1,322 @@ +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { QuoteItem } from "@/lib/types"; + +/** + * Mock the attachments sub-component so the card tests don't need a + * live network — `ProcessItemAttachments` opens a popover that hits + * `/api/v1/quotes/items/[id]/attachments` on mount. + */ +vi.mock("@/components/quotes/process-item-attachments", () => ({ + ProcessItemAttachments: ({ itemId }: { itemId: string }) => ( +
+ 📎 0 +
+ ), +})); + +import { ProcessItemCard } from "./process-item-card"; + +const baseItem: QuoteItem = { + id: "qi-1", + description: "M20 x 30 set screw black", + sku: "", + quantity: 10, + cost_price: 8.16, + selling_price: 10.61, + markup: 30.02, + supplier: "sup-1", + pricing_source: "", + pricing_notes: "", + is_processed: false, +}; + +const noop = (): void => {}; +const asyncNoop = async (): Promise => {}; + +const handlers = () => ({ + onUpdateField: vi.fn(noop), + onToggleRow: vi.fn(noop), + onOpenPricing: vi.fn(noop), + onSave: vi.fn(asyncNoop), + onDelete: vi.fn(asyncNoop), + onLookupSku: vi.fn(asyncNoop), +}); + +const renderCard = (overrides: Partial[0]> = {}) => { + const merged = { + item: baseItem, + index: 1, + isExpanded: false, + savingId: null, + lookingUpId: null, + ...handlers(), + ...overrides, + }; + return render(); +}; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** + * React's controlled `` + `user.clear()` is + * fiddly in jsdom: clear fires input events but the controlled + * value is still the original number, so subsequent type calls + * append. We bypass userEvent for numeric inputs and drive + * fireEvent.change directly, using the React-aware value setter + * that testing-library ships. + */ +function setControlledValue( + input: HTMLInputElement | HTMLTextAreaElement, + value: string, +) { + const proto = + input instanceof HTMLTextAreaElement + ? window.HTMLTextAreaElement.prototype + : window.HTMLInputElement.prototype; + const setter = Object.getOwnPropertyDescriptor(proto, "value")?.set; + setter?.call(input, value); + input.dispatchEvent(new Event("input", { bubbles: true })); + input.dispatchEvent(new Event("change", { bubbles: true })); +} + +describe("ProcessItemCard", () => { + it("renders the item number and description as the card header", () => { + renderCard(); + const card = screen.getByTestId("process-item-card-qi-1"); + // The header h3 carries the title. + const header = card.querySelector("h3")!; + expect(header).toHaveTextContent("M20 x 30 set screw black"); + // The index renders as "1" inside its own span with aria-label. + expect(within(card).getByLabelText("Item 1")).toHaveTextContent("1"); + }); + + it("renders a check-circle when the item is already processed", () => { + renderCard({ item: { ...baseItem, is_processed: true } }); + expect(screen.getByLabelText("Processed")).toBeInTheDocument(); + }); + + it("shows the 'No description' italic placeholder in the header when description is empty", () => { + renderCard({ item: { ...baseItem, description: "" } }); + // The placeholder lives in the header h3 (capital N). + const header = screen.getByTestId("process-item-card-qi-1").querySelector("h3")!; + expect(within(header).getByText(/no description/i)).toBeInTheDocument(); + // The 'no description' BADGE also appears (lowercase n). + const badges = screen.getByTestId("card-row-badges-qi-1"); + expect(within(badges).getByText("no description")).toBeInTheDocument(); + }); + + it("shows the × supplier and × priced badges when supplier + pricing are missing", () => { + renderCard({ + item: { + ...baseItem, + supplier: "", + cost_price: 0, + selling_price: 0, + }, + }); + const badges = screen.getByTestId("card-row-badges-qi-1"); + expect(within(badges).getByText("✕ supplier")).toBeInTheDocument(); + expect(within(badges).getByText("✕ priced")).toBeInTheDocument(); + }); + + it("renders the green matched badge when pricing_source is catalogue", () => { + renderCard({ + item: { + ...baseItem, + pricing_source: "catalogue", + supplier: "sup-1", + }, + }); + expect(within(screen.getByTestId("card-row-badges-qi-1")).getByText("matched")).toBeInTheDocument(); + }); + + it("renders the amber 'no match' badge when pricing_notes contains the phrase", () => { + renderCard({ + item: { + ...baseItem, + pricing_source: "ai", + pricing_notes: "Catalogue lookup: no match — fell back", + }, + }); + expect(within(screen.getByTestId("card-row-badges-qi-1")).getByText("no match")).toBeInTheDocument(); + }); + + it("hides the badges container when no badge condition is met", () => { + // baseItem has: description, supplier, cost_price, selling_price all + // set, pricing_notes empty, pricing_source "" (not catalogue/manual). + // → none of the 6 badge conditions match → no container. + renderCard(); + expect(screen.queryByTestId("card-row-badges-qi-1")).not.toBeInTheDocument(); + }); + + it("calls onUpdateField with the new value when the description textarea changes", async () => { + const onUpdateField = vi.fn(noop); + renderCard({ onUpdateField }); + + const ta = screen.getByTestId("card-description-qi-1") as HTMLTextAreaElement; + setControlledValue(ta, "M24 hex bolt"); + + expect(onUpdateField).toHaveBeenCalledWith("qi-1", "description", "M24 hex bolt"); + }); + + it("calls onUpdateField with the new value when the SKU input changes", async () => { + const onUpdateField = vi.fn(noop); + renderCard({ onUpdateField }); + + const input = screen.getByTestId("card-sku-qi-1") as HTMLInputElement; + setControlledValue(input, "X"); + + expect(onUpdateField).toHaveBeenCalledWith("qi-1", "sku", "X"); + }); + + it("calls onLookupSku when the 🔍 button is clicked", async () => { + const user = userEvent.setup(); + const onLookupSku = vi.fn(asyncNoop); + renderCard({ onLookupSku }); + + await user.click(screen.getByTestId("card-sku-lookup-qi-1")); + + expect(onLookupSku).toHaveBeenCalledWith("qi-1", ""); + }); + + it("calls onLookupSku when Enter is pressed in the SKU input", async () => { + const user = userEvent.setup(); + const onLookupSku = vi.fn(asyncNoop); + renderCard({ + item: { ...baseItem, sku: "JEITD-06" }, + onLookupSku, + }); + + const input = screen.getByTestId("card-sku-qi-1") as HTMLInputElement; + input.focus(); + await user.keyboard("{Enter}"); + + expect(onLookupSku).toHaveBeenCalledWith("qi-1", "JEITD-06"); + }); + + it("disables the lookup button while this row is looking up", () => { + renderCard({ lookingUpId: "qi-1" }); + expect(screen.getByTestId("card-sku-lookup-qi-1")).toBeDisabled(); + }); + + it("calls onUpdateField('quantity', N) when the qty input changes", async () => { + const onUpdateField = vi.fn(noop); + renderCard({ onUpdateField }); + + const qty = screen.getByTestId("card-qty-qi-1") as HTMLInputElement; + setControlledValue(qty, "5"); + + expect(onUpdateField).toHaveBeenCalledWith("qi-1", "quantity", 5); + }); + + it("shows the markup percentage when markup > 0", () => { + renderCard({ item: { ...baseItem, markup: 25 } }); + expect(screen.getByTestId("card-markup-qi-1")).toHaveTextContent("25%"); + }); + + it("shows a dash for markup when markup is 0", () => { + renderCard({ item: { ...baseItem, markup: 0 } }); + expect(screen.getByTestId("card-markup-qi-1")).toHaveTextContent("—"); + }); + + it("renders the attachments stub from the mocked component", () => { + renderCard(); + expect(screen.getByTestId("attach-stub-qi-1")).toBeInTheDocument(); + }); + + it("hides the RFQ block by default and shows it when isExpanded", () => { + const { rerender } = renderCard({ isExpanded: false }); + expect(screen.queryByTestId("card-rfq-block-qi-1")).not.toBeInTheDocument(); + + rerender( + , + ); + expect(screen.getByTestId("card-rfq-block-qi-1")).toBeInTheDocument(); + expect(screen.getByTestId("card-rfq-desc-qi-1")).toBeInTheDocument(); + expect(screen.getByTestId("card-rfq-notes-qi-1")).toBeInTheDocument(); + }); + + it("calls onToggleRow when the RFQ details toggle is pressed", async () => { + const user = userEvent.setup(); + const onToggleRow = vi.fn(noop); + renderCard({ onToggleRow }); + + await user.click(screen.getByTestId("card-toggle-rfq-qi-1")); + + expect(onToggleRow).toHaveBeenCalledWith("qi-1"); + }); + + it("fires onOpenPricing when the Price button is clicked", async () => { + const user = userEvent.setup(); + const onOpenPricing = vi.fn(noop); + renderCard({ onOpenPricing }); + + await user.click(screen.getByTestId("card-price-qi-1")); + + expect(onOpenPricing).toHaveBeenCalledTimes(1); + expect(onOpenPricing.mock.calls[0][0]).toMatchObject({ id: "qi-1" }); + }); + + it("fires onSave when the Save button is clicked (and disables it while saving)", async () => { + const user = userEvent.setup(); + const onSave = vi.fn(asyncNoop); + const { rerender } = renderCard({ onSave }); + + const saveBtn = screen.getByTestId("card-save-qi-1"); + expect(saveBtn).not.toBeDisabled(); + + await user.click(saveBtn); + expect(onSave).toHaveBeenCalledWith("qi-1"); + + // While this row is saving, the button disables + rerender( + , + ); + expect(screen.getByTestId("card-save-qi-1")).toBeDisabled(); + }); + + it("shows an inline confirm block when Delete is pressed, and confirms/cancels", async () => { + const user = userEvent.setup(); + const onDelete = vi.fn(asyncNoop); + renderCard({ onDelete }); + + expect(screen.queryByTestId("card-delete-confirm-qi-1")).not.toBeInTheDocument(); + + // Click Delete — confirm block appears + await user.click(screen.getByTestId("card-delete-qi-1")); + const confirmBlock = screen.getByTestId("card-delete-confirm-qi-1"); + expect(confirmBlock).toBeInTheDocument(); + expect(within(confirmBlock).getByText("Confirm delete")).toBeInTheDocument(); + + // Click Cancel — confirm block disappears, action row returns + await user.click(within(confirmBlock).getByText("Cancel")); + expect(screen.queryByTestId("card-delete-confirm-qi-1")).not.toBeInTheDocument(); + expect(screen.getByTestId("card-row-actions-qi-1")).toBeInTheDocument(); + expect(onDelete).not.toHaveBeenCalled(); + + // Click Delete again, then Confirm delete — fires onDelete + await user.click(screen.getByTestId("card-delete-qi-1")); + await user.click(screen.getByTestId("card-delete-confirm-button-qi-1")); + expect(onDelete).toHaveBeenCalledWith("qi-1"); + }); +}); \ No newline at end of file diff --git a/src/components/quotes/process-item-card.tsx b/src/components/quotes/process-item-card.tsx new file mode 100644 index 0000000..2b6b967 --- /dev/null +++ b/src/components/quotes/process-item-card.tsx @@ -0,0 +1,482 @@ +"use client"; + +import { useState } from "react"; +import { Check, CheckCircle, ChevronDown, ChevronUp, DollarSign, FileText, RefreshCw, Save, Search, Trash2, XCircle } from "lucide-react"; + +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { Textarea } from "@/components/ui/textarea"; +import { ProcessItemAttachments } from "@/components/quotes/process-item-attachments"; +import type { QuoteItem } from "@/lib/types"; + +/** + * Card-per-item layout for the quote-process page on small screens. + * + * Why this exists + * ─────────────── + * The desktop layout is a 6-column table (``) + * — operators with a real keyboard scan rows side-by-side. On a phone, + * that table is ~434 px wide in a ~396 px viewport, so the operator has + * to horizontally scroll to reach the Save / Price / Delete buttons. + * Description text wraps mid-word in a 130 px column. Error badges + * overflow visually into the Qty column. None of it is usable. + * + * This component renders a vertical card per item that: + * + * 1. index + title ← one row, truncates cleanly + * 2. status badges ← own row, no overflow into other cells + * 3. description textarea ← full width + * 4. SKU + lookup button ← own row + * 5. qty / markup / attach ← 3-column grid with stacked labels + * 6. RFQ details toggle ← own row, full width + * 7. price / save / delete ← own row, 3 labelled buttons at 44 px + * + * The page branches on viewport: this card renders below the `md` + * breakpoint, the table renders at `md+`. Both render the same data + * from the same callbacks — single source of truth, two presentations. + * + * ## Props + * + * The component is intentionally callback-only. The parent owns the + * state (expandedRows, savingId, lookingUpId) and passes down + * `isExpanded` + the relevant pending-id. Keeps the card pure and + * easy to test in isolation. + * + * The callbacks mirror the page-level handlers exactly: + * + * - `onUpdateField` → `updateItemField` on the page + * - `onToggleRow` → `toggleRow` on the page + * - `onOpenPricing` → `openPricingModal` on the page + * - `onSave` → `saveItem` on the page + * - `onDelete` → `deleteItem` on the page + * - `onLookupSku` → `lookupItem` on the page + */ +export type ProcessItemCardProps = { + item: QuoteItem; + /** 1-based display index (not the database id). */ + index: number; + /** Whether this row's RFQ details row is expanded. */ + isExpanded: boolean; + /** PB id of the row currently being saved (drives Save button's spinner). */ + savingId: string | null; + /** PB id of the row currently doing a catalogue lookup. */ + lookingUpId: string | null; + onUpdateField: ( + itemId: string, + field: keyof QuoteItem, + value: string | number, + ) => void; + onToggleRow: (itemId: string) => void; + onOpenPricing: (item: QuoteItem) => void; + onSave: (itemId: string) => Promise | void; + onDelete: (itemId: string) => Promise | void; + onLookupSku: (itemId: string, query: string) => Promise | void; +}; + +const baseInputClass = "bg-input-highlight"; + +export function ProcessItemCard({ + item, + index, + isExpanded, + savingId, + lookingUpId, + onUpdateField, + onToggleRow, + onOpenPricing, + onSave, + onDelete, + onLookupSku, +}: ProcessItemCardProps) { + /** + * Local toggle for the "delete this row?" confirm step. We don't + * bounce a confirm modal for every delete on mobile — the row + * itself grows a tiny "Are you sure?" + cancel block so the + * operator can undo without breaking thumb flow. + */ + const [confirmingDelete, setConfirmingDelete] = useState(false); + + const hasBadges = + item.pricing_source === "catalogue" || + item.pricing_source === "manual" || + !item.description?.trim() || + !item.supplier || + !(item.cost_price && item.selling_price) || + !!(item.pricing_notes && + item.pricing_source !== "catalogue" && + item.pricing_source !== "manual"); + + const isSaving = savingId === item.id; + const isLookingUp = lookingUpId === item.id; + + return ( +
+ {/* Row 1: index + title */} +
+ + {index} + + {item.is_processed ? ( + + ) : null} +

+ {item.description || ( + No description + )} +

+
+ + {/* Row 2: status badges — own line so they never overflow into + neighbouring cells. Each badge is gated on its own condition; + the whole row only renders if at least one badge is relevant. */} + {hasBadges ? ( +
+ {!item.description?.trim() ? ( + + no description + + ) : null} + {!item.supplier ? ( + + ✕ supplier + + ) : null} + {!(item.cost_price && item.selling_price) ? ( + + ✕ priced + + ) : null} + {item.pricing_source === "catalogue" ? ( + + + {item.supplier ? "matched" : "matched (no supplier)"} + + ) : null} + {item.pricing_source === "manual" ? ( + + manual + + ) : null} + {item.pricing_notes && + item.pricing_source !== "catalogue" && + item.pricing_source !== "manual" ? ( + item.pricing_notes.toLowerCase().includes("no match") ? ( + + + no match + + ) : ( + + tagged + + ) + ) : null} +
+ ) : null} + + {/* Row 3: description textarea (full width) */} +
+ +