diff --git a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts index 96f5e3b7..82ae4333 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts @@ -93,6 +93,7 @@ export interface VendorRosterForm { roster: VendorCompanyRoster | undefined; companies: VendorFacetCompany[]; trades: string[]; + tradesLoading: boolean; selectedCompanyId: string | number | null; selectCompany: (company: VendorFacetCompany | null) => Promise; clearSelectedCompany: (nextName?: string) => void; @@ -110,7 +111,7 @@ export function useVendorRosterForm({ mode === "update" ? vendorId : undefined, mode === "update" ? companyId : undefined, ); - const { data: facets } = useVendorFacets(); + const { data: facets, isLoading: facetsLoading } = useVendorFacets(); const save = useSaveVendorCompanyRoster(); const [conflict, setConflict] = useState(null); @@ -223,6 +224,7 @@ export function useVendorRosterForm({ roster: routeRoster, companies, trades, + tradesLoading: facetsLoading, selectedCompanyId: selection.selectedCompanyId, selectCompany: selection.selectCompany, clearSelectedCompany: selection.clearSelectedCompany, diff --git a/src/app/(protected)/vendors/_components/vendor-create-modal.tsx b/src/app/(protected)/vendors/_components/vendor-create-modal.tsx index 31804a3c..002a81bf 100644 --- a/src/app/(protected)/vendors/_components/vendor-create-modal.tsx +++ b/src/app/(protected)/vendors/_components/vendor-create-modal.tsx @@ -68,6 +68,7 @@ export function VendorCreateModal({ open, onClose }: VendorCreateModalProps) { errors={form.errors} companies={form.companies} tradeOptions={form.trades} + tradeOptionsLoading={form.tradesLoading} selectedCompanyId={form.selectedCompanyId} onSelectCompany={form.selectCompany} onClearSelectedCompany={form.clearSelectedCompany} diff --git a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx index b1a24c3c..62d65f15 100644 --- a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx +++ b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx @@ -299,6 +299,7 @@ function DrawerEditor({ control={form.control} errors={form.errors} tradeOptions={form.trades} + tradeOptionsLoading={form.tradesLoading} showTechnicianStatus={false} /> {selectedIndex >= 0 && ( diff --git a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx index d6d5cbf7..93a10fd9 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx @@ -25,6 +25,7 @@ interface VendorRosterFormFieldsProps { errors: FieldErrors; companies?: VendorFacetCompany[]; tradeOptions?: string[]; + tradeOptionsLoading?: boolean; selectedCompanyId?: string | number | null; onSelectCompany?: (company: VendorFacetCompany | null) => Promise; onClearSelectedCompany?: (nextName?: string) => void; @@ -236,6 +237,7 @@ interface TechnicianRowProps { onRemove: () => void; canRemove: boolean; tradeOptions: string[]; + tradeOptionsLoading?: boolean; showStatus: boolean; } @@ -246,6 +248,7 @@ function TechnicianRow({ onRemove, canRemove, tradeOptions, + tradeOptionsLoading, showStatus, }: TechnicianRowProps) { return ( @@ -315,7 +318,12 @@ function TechnicianRow({ )} /> - + {showStatus && ( ; errors: FieldErrors; tradeOptions: string[]; + tradeOptionsLoading?: boolean; showStatus: boolean; }) { const { fields, append, remove } = useFieldArray({ control, name: "technicians" }); @@ -389,6 +399,7 @@ function TechniciansFieldArray({ onRemove={() => remove(index)} canRemove tradeOptions={tradeOptions} + tradeOptionsLoading={tradeOptionsLoading} showStatus={showStatus} /> )) @@ -399,7 +410,13 @@ function TechniciansFieldArray({ } export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) { - const { control, errors, tradeOptions = [], showTechnicianStatus = true } = props; + const { + control, + errors, + tradeOptions = [], + tradeOptionsLoading = false, + showTechnicianStatus = true, + } = props; return ( @@ -408,6 +425,7 @@ export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) { control={control} errors={errors} tradeOptions={tradeOptions} + tradeOptionsLoading={tradeOptionsLoading} showStatus={showTechnicianStatus} /> diff --git a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx index f0316957..45f1bb19 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx @@ -124,6 +124,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa control={form.control} errors={form.errors} tradeOptions={form.trades} + tradeOptionsLoading={form.tradesLoading} {...companySelectionProps} /> diff --git a/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx b/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx index ff63ef4c..11a7ea4b 100644 --- a/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx +++ b/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useRef, useState } from "react"; import { Controller, type Control } from "react-hook-form"; import AddIcon from "@mui/icons-material/Add"; import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; @@ -14,8 +14,43 @@ import { TextField, Typography, } from "@mui/material"; +import { Text } from "@/components/ui/text"; import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema"; +/** + * `Text variant="feedback"` stays mounted when silent so the aria-live region can announce later. + * An empty paragraph still claims a line box, which would add a permanent gap under the trades + * input and shift the approved drawer layout. Collapse the box while keeping the node in the DOM + * (never `display: none`, which would stop announcements). + */ +function collapsedWhenSilent(visible: boolean) { + return visible ? undefined : { height: 0, margin: 0, overflow: "hidden" as const }; +} + +function TradeFeedbackRegions({ + listUnavailable, + message, +}: { + listUnavailable: boolean; + message: string | null; +}) { + return ( + <> + + Trade list is unavailable right now — you can still type a trade manually. + + + {message} + + + ); +} + function splitTrades(value: string | undefined): string[] { return (value ?? "") .split(",") @@ -23,18 +58,59 @@ function splitTrades(value: string | undefined): string[] { .filter(Boolean); } +function dedupeTrades(options: string[], current: string[]): string[] { + const seen = new Set(); + const merged: string[] = []; + for (const trade of [...options, ...current]) { + const key = trade.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + merged.push(trade); + } + return merged; +} + +function matchTradeOption(input: string, options: string[]): string | null { + const exact = options.find((option) => option === input); + if (exact) return exact; + const normalized = input.toLowerCase(); + return options.find((option) => option.toLowerCase() === normalized) ?? null; +} + interface VendorTradeSpecialtiesFieldProps { control: Control; index: number; tradeOptions: string[]; + tradeOptionsLoading?: boolean; +} + +/** + * SH-249: an empty tradeOptions array means two different things. While the facets + * query is in flight it means "not loaded yet"; only once it settles does it mean + * "vocabulary genuinely unavailable". Failing open during the cold-fetch window let + * free text through the canonical gate permanently, because anything committed there + * is added to knownTradesRef and stays selectable after the real list arrives. + * + * SH-249: why a typed trade was refused. Extracted to module scope so the field + * component stays inside the maintainability gate's function-length cap. + */ +function resolveRejectionMessage(value: string, loading: boolean): string { + return loading + ? "Trades are still loading. Wait for the list, then pick a trade." + : `"${value}" is not in the trades list. Pick a trade from the list.`; } export function VendorTradeSpecialtiesField({ control, index, tradeOptions, + tradeOptionsLoading = false, }: VendorTradeSpecialtiesFieldProps) { const [tradeInput, setTradeInput] = useState(""); + const [feedbackMessage, setFeedbackMessage] = useState(null); + const knownTradesRef = useRef>(new Set()); + const tradeListUnavailable = !tradeOptionsLoading && tradeOptions.length === 0; + const gateActive = tradeOptions.length > 0 || tradeOptionsLoading; return ( @@ -47,10 +123,28 @@ export function VendorTradeSpecialtiesField({ name={`technicians.${index}.tradeSpecialties`} render={({ field }) => { const trades = splitTrades(field.value); + for (const trade of trades) knownTradesRef.current.add(trade); + const selectableTrades = dedupeTrades(tradeOptions, [...knownTradesRef.current]); const commit = (next: string[]) => field.onChange(next.join(", ")); const add = (value: string) => { const normalized = value.trim(); - if (normalized && !trades.includes(normalized)) commit([...trades, normalized]); + setFeedbackMessage(null); + if (!normalized) { + setTradeInput(""); + return; + } + const canonical = matchTradeOption(normalized, selectableTrades); + if (gateActive && !canonical) { + setFeedbackMessage(resolveRejectionMessage(normalized, tradeOptionsLoading)); + return; + } + const trade = canonical ?? normalized; + if (trades.some((existing) => existing.toLowerCase() === trade.toLowerCase())) { + setFeedbackMessage(`${trade} is already on this technician.`); + setTradeInput(""); + return; + } + commit([...trades, trade]); setTradeInput(""); }; const move = (tradeIndex: number, direction: -1 | 1) => { @@ -64,31 +158,31 @@ export function VendorTradeSpecialtiesField({ return ( - {trades.length === 0 ? ( - - No trades selected. - - ) : ( - trades.map((trade, tradeIndex) => ( - - commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex)) - } - deleteIcon={} - /> - )) - )} + + No trades selected. + + {trades.map((trade, tradeIndex) => ( + + commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex)) + } + deleteIcon={} + /> + ))} { - if (reason === "input") setTradeInput(value); + if (reason === "input") { + setTradeInput(value); + setFeedbackMessage(null); + } }} onChange={(_event, value) => { if (typeof value === "string") add(value); @@ -147,6 +241,9 @@ export function VendorTradeSpecialtiesField({ ); }} /> + {/* Outside the spaced Stack: an always-mounted live region would otherwise inherit Stack + spacing and add a permanent gap under the trades input, shifting the approved layout. */} + ); } diff --git a/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx b/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx new file mode 100644 index 00000000..03990707 --- /dev/null +++ b/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx @@ -0,0 +1,192 @@ +import { fireEvent, screen } from "@testing-library/react"; +import { useEffect } from "react"; +import { useForm, useWatch, type Control } from "react-hook-form"; +import { describe, expect, it } from "vitest"; +import { VendorTradeSpecialtiesField } from "@/app/(protected)/vendors/_components/vendor-trade-specialties-field"; +import { + emptyRosterTechnician, + emptyVendorCompanyRosterForm, + type VendorCompanyRosterFormValues, +} from "@/domain/vendors/schemas/vendor-roster-schema"; +import { renderWithProviders } from "@/test/test-utils"; + +function TradeSpecialtiesSpy({ + control, + valuesRef, +}: { + control: Control; + valuesRef: { current: string }; +}) { + const value = useWatch({ control, name: "technicians.0.tradeSpecialties" }) ?? ""; + useEffect(() => { + valuesRef.current = value; + }, [value, valuesRef]); + return null; +} + +function TradeSpecialtiesHarness({ + tradeOptions, + tradeOptionsLoading = false, + initialTradeSpecialties = "", + valuesRef, +}: { + tradeOptions: string[]; + tradeOptionsLoading?: boolean; + initialTradeSpecialties?: string; + valuesRef: { current: string }; +}) { + const { control } = useForm({ + defaultValues: { + ...emptyVendorCompanyRosterForm, + technicians: [{ ...emptyRosterTechnician, tradeSpecialties: initialTradeSpecialties }], + }, + }); + return ( + <> + + + + ); +} + +function getAddTradeInput(): HTMLElement { + return screen.getByRole("combobox", { name: "Add Trade" }); +} + +function typeAndCommitTrade(input: HTMLElement, value: string, commit: "enter" | "button") { + fireEvent.change(input, { target: { value } }); + if (commit === "enter") { + fireEvent.keyDown(input, { key: "Enter" }); + } else { + fireEvent.click(screen.getByRole("button", { name: "Add trade" })); + } +} + +describe("VendorTradeSpecialtiesField", () => { + it("offers facet trades as selectable options when the facets query is populated", async () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + fireEvent.change(input, { target: { value: "H" } }); + + fireEvent.click(await screen.findByRole("option", { name: "HVAC" })); + + expect(await screen.findByText("HVAC (primary)")).toBeInTheDocument(); + expect(valuesRef.current).toBe("HVAC"); + }); + + it("does not fail open while the trades facet is still loading", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + // A cold fetch must not look like an outage. + expect(screen.queryByText(/trade list is unavailable right now/i)).toBeNull(); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "Plumbing", "enter"); + + expect(screen.getByText(/trades are still loading/i)).toBeInTheDocument(); + expect(screen.queryByText("Plumbing (primary)")).toBeNull(); + expect(valuesRef.current).toBe(""); + }); + + it("has no selectable options when the trades facet is empty, warns, and still accepts typed trades", () => { + const valuesRef = { current: "" }; + renderWithProviders(); + + expect(screen.getByText(/trade list is unavailable right now/i)).toBeInTheDocument(); + + const input = getAddTradeInput(); + fireEvent.change(input, { target: { value: "P" } }); + expect(screen.queryByRole("listbox")).toBeNull(); + expect(screen.queryAllByRole("option")).toHaveLength(0); + + typeAndCommitTrade(input, "Plumbing", "enter"); + + expect(screen.getByText("Plumbing (primary)")).toBeInTheDocument(); + expect(valuesRef.current).toBe("Plumbing"); + }); + + it("rejects trades outside the canonical list with visible feedback and keeps the draft", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "Sprinkler Fitting", "button"); + + expect(screen.getByText(/is not in the trades list/i)).toBeInTheDocument(); + expect(screen.queryByText("Sprinkler Fitting (primary)")).toBeNull(); + expect(valuesRef.current).toBe(""); + expect(input).toHaveValue("Sprinkler Fitting"); + }); + + it("commits the canonical casing when a typed trade matches case-insensitively", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "hvac", "enter"); + + expect(screen.getByText("HVAC (primary)")).toBeInTheDocument(); + expect(valuesRef.current).toBe("HVAC"); + }); + + it("keeps legacy free-text trades visible, removable, and re-selectable", async () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + expect(screen.getByText("Backflow Testing (primary)")).toBeInTheDocument(); + expect(screen.getByText("HVAC")).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("remove-trade-Backflow Testing")); + expect(screen.queryByText(/Backflow Testing/)).toBeNull(); + expect(valuesRef.current).toBe("HVAC"); + + const input = getAddTradeInput(); + fireEvent.change(input, { target: { value: "Back" } }); + fireEvent.click(await screen.findByRole("option", { name: "Backflow Testing" })); + + expect(await screen.findByText("Backflow Testing")).toBeInTheDocument(); + expect(valuesRef.current).toBe("HVAC, Backflow Testing"); + }); + + it("flags duplicate trades instead of silently doing nothing", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "plumbing", "enter"); + + expect(screen.getByText(/is already on this technician/i)).toBeInTheDocument(); + expect(valuesRef.current).toBe("Plumbing"); + }); +});