From 038ce263da3ec14b0607ae0a3ae40963e427e9ed Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Wed, 19 Aug 2026 13:22:11 -0300 Subject: [PATCH] fix(vendors): vendor detail parity with the design prototype (SH-253) Three divergences in the vendor detail drawer: - The header rendered the company as a subtitle under a title that had already fallen back to the company, printing the same name twice when the row carried no technician. The subtitle is now dropped when it would repeat the title. - The Google Maps link appeared only when a URL was stored, unlabelled, so an empty value left no trace of the field. It is now a labelled field below Address that reads em-dash when unset, as every other field in the panel does. - The status sat immediately beside Total Jobs. It now sits opposite it at the right edge, and reuses the vendors table's dot-plus-text badge so the state never reads by colour alone. --- .../_components/vendor-detail-drawer.tsx | 60 ++++++++------ .../_components/vendor-status-badge.tsx | 20 +++++ .../vendors/_components/vendors-table.tsx | 22 +----- .../vendors/vendor-detail-drawer.test.tsx | 79 +++++++++++++++++++ 4 files changed, 137 insertions(+), 44 deletions(-) create mode 100644 src/app/(protected)/vendors/_components/vendor-status-badge.tsx diff --git a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx index 74d6ef02..0c490350 100644 --- a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx +++ b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx @@ -17,6 +17,7 @@ import { Switch, } from "@mui/material"; import { VendorRosterConflictAlert } from "./vendor-roster-conflict-alert"; +import { VendorStatusBadge } from "./vendor-status-badge"; import { VendorRosterFormFields } from "./vendor-roster-form-fields"; import { VendorRosterLoadErrorAlert } from "./vendor-roster-load-error"; import { useVendorRosterForm } from "./use-vendor-roster-form"; @@ -81,6 +82,13 @@ function DrawerHeader({ technician?: VendorRosterTechnician; onClose: () => void; }) { + const title = technician?.contactName || vendor?.contactName || roster?.name || "Vendor company"; + const company = roster?.name || vendor?.companyName || "Vendor details"; + // The company is a subtitle under the technician's name. When there is no + // technician the title already falls back to the company, and repeating it + // would render the same line twice. + const subtitle = company === title ? null : company; + return ( - {technician?.contactName || vendor?.contactName || roster?.name || "Vendor company"} - - - {roster?.name || vendor?.companyName || "Vendor details"} + {title} + {subtitle !== null && ( + + {subtitle} + + )} @@ -118,17 +128,24 @@ function CompanySection({ roster }: { roster: VendorCompanyRoster }) { - {Boolean(roster.googleMapsUrl) && ( - - - Open in Google Maps - - )} + + + Google Maps + + {roster.googleMapsUrl ? ( + + + Open in Google Maps + + ) : ( + — + )} + ); } @@ -154,23 +171,14 @@ function TechnicianSection({ technician }: { technician: VendorRosterTechnician )} - + {technician.totalJobs ?? 0} Total Jobs - - - Status - - - + diff --git a/src/app/(protected)/vendors/_components/vendor-status-badge.tsx b/src/app/(protected)/vendors/_components/vendor-status-badge.tsx new file mode 100644 index 00000000..7fee8970 --- /dev/null +++ b/src/app/(protected)/vendors/_components/vendor-status-badge.tsx @@ -0,0 +1,20 @@ +import { Box, Stack } from "@mui/material"; +import { Text } from "@/components/ui/text"; + +/** Dot plus text, so the status never reads by colour alone. */ +export function VendorStatusBadge({ isActive }: { isActive: boolean }) { + return ( + + + {isActive ? "Active" : "Inactive"} + + ); +} diff --git a/src/app/(protected)/vendors/_components/vendors-table.tsx b/src/app/(protected)/vendors/_components/vendors-table.tsx index db588cff..e524630c 100644 --- a/src/app/(protected)/vendors/_components/vendors-table.tsx +++ b/src/app/(protected)/vendors/_components/vendors-table.tsx @@ -18,6 +18,7 @@ import { TableRow, Tooltip, } from "@mui/material"; +import { VendorStatusBadge } from "./vendor-status-badge"; import { Text } from "@/components/ui/text"; import type { VendorListItem } from "@/domain/vendors/types/vendor"; @@ -50,23 +51,6 @@ function stopPropagation(event: MouseEvent): void { event.stopPropagation(); } -function VendorStatus({ isActive }: { isActive: boolean }) { - return ( - - - {isActive ? "Active" : "Inactive"} - - ); -} - interface VendorTableRowProps { row: VendorListItem; onOpenDetail: (row: VendorListItem) => void; @@ -177,7 +161,9 @@ function VendorTableRow({ row, onOpenDetail, onOpenEdit }: VendorTableRowProps) {row.totalJobs ?? 0} - + + + { expect(screen.queryByRole("switch", { name: "Active status" })).not.toBeInTheDocument(); }); }); + +describe("VendorDetailDrawer design parity", () => { + it("renders the company once when there is no technician name to head the panel", () => { + useVendorCompanyRoster.mockReturnValue(rosterWith([])); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + const heading = screen.getByRole("heading", { level: 2, name: "Gateway Plumbing" }); + expect(heading).toBeInTheDocument(); + // The header block holds the title alone — no subtitle repeating the company. + expect(heading.parentElement?.children).toHaveLength(1); + }); + + it("keeps the company subtitle under a technician name", () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([{ id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }]), + ); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + expect(screen.getByRole("heading", { level: 2, name: "Adam" })).toBeInTheDocument(); + expect(screen.getAllByText("Gateway Plumbing").length).toBeGreaterThan(0); + }); + + it("labels the Google Maps field below Address even when no URL is stored", () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([{ id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }]), + ); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + expect(screen.getByText("Google Maps")).toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /Open in Google Maps/ })).not.toBeInTheDocument(); + }); + + it("links out to Google Maps when a URL is stored", () => { + const roster = rosterWith([ + { id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }, + ]); + roster.data.googleMapsUrl = "https://maps.google.com/?q=1+Industrial+Pkwy"; + useVendorCompanyRoster.mockReturnValue(roster); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + expect(screen.getByRole("link", { name: /Open in Google Maps/ })).toHaveAttribute( + "href", + "https://maps.google.com/?q=1+Industrial+Pkwy", + ); + }); + + it("puts the status opposite Total Jobs and drops the redundant Status label", () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([{ id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }]), + ); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + const row = screen.getByText("Total Jobs").closest("div")?.parentElement; + expect(row).not.toBeNull(); + expect(getComputedStyle(row as Element).justifyContent).toBe("space-between"); + expect(row).toHaveTextContent("Active"); + expect(screen.queryByText("Status")).not.toBeInTheDocument(); + }); +});