From fd2afeba4e6902184b4554c60eee95c6864519d8 Mon Sep 17 00:00:00 2001
From: David <david@DavidsMcBookNeo.home>
Date: Sat, 3 Oct 2026 15:43:46 +0200
Subject: [PATCH] Invoices: a machine chosen in the dropdown was never actually
 booked
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Selecting a plant item in the Booked to dropdown silently did nothing. The menu
opened, the option highlighted, and clicking it produced no request at all — the
menu just closed. Reproduced with a real browser click: no POST to /book.

The menu is rendered with createPortal(..., document.body) because the line table
lives in an overflow-x-auto container that would clip an in-place dropdown. A
portal is not a DOM descendant of the component that made it, so the click-away
handler's `boxRef.contains(target)` reported every option as an outside click:
mousedown closed the menu, React unmounted the button, and the click that would
have booked it never landed. The menu now has its own ref and is treated as part
of the control. Verified with a real click — the booking posts, the site clears,
and the unbooked badge goes.

The site dropdown was never affected; it is a native <select>, outside the portal.

Also on this screen:

- The Booked to column needs min-w-[240px] with the two pickers stacked inside
  it. At 768px the column is only ~166px, so two 200px controls could not sit
  side by side; as flex children they compressed into each other by ~12px, and
  once they were not allowed to shrink they spilled past the edge of the table.
- Dropped the "Booked to income / VAT to SARS / Billed to customer / Still
  unbooked" money block, its component and the variables feeding it. The status
  line stays: "1 line not yet booked to a machine".

Typecheck and build green. Checked at 768x1024 and 375x812: no page overflow, no
control past the table edge, no tap target under 44px.
---
 AGENTS.md                      | 20 +++++++++-
 components/InvoiceTable.tsx    | 69 ++++++----------------------------
 components/MachineCombobox.tsx | 13 ++++++-
 3 files changed, 42 insertions(+), 60 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index bb9f2fb..95c4c9b 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -124,8 +124,24 @@ cells repeat it exactly.
 
 Two more traps on this screen: a `flex-wrap` KPI row sizes tiles by CONTENT, so the wide
 "booked to income" tile pushed the third one onto a second row below ~644px — it is a
-`grid-cols-3` now. And the two pickers in the **Booked to** cell are `shrink-0`; as flex
-children they compressed into each other by ~12px instead of sitting side by side.
+`grid-cols-3` now. And the **Booked to** column needs `min-w-[240px]` with the two
+pickers stacked inside it: at 768px the column is only ~166px, so two 200px controls
+could not sit side by side, and when they were not allowed to shrink they spilled past
+the edge of the table.
+
+## The machine dropdown — a portal is NOT inside its trigger
+
+`MachineCombobox` renders its menu with `createPortal(..., document.body)`, because the
+line table lives in an `overflow-x-auto` container that would clip an in-place dropdown.
+
+**A portal is not a DOM descendant of the component that made it.** The click-away
+handler tested `boxRef.contains(target)`, so every option in the menu read as an outside
+click: `mousedown` closed the menu, React unmounted the button, and the `click` that
+would have booked it never fired. Selecting a machine silently did nothing — the menu
+just closed. The menu now has its own ref and is treated as part of the control.
+
+Symptom to recognise: a dropdown opens, options highlight, and choosing one does nothing
+at all — no request, no error, no change. Suspect the outside-click test before the API.
 
 ## Money on invoices — VAT is NOT revenue
 
diff --git a/components/InvoiceTable.tsx b/components/InvoiceTable.tsx
index 5c2a573..09e53f5 100644
--- a/components/InvoiceTable.tsx
+++ b/components/InvoiceTable.tsx
@@ -24,11 +24,6 @@ export function InvoiceSummary({
   //   ex-VAT  = what actually lands in the P&L as income for the machine.
   //   incl-VAT= what the debtor actually pays (ex-VAT + SARS VAT).
   // Booking is done ex-VAT: VAT is collected for SARS, never income.
-  const sum = (ls: BookedLine[], k: "amount_excl" | "vat") => ls.reduce((s, l) => s + (l[k] || 0), 0);
-  const bookedExVat = sum(lines.filter((l) => l.isBooked), "amount_excl");
-  const bookedVat = sum(lines.filter((l) => l.isBooked), "vat");
-  const unbookedExVat = sum(unbooked, "amount_excl");
-  const unbookedVat = sum(unbooked, "vat");
   const period = `${invoice.period_from.slice(0, 10)} → ${invoice.period_to.slice(0, 10)}`;
   const periodMonth = new Date(invoice.period_to).toLocaleDateString("en-ZA", {
     month: "long",
@@ -115,7 +110,6 @@ export function InvoiceSummary({
               {unbooked.length} line{unbooked.length > 1 ? "s" : ""} not yet booked to a machine — choose a
               plant item below.
             </p>
-            <BookingMoney bookedExVat={bookedExVat} bookedVat={bookedVat} unbookedExVat={unbookedExVat} unbookedVat={unbookedVat} />
           </div>
         </div>
       ) : (
@@ -123,7 +117,6 @@ export function InvoiceSummary({
           <CheckCircle2 className="w-4 h-4 text-[#00798C] shrink-0 mt-0.5" />
           <div className="text-sm text-[#00798C]">
             <p>All {lines.length} lines booked to {periodMonth} income.</p>
-            <BookingMoney bookedExVat={bookedExVat} bookedVat={bookedVat} />
           </div>
         </div>
       )}
@@ -164,48 +157,6 @@ export function InvoiceSummary({
   );
 }
 
-// Shows the booked money three ways at once, so nobody has to guess which figure the
-// dashboard is using:
-//
-//   booked to income (ex VAT) → what lands in the P&L for the machine
-//   VAT                       → collected for SARS, NOT income
-//   billed to customer        → ex-VAT + VAT, what the debtor actually pays
-function BookingMoney({
-  bookedExVat,
-  bookedVat,
-  unbookedExVat = 0,
-  unbookedVat = 0,
-}: {
-  bookedExVat: number;
-  bookedVat: number;
-  unbookedExVat?: number;
-  unbookedVat?: number;
-}) {
-  return (
-    <div className="mt-1.5 flex flex-wrap gap-x-5 gap-y-1">
-      <Item label="Booked to income (ex VAT)" value={bookedExVat} strong />
-      <Item label="VAT (to SARS, not income)" value={bookedVat} />
-      <Item label="Billed to customer" value={bookedExVat + bookedVat} strong />
-      {unbookedExVat > 0 && (
-        <>
-          <Item label="Still unbooked (ex VAT)" value={unbookedExVat} strong />
-          <Item label="Still unbooked (billed)" value={unbookedExVat + unbookedVat} />
-        </>
-      )}
-    </div>
-  );
-}
-
-function Item({ label, value, strong }: { label: string; value: number; strong?: boolean }) {
-  return (
-    <span className="inline-flex items-baseline gap-1.5">
-      <span className="opacity-80">{label}</span>
-      <span className={`font-semibold tabular-nums ${strong ? "underline decoration-dotted underline-offset-2" : ""}`}>
-        {ZAR2(value)}
-      </span>
-    </span>
-  );
-}
 
 function TotalRow({ label, value, strong }: { label: string; value: number; strong?: boolean }) {
   return (
@@ -246,7 +197,9 @@ export function InvoiceTable({ lines, assets }: { lines: BookedLine[]; assets: A
               <th className="text-right py-3 px-3 font-medium whitespace-nowrap">Hire ex VAT</th>
               <th className="text-right py-3 px-3 font-medium whitespace-nowrap">VAT</th>
               <th className="text-right py-3 px-3 font-medium whitespace-nowrap">Total incl</th>
-              <th className="text-left py-3 px-3 font-medium whitespace-nowrap">Booked to</th>
+              <th className="text-left py-3 px-3 font-medium whitespace-nowrap min-w-[240px]">
+                Booked to
+              </th>
             </tr>
           </thead>
           <tbody>
@@ -281,13 +234,15 @@ export function InvoiceTable({ lines, assets }: { lines: BookedLine[]; assets: A
                 <td className="py-3 px-3 text-right tabular-nums font-semibold text-[#003D5B] dark:text-white whitespace-nowrap">
                   {ZAR2(l.total_incl)}
                 </td>
-                <td className="py-3 px-3">
-                  {/* shrink-0 on both: as flex children they were free to compress, and
-                      the site select ended up overlapping the machine picker by ~12px
-                      instead of sitting beside it. They may not overlap, so neither may
-                      shrink — the cell scrolls with the rest of the table if it must. */}
-                  <div className="flex flex-col sm:flex-row gap-2 sm:items-start">
-                    <div className="min-w-[200px] shrink-0">
+                <td className="py-3 px-3 min-w-[240px]">
+                  {/* Stacked, not side by side. At 768px the Booked to column is only
+                      ~166px, so two 200px pickers could never fit across — as flex
+                      children they were compressed into each other by ~12px, and once
+                      forced not to shrink they spilled PAST the edge of the table. The
+                      column now declares a min-width, the controls stack inside it, and
+                      the table scrolls inside its own box instead of overflowing. */}
+                  <div className="flex flex-col gap-2">
+                    <div className="min-w-[220px]">
                       <MachineCombobox
                         lineId={l.id}
                         value={l.booked_asset || ""}
diff --git a/components/MachineCombobox.tsx b/components/MachineCombobox.tsx
index a1fa2b5..eeb3ccf 100644
--- a/components/MachineCombobox.tsx
+++ b/components/MachineCombobox.tsx
@@ -39,6 +39,7 @@ export function MachineCombobox({
   const [error, setError] = useState<string | null>(null);
   const [highlight, setHighlight] = useState(0);
   const boxRef = useRef<HTMLDivElement>(null);
+  const menuRef = useRef<HTMLDivElement>(null);
   const inputRef = useRef<HTMLInputElement>(null);
   const listRef = useRef<HTMLUListElement>(null);
   // The line table lives inside an overflow-x-auto scroll container, which CLIPS any
@@ -112,10 +113,19 @@ export function MachineCombobox({
 
 
   // Click away closes.
+  //
+  // The menu is PORTALED into <body>, so it is not a DOM descendant of boxRef. Testing
+  // `boxRef.contains(target)` alone therefore reported every option as an outside
+  // click: mousedown closed the menu, React unmounted the button, and the click that
+  // would have booked it never landed. Selecting a machine silently did nothing. The
+  // menu has to be treated as part of the control.
   useEffect(() => {
     if (!open) return;
     const onDown = (e: MouseEvent) => {
-      if (boxRef.current && !boxRef.current.contains(e.target as Node)) setOpen(false);
+      const t = e.target as Node;
+      if (boxRef.current?.contains(t)) return;
+      if (menuRef.current?.contains(t)) return;
+      setOpen(false);
     };
     document.addEventListener("mousedown", onDown);
     return () => document.removeEventListener("mousedown", onDown);
@@ -231,6 +241,7 @@ export function MachineCombobox({
 
       {open && anchor && createPortal(
         <div
+          ref={menuRef}
           style={{
             position: "fixed",
             top: flipUp ? Math.max(8, anchor.top - menuHeight - 6) : anchor.top + 4,