From 73b20464fc4d88c356dca14906fa903856c7a2bf Mon Sep 17 00:00:00 2001 From: Josh Zhang <39790535+jiashuoz@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:51:25 -0700 Subject: [PATCH] fix(web): make the add-on stepper's total-quantity semantics explicit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dogfooding on staging surfaced the misread immediately: owning 1 add-on and stepping to 2, the page showed 'Update add-ons' with 'New total: $4/mo' — which reads as 'buy 2 more for another $4', when it means 'own 2 in total, +$2/mo'. On a money control that ambiguity is a defect. - The stepper gains a visible 'Total add-ons' label. - A staged change names the total on the button itself: 'Update to 2 add-ons' / 'Buy 2 add-ons'. - The preview states both the new monthly total AND the delta this click commits: 'New monthly total: $4/mo (+1 add-on, +$2/mo)' (negative deltas rendered on decreases). Tests: 2 new cases pinning the reported scenario (add one more to an owned add-on; reduce shows a negative delta); labels updated across the suite, 17/17; full web suite green; tsc + prod build clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ScEpytuD7GvaXEssvJ2W32 --- web/src/app/(app)/billing/page.addon.test.tsx | 61 ++++++++++++++++--- web/src/app/(app)/billing/page.tsx | 35 +++++++++-- 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/web/src/app/(app)/billing/page.addon.test.tsx b/web/src/app/(app)/billing/page.addon.test.tsx index 606ef014c..ee91b4b75 100644 --- a/web/src/app/(app)/billing/page.addon.test.tsx +++ b/web/src/app/(app)/billing/page.addon.test.tsx @@ -222,8 +222,10 @@ describe("BillingPage — inbox add-on", () => { const inc = screen.getByRole("button", { name: "Increase add-on quantity" }); await userEvent.click(inc); await userEvent.click(inc); - expect(screen.getByText(/New total:/)).toHaveTextContent("$4/mo"); - await userEvent.click(screen.getByRole("button", { name: "Buy add-ons" })); + expect(screen.getByText(/New monthly total:/)).toHaveTextContent( + "$4/mo (+2 add-ons, +$4/mo)", + ); + await userEvent.click(screen.getByRole("button", { name: "Buy 2 add-ons" })); await waitFor(() => expect(addonPosts()).toEqual([{ quantity: 2 }])); // Took the redirect branch: no in-place provisioning notice, no @@ -244,7 +246,7 @@ describe("BillingPage — inbox add-on", () => { expect(screen.getByLabelText("Add-on quantity")).toHaveValue(1); await userEvent.click(screen.getByRole("button", { name: "Increase add-on quantity" })); - await userEvent.click(screen.getByRole("button", { name: "Update add-ons" })); + await userEvent.click(screen.getByRole("button", { name: "Update to 2 add-ons" })); // An in-place increase charges immediately → the confirm carries the // new total. @@ -267,12 +269,14 @@ describe("BillingPage — inbox add-on", () => { await screen.findByText("Inbox add-on"); await userEvent.click(screen.getByRole("button", { name: "Increase add-on quantity" })); - await userEvent.click(screen.getByRole("button", { name: "Update add-ons" })); + await userEvent.click(screen.getByRole("button", { name: "Update to 2 add-ons" })); expect(window.confirm).toHaveBeenCalled(); expect(addonPosts()).toEqual([]); - // Declining leaves the page interactive. - expect(screen.getByRole("button", { name: "Update add-ons" })).not.toBeDisabled(); + // Declining leaves the page interactive, the staged total still named. + expect( + screen.getByRole("button", { name: "Update to 2 add-ons" }), + ).not.toBeDisabled(); }); it("disables the action when the desired quantity equals the current one", async () => { @@ -335,11 +339,13 @@ describe("BillingPage — inbox add-on", () => { await screen.findByText("Inbox add-on"); await userEvent.click(screen.getByRole("button", { name: "Decrease add-on quantity" })); - await userEvent.click(screen.getByRole("button", { name: "Update add-ons" })); + await userEvent.click(screen.getByRole("button", { name: "Update to 0 add-ons" })); await waitFor(() => expect(window.alert).toHaveBeenCalled()); // Failure clears the in-flight state so the user can retry. - expect(screen.getByRole("button", { name: "Update add-ons" })).not.toBeDisabled(); + expect( + screen.getByRole("button", { name: "Update to 0 add-ons" }), + ).not.toBeDisabled(); }); }); @@ -354,7 +360,7 @@ describe("BillingPage — add-on provisioning sync", () => { async function stageAndApply(user: ReturnType) { await screen.findByText("Inbox add-on"); await user.click(screen.getByRole("button", { name: "Increase add-on quantity" })); - await user.click(screen.getByRole("button", { name: "Update add-ons" })); + await user.click(screen.getByRole("button", { name: "Update to 2 add-ons" })); await waitFor(() => expect(screen.getByText(/Updating your add-ons/)).toBeInTheDocument(), ); @@ -445,3 +451,40 @@ describe("BillingPage — owned add-on visibility", () => { expect(screen.queryByText(/inbox add-on ·/)).not.toBeInTheDocument(); }); }); + +describe("BillingPage — total-vs-delta clarity", () => { + it("names the staged TOTAL and the delta when adding one more to an owned add-on", async () => { + limitsPayload = PRO_LIMITS; + planPayload = proPlan(1); + renderPage(); + + await screen.findByText("Inbox add-on"); + // The stepper is labeled as the account TOTAL. + expect(screen.getByText("Total add-ons")).toBeInTheDocument(); + + // Owning 1 and stepping to 2 = buying ONE more — the button and the + // preview both say so, so "2" can't read as "add 2 minimum". + await userEvent.click(screen.getByRole("button", { name: "Increase add-on quantity" })); + expect( + screen.getByRole("button", { name: "Update to 2 add-ons" }), + ).toBeInTheDocument(); + expect(screen.getByText(/New monthly total:/)).toHaveTextContent( + "$4/mo (+1 add-on, +$2/mo)", + ); + }); + + it("shows a negative delta when reducing", async () => { + limitsPayload = PRO_LIMITS; + planPayload = proPlan(3); + renderPage(); + + await screen.findByText("Inbox add-on"); + await userEvent.click(screen.getByRole("button", { name: "Decrease add-on quantity" })); + expect( + screen.getByRole("button", { name: "Update to 2 add-ons" }), + ).toBeInTheDocument(); + expect(screen.getByText(/New monthly total:/)).toHaveTextContent( + "$4/mo (\u22121 add-on, \u2212$2/mo)", + ); + }); +}); diff --git a/web/src/app/(app)/billing/page.tsx b/web/src/app/(app)/billing/page.tsx index 554f79593..0dd5bfbd0 100644 --- a/web/src/app/(app)/billing/page.tsx +++ b/web/src/app/(app)/billing/page.tsx @@ -948,6 +948,13 @@ export default function BillingPage() { )}
+ {/* The stepper holds the desired TOTAL, not a delta — a + user who owns 1 and wants one more steps to 2. Say so + on screen: without the label, "2" next to an owned + "1" reads as "buy 2 more". */} +
applyAddon(addonQty)} className="px-3 py-1.5 rounded-md text-sm font-medium bg-accent text-white hover:bg-accent/90 transition disabled:opacity-50 disabled:cursor-not-allowed" > + {/* When a change is staged, the label names the TOTAL + being committed ("Update to 2 add-ons"), so the + action can't be misread as buying a delta. */} {actionPending === "addon" ? addonInPlace ? "Saving…" : "Opening…" : addonSync === "pending" ? "Updating…" + : addonQty !== addonServerQty + ? `${addonInPlace ? "Update to" : "Buy"} ${formatNumber( + addonQty, + )} add-on${addonQty === 1 ? "" : "s"}` : addonInPlace ? "Update add-ons" : "Buy add-ons"} - {/* Proposed new monthly total, shown the moment the - staged quantity diverges — nobody should commit to a - number they haven't seen. */} + {/* Proposed new monthly total plus the delta from what's + owned, shown the moment the staged quantity diverges — + nobody should commit to a number they haven't seen, + and the delta says what this click actually changes. */} {addonQty !== addonServerQty && ( - New total:{" "} + New monthly total:{" "} {addonQty === 0 ? "$0/mo" : formatPrice( addonQty * planData.addon.monthly_price_cents_per_unit, - )} + )}{" "} + ({addonQty > addonServerQty ? "+" : "−"} + {formatNumber(Math.abs(addonQty - addonServerQty))} add-on + {Math.abs(addonQty - addonServerQty) === 1 ? "" : "s"},{" "} + {addonQty > addonServerQty ? "+" : "−"} + {formatPrice( + Math.abs(addonQty - addonServerQty) * + planData.addon.monthly_price_cents_per_unit, + )} + ) )}