David/mega-plant
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Invoices: a machine chosen in the dropdown was never actually booked

Browse source 
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.
This commit is contained in:
David 2026-10-03 15:43:46 +02:00
parent 39633838e0
commit fd2afeba4e
3 changed files with 42 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
AGENTS.md
View file

@ -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

69
components/InvoiceTable.tsx
View file

@ -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 || ""}

13
components/MachineCombobox.tsx
View file

@ -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,