refactor(collection-confirm): make PO cards start collapsed by default

Remove the initial open state for the first PO card, and update all tests to expect the default collapsed state. Users can expand cards manually to view contents, improving scannability for long PO queues.
This commit is contained in:
SJCoetz 2026-08-05 23:08:44 +02:00
parent 12fa65a481
commit 9f601002af
2 changed files with 96 additions and 28 deletions

View file

@ -121,6 +121,7 @@ describe("CollectionConfirmPage", () => {
it("loads rows via /api/v1/collection-confirm and renders one PoCard per PO", async () => {
const fetchSpy = vi.spyOn(window, "fetch");
const user = userEvent.setup();
render(<Page />);
// PoCard renders after the fetch resolves.
@ -136,7 +137,8 @@ describe("CollectionConfirmPage", () => {
}),
);
// Both rows are rendered inside the PoCard.
// Both rows are rendered inside the PoCard (body opened).
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
expect(screen.getByText("Wheel hub kit")).toBeInTheDocument();
expect(screen.getByText("20mm cable ties")).toBeInTheDocument();
@ -168,6 +170,10 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// All cards start collapsed. Open PO-1's body so the chip
// strip and preview are mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// Two chips, one per filename. The preview block is mounted
// as soon as a card has attachments — the first attachment is
// shown by default.
@ -216,12 +222,17 @@ describe("CollectionConfirmPage", () => {
];
buildFetchSpy(photoRows);
const user = userEvent.setup();
render(<Page />);
await waitFor(() =>
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// All cards start collapsed. Open PO-1's body so the
// preview + scroll container are mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// No modal — the preview lives inside the card so the
// operator can scroll the items table above and keep the
// document in view.
@ -259,6 +270,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the preview is mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// Preview mounted by default with the first chip active.
expect(screen.getByTestId("po-card-preview-PO-1")).toBeInTheDocument();
const chip = screen.getByRole("button", { name: /invoice-1\.pdf 1 of 1/i });
@ -296,6 +310,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the chips and preview are mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// First chip is active by default; preview shows invoice-1.
const preview = screen.getByTestId("po-card-preview-PO-1");
expect(
@ -335,6 +352,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body first.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
const chip = screen.getByRole("button", { name: /invoice-1\.pdf 1 of 1/i });
const preview = screen.getByTestId("po-card-preview-PO-1");
const viewer = within(preview).getByTestId("invoice-viewer");
@ -367,6 +387,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the preview is mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// Preview is mounted by default — the open-new-tab button is
// already on screen, no chip click needed.
const viewer = within(screen.getByTestId("po-card-preview-PO-1")).getByTestId("invoice-viewer");
@ -389,12 +412,16 @@ describe("CollectionConfirmPage", () => {
];
buildFetchSpy(noPhotoRows);
const user = userEvent.setup();
render(<Page />);
await waitFor(() =>
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the upload prompt is visible.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// The PoCard shows an "Upload invoice" prompt in place of the
// photo strip — the operator can upload a counter-collection
// or late email invoice from right inside the card.
@ -433,11 +460,11 @@ describe("CollectionConfirmPage", () => {
expect(screen.queryByTestId("collection-confirm-queue-PO-1")).not.toBeInTheDocument();
expect(screen.queryByTestId("po-detail-modal")).not.toBeInTheDocument();
// The first card's body is open by default so the operator
// lands on a ready workspace: table, chip strip, and preview
// are all mounted without an extra click.
expect(card.getAttribute("data-open")).toBe("true");
expect(within(card).getByText("Wheel hub kit")).toBeInTheDocument();
// All cards are collapsed by default — only the header is
// visible. The operator opens the card they want to work on
// so a long queue is scannable.
expect(card.getAttribute("data-open")).toBe("false");
expect(within(card).queryByText("Wheel hub kit")).not.toBeInTheDocument();
// Total = qty_collected (10) × cost_price (45) = 450.00.
// ZAR currency uses "R 450,00" via en-ZA locale; the comma +
// space is the localised thousands/decimal separator.
@ -450,8 +477,15 @@ describe("CollectionConfirmPage", () => {
within(card).getByTestId("po-card-orders-PO-1"),
).toHaveTextContent("SO-row-photo-1");
// Clicking the header collapses the body so a long queue
// stays scannable.
// Clicking the header expands the body so the operator can
// work on this PO. Toggle again to collapse.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
expect(card.getAttribute("data-open")).toBe("true");
expect(within(card).getByText("Wheel hub kit")).toBeInTheDocument();
expect(
within(card).getByTestId("po-card-preview-PO-1"),
).toBeInTheDocument();
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
expect(card.getAttribute("data-open")).toBe("false");
expect(within(card).queryByText("Wheel hub kit")).not.toBeInTheDocument();
@ -504,8 +538,8 @@ describe("CollectionConfirmPage", () => {
// Both POs render as their own cards — no selection state, no
// queue sidebar, no swapping workspace. Each card is the
// whole workspace for its PO, but only the first one starts
// expanded so a long queue is scannable.
// whole workspace for its PO, but every card starts collapsed
// so a long queue is scannable.
await waitFor(() => {
expect(screen.getByTestId("po-card-PO-A")).toBeInTheDocument();
expect(screen.getByTestId("po-card-PO-B")).toBeInTheDocument();
@ -514,21 +548,23 @@ describe("CollectionConfirmPage", () => {
const cardA = screen.getByTestId("po-card-PO-A");
const cardB = screen.getByTestId("po-card-PO-B");
// PO-A: open by default, table + chip + preview all visible.
expect(cardA.getAttribute("data-open")).toBe("true");
expect(within(cardA).getByText("Alpha row")).toBeInTheDocument();
expect(
within(cardA).getByRole("button", { name: /invoice-a\.pdf 1 of 1/i }),
).toBeInTheDocument();
expect(within(cardA).getByTestId("po-card-preview-PO-A")).toBeInTheDocument();
// Order number rides in the header even for the open card.
// PO-A: collapsed by default. Header is visible (PO + supplier
// + order + total) so the operator can scan, but the table
// and preview are hidden until they expand it.
expect(cardA.getAttribute("data-open")).toBe("false");
expect(within(cardA).getByTestId("po-card-title")).toHaveTextContent(
"PO PO-A — Alpha Bearings",
);
expect(
within(cardA).getByTestId("po-card-orders-PO-A"),
).toHaveTextContent("SO-A");
expect(within(cardA).getByTestId("po-card-total-PO-A")).toBeInTheDocument();
expect(within(cardA).queryByText("Alpha row")).not.toBeInTheDocument();
expect(
within(cardA).queryByTestId("po-card-preview-PO-A"),
).not.toBeInTheDocument();
// PO-B: collapsed by default. Header is visible (PO + supplier
// + order + total) so the operator can scan, but the table
// and preview are hidden until they expand it.
// PO-B: also collapsed by default.
expect(cardB.getAttribute("data-open")).toBe("false");
expect(within(cardB).getByTestId("po-card-title")).toHaveTextContent(
"PO PO-B — Beta Castings",
@ -542,8 +578,8 @@ describe("CollectionConfirmPage", () => {
within(cardB).queryByTestId("po-card-preview-PO-B"),
).not.toBeInTheDocument();
// Clicking PO-B's header expands it — operator moves on to
// the next PO when ready.
// Clicking PO-B's header expands it — operator opens the card
// they want to work on, then moves on.
await user.click(screen.getByTestId("po-card-toggle-PO-B"));
expect(cardB.getAttribute("data-open")).toBe("true");
expect(within(cardB).getByText("Beta row")).toBeInTheDocument();
@ -577,6 +613,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the items table is mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
fetchSpy.mockClear();
await user.click(screen.getByTestId("sku-edit-row-1"));
const input = screen.getByTestId("sku-input-row-1");
@ -612,12 +651,16 @@ describe("CollectionConfirmPage", () => {
];
buildFetchSpy(photoRows);
const user = userEvent.setup();
render(<Page />);
await waitFor(() =>
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the chips and preview are mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// The chips live on the card's strip — one per distinct
// filename. The preview block is mounted by default with the
// first chip active.
@ -682,6 +725,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the upload input is reachable.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
const fileInput = screen.getByLabelText(/Upload invoice for PO PO-1/i);
const file = new File(["pdf"], "invoice-late.pdf", { type: "application/pdf" });
await user.upload(fileInput, file);
@ -702,6 +748,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the items table is mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// Wheel hub kit cost is 45 → displayed as R45.00.
expect(screen.getByText("R45.00")).toBeInTheDocument();
@ -723,6 +772,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the items table is mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// Reset spy after the initial GET to isolate the cost POST.
fetchSpy.mockClear();
@ -771,6 +823,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the items table is mounted.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
fetchSpy.mockClear();
await user.click(screen.getByTestId("cost-edit-row-1"));
@ -799,6 +854,10 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the items table and confirm button
// are reachable.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// 1) Apply a cost override on row-1 (Enter triggers the cost POST +
// records the override in local state for the confirm submit).
await user.click(screen.getByTestId("cost-edit-row-1"));
@ -914,6 +973,9 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-SP-001")).toBeInTheDocument(),
);
// Open the card body so the footer is reachable.
await user.click(screen.getByTestId("po-card-toggle-SP-001"));
// Stock-only card: the "Book to stock" button is present, the
// "Confirm for invoicing" button is hidden.
expect(
@ -988,6 +1050,10 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
);
// Open the card body so the items table and confirm button
// are reachable.
await user.click(screen.getByTestId("po-card-toggle-PO-1"));
// Before the edit, the cell exposes `null` as its raw value.
const cell = document.querySelector('[data-cost-value]');
expect(cell).not.toBeNull();
@ -1070,12 +1136,15 @@ describe("CollectionConfirmPage", () => {
render(<Page />);
// Both POs render as their own self-contained cards (no queue
// sidebar, no modal).
// sidebar, no modal). All cards are collapsed by default.
await waitFor(() =>
expect(screen.getByTestId("po-card-PO-A")).toBeInTheDocument(),
);
expect(screen.getByTestId("po-card-PO-B")).toBeInTheDocument();
// Open PO-A's body so its items table is reachable.
await user.click(screen.getByTestId("po-card-toggle-PO-A"));
// 1) Edit PO-A's row cost — Enter triggers the cost POST + records
// the override in local state.
await user.click(screen.getByTestId("cost-edit-row-a"));
@ -1087,8 +1156,8 @@ describe("CollectionConfirmPage", () => {
expect(screen.getByText("R150.00")).toBeInTheDocument(),
);
// 2) Open PO-B's confirm dialog. PO-B is collapsed by default
// (only the first card starts open), so expand it first.
// 2) Open PO-B's confirm dialog. All cards are collapsed by
// default, so expand PO-B first.
fetchSpy.mockClear();
await user.click(screen.getByTestId("po-card-toggle-PO-B"));
await user.click(screen.getByTestId("po-card-confirm-PO-B"));

View file

@ -422,7 +422,7 @@ export default function CollectionConfirmPage() {
</div>
<div className="space-y-4" data-od-id="collection-confirm-list">
{groupedRows.map((group, index) => {
{groupedRows.map((group) => {
const firstRow = group.rows[0];
const supplierId =
firstRow?.expand?.purchase_order?.expand?.supplier?.id ?? "";
@ -438,7 +438,6 @@ export default function CollectionConfirmPage() {
rows={group.rows}
poNumber={group.poNumber || "No PO"}
supplier={{ id: supplierId, name: supplierName }}
initiallyOpen={index === 0}
onCostChange={(id, cost) => void handleCostChange(id, cost)}
onSkuChange={(id, sku) => void handleSkuChange(id, sku)}
onConfirm={() =>