Files
integreat/docs/plans/2026-06-27-001-fix-ssr-modal-regressions-plan.md
Bryce da2cc711d4 fix(ssr): vendor + client wizard step layout (vertical timeline)
BUG E/F: the Vendor and Client wizard step cards overflowed — horizontal
scrollbar, clipped fields, a huge empty grey left region. The step-card lays the
step timeline in a vertical left sidebar (grow-0, self-stretch), but
vendor-timeline/client-timeline rendered the HORIZONTAL `timeline` component,
which forced the shrink-to-fit sidebar to ~full card width and pushed the body
off-screen.

- vendor-timeline / client-timeline: use timeline/vertical-timeline +
  vertical-timeline-step (the components already existed) instead of the
  horizontal pair.
- vendors.clj: step bodies w-[600px] h-[350px] -> w-full h-[350px] so the body
  fills the width left of the now-narrow vertical timeline.

Verified live (agent-browser): Vendor 760x520, Client 820x560, vertical timeline,
no horizontal overflow, Info->Terms navigation + validation re-render lay out
correctly. Both files pass lein cljfmt check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 13:18:27 -07:00

203 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# SSR Modal Regression Fixes — QA Findings & Resumable Task List
> **Status:** BUG A, BUG C, BUG E/F, and the CSS size root cause are **FIXED + verified live**
> (2026-06-27). Remaining: animations (§3), BUG D, BUG B. Resume from §4.
> **Owner:** Bryce
> **Branch:** `integreat-execute-refactor`
> **Date:** 2026-06-27
> **Context:** Follow-up to the SSR rendering-modernization migration
> (`2026-06-02-001-refactor-ssr-rendering-modernization-plan.md`, Phases 211 complete).
> The migration to simplified route rendering + whole-form/wizard swaps introduced
> modal **size**, **animation**, and a set of **HTTP 500** regressions. This doc records a
> full browser QA pass (agent-browser, 9 migrated modals) with root causes and fixes.
## 0. How to resume / reproduce
- App is running on the port in `.http-port` (was `42987`); nREPL in `nrepl-port` (was `35689`).
- Browser session: `agent-browser --session integreat ...`. Log in via
`http://localhost:<port>/dev-login` → "Continue to dashboard".
- **Test at a realistic viewport:** `agent-browser --session integreat set viewport 1440 900`
(a short viewport exaggerates modal overflow and gives false positives).
- Screenshots from this pass are in `./tmp/` (gitignored): `02..19-*.png`. Raw running notes:
`./tmp/findings.md`.
- After rebuilding CSS, the browser caches the old `output.css`; bust it by swapping the
`<link>` href (`?v=<n>`) or hard-reloading, or you'll keep seeing the old sizes.
## 1. Test matrix (9 migrated modals)
| Modal | File | Type | Result |
|-------|------|------|--------|
| Transaction Edit | `transaction/edit.clj` | form | Opens OK; **BUG C** — whole-form swap 500 |
| Transaction Bulk Code | `transaction/bulk_code.clj` | form | ✅ Works |
| Sales Summary Edit | `pos/sales_summaries.clj` | form | ✅ Works |
| Invoice Bulk Edit | `invoices.clj` | form | ✅ Works (empty-selection 500 = **BUG D**) |
| Transaction Rule | `admin/transaction_rules.clj` | wizard | ✅ Works |
| Invoice Pay | `invoices.clj` | wizard | ✅ Works (narrow until CSS rebuild — see §2) |
| New Invoice | `invoice/new_invoice_wizard.clj` | wizard | **BUG A** — choose-client 500 |
| Vendor | `admin/vendors.clj` | wizard | Size broken (§2) + **BUG E** inner overflow |
| Client | `admin/clients.clj` | wizard | Size broken (§2) + **BUG F** inner overflow |
Cross-cutting: **§2** (stale CSS → sizes), **BUG B** (footer EDN leak), **animations** (§3).
## 2. ★ ROOT CAUSE of broken modal SIZES — stale compiled Tailwind CSS
`resources/public/output.css` (committed, last built **Jun 24**) is **missing the migration's
newer arbitrary size classes**. Tailwind only compiles classes present in source at build time,
so any size value added/changed after Jun 24 isn't in the CSS and the element falls back to its
`w-full h-full` sibling class → the modal balloons to full width.
- **Present** (these modals size correctly today): `md:w-[750px]` (New Invoice), `md:w-[950px]`
(Transaction Edit / Bulk Code / Sales Summary), `md:h-[600px]`, `md:h-[650px]`, `w-[850px]`
(Transaction Rule).
- **Missing** (these balloon / mis-size): `md:w-[760px]` (Vendor), `md:w-[820px]` (Client),
`md:h-[520px]`, `md:h-[560px]`, `w-[50em]` (Invoice Pay).
**Verified:** rebuilding to a temp file generates all missing classes; after rebuild + cache-bust,
the Vendor card went **1257px → 760×520** and Client → **820×560** (exactly as declared).
**DEEPER ROOT CAUSE found while fixing (2026-06-27):** `tailwind.config.js` `content` only scanned
`./src/**/*.{cljs,clj,cljc}` — it **never scanned `resources/templates/**/*.html`** (the 46 Selmer
templates the migration introduced). So a naive rebuild *drops* every template-only class
(e.g. `md:w-[950px]` / `md:h-[650px]`, used only in `templates/transaction-edit/edit-modal.html`,
which would have re-broken Transaction Edit / Bulk Code / Sales Summary). The durable fix is to add
the templates glob to the config, then rebuild.
**FIX — DONE & verified:**
1. Added `"./resources/templates/**/*.html"` to `tailwind.config.js` `content`.
2. `npx tailwindcss -i resources/input.css -o resources/public/output.css` (kept unminified to
match the committed file; add `--minify` only if the prod pipeline minifies).
- Verified: all modal size classes now present (`md:w-[760px]`, `md:w-[820px]`, `md:h-[520px]`,
`md:h-[560px]`, `w-[50em]` **and** the template-sourced `md:w-[950px]`/`md:h-[650px]`). Class-diff
vs the old CSS shows the only removed classes are orphaned (the deleted `mm/*` modal-stack
`forward`/`backward`/`group/transition`/`htmx-*:translate-x-2/3` animation set + the unused
`lg:w-[900px]` sales size) — none still referenced in src/ or templates/. Vendor modal confirmed
live at **760×520** (was 1257px).
- **Still TODO:** confirm the prod/CI build runs this tailwind step (so output.css can't drift
again) — ideally wire it into `lein build` / buildspec.
> Note: during QA I rebuilt `output.css` to verify, then reverted it so the working tree is clean
> for review. The verified rebuild is preserved at `tmp/output-rebuilt.css`; backup at
> `tmp/output.css.bak`.
## 3. Modal ANIMATIONS
The new wizard step cards swap via `hx-target "this" hx-swap "outerHTML"` but their
`modal-card-advanced` carries **no transition classes**, so step→step and modal enter/leave do not
animate:
- New Invoice steps (`render-basic-details`, `render-accounts`): card class is only
`md:w-[750px] md:h-[600px] w-full h-full` — no `htmx-swapping:`/`htmx-added:` variants.
- `next-steps-modal` (new_invoice_wizard.clj ~678): only static `scale-100 translate-x-0
opacity-100` — missing the `htmx-swapping:`/`htmx-added:` swap variants.
- By contrast, `dialog/success-modal-` and the Invoice Pay card (`last-modal-step transition
duration-150`) keep transitions, so the intended pattern still exists to copy from.
**FIX:** add the swap-transition classes to the wizard step cards (mirror `success-modal-`'s
`transition duration-300 ease-in-out htmx-swapping:... htmx-added:...` string, or the lighter
`last-modal-step transition duration-150` used by the transaction-edit card). Confirm the
`htmx-swapping:`/`htmx-added:` custom variants survive the §2 CSS rebuild.
## 4. Bug list + fixes (prioritized task list)
### P0 — functional 500s
**BUG A — New Invoice: choosing a client → 500 from `/invoice/new/due-date`.**
- File: `src/clj/auto_ap/ssr/invoice/new_invoice_wizard.clj`, fn `due-date` (and same latent bug in
`scheduled-payment-date`).
- Exception: `ClassCastException: org.joda.time.DateTime cannot be cast to java.util.Date`.
- Cause: `form-vendor-client` decodes `:invoice/date`/`:invoice/due` to **clj-time `DateTime`**
(via `clj-date-schema`), but `due-date` then calls `(some-> date coerce/from-date)` —
`coerce/from-date` expects a `java.util.Date`. `scheduled-payment-date` has the same
`(some-> due coerce/from-date)` but is masked because `due` is nil-guarded by a `when`.
- Repro: open New Invoice, pick any client (vendor still empty) → red "unexpected error" banner;
network shows PUT `/invoice/new/due-date` = 500. REPL-confirmed the decoded date is already a
`DateTime` and `time/plus` works on it directly.
- **FIX — DONE & verified live:** dropped the `coerce/from-date` calls — `due-date` now uses the
decoded `date`/`due` `DateTime`s directly (removed the `date (some-> date coerce/from-date)`
rebind and the `due` coerce); same for `scheduled-payment-date`. Live: selecting a client now
fires PUT `/invoice/new/due-date` + `/scheduled-payment-date` → **200** (was 500).
**BUG C — Transaction Edit: any whole-form swap (mode toggle, vendor change, add/remove row, $/%
toggle) → 500 from `/transaction2/edit-form-changed`, whenever the txn has ≥1 autopay-invoice
match.** (HIGH severity — breaks the flagship modal's core swap doctrine.)
- File: `src/clj/auto_ap/ssr/transaction/edit.clj`.
- Exception: `ClassCastException: PersistentVector cannot be cast to Named` at **edit.clj:849**
(`links-body*` does `(name (:action step-params))`).
- Cause: the link-panels each render a hidden `<input name="action">`. The **unpaid** panel
(line 690) and **rule** panel (line 730) add `:form ""` to exclude that hidden from form
serialization. The **autopay** panel (**line 661**) is **missing `:form ""`**, so its
`action="link-autopay-invoices"` hidden is serialized alongside the main `action` hidden (line
862, value "manual"). Ring collapses the duplicate `action` params into a vector → `(name
vector)` throws.
- Repro: edit a transaction whose "Link to autopay invoices" tab shows a badge count ≥1, click
"Switch to advanced mode". REPL-confirmed: single `action` → 200; `action` as a vector → the
exact 500. (Txns with 0 autopay matches render `panel-empty*` with no action-hidden and swap
fine — hence intermittent.)
- **FIX — DONE & verified live:** added `:form ""` to the autopay panel's action-hidden
(**edit.clj:661**) to match the unpaid/rule panels. Live: on a txn with an autopay match,
"Switch to advanced mode" now POSTs `edit-form-changed` → **200** with a single `action=manual`
param (the duplicate `action=link-autopay-invoices` is gone) and the mode toggles correctly.
- (Optional hardening, not applied: make `links-body*` coerce a vector action via
`(some-> (:action step-params) (as-> a (if (coll? a) (last a) a)) name)`.)
### P1 — sizing / layout (after §2 CSS rebuild)
**§2 CSS rebuild** — do first; fixes Vendor/Client/Invoice-Pay widths.
**BUG E/F — Vendor + Client wizard step layout overflows the card** — **DONE & verified live.**
- **Actual root cause (found while fixing):** `step-card` places the step timeline in a *vertical*
left sidebar (`grow-0 ... self-stretch hidden md:block`), but `vendor-timeline`/`client-timeline`
rendered the **horizontal** `timeline/timeline` component. A horizontal `<ol>` in a shrink-to-fit
sidebar forced the sidebar to ~full card width, pushing the body off-screen → horizontal scroll,
clipped fields, the "huge grey left region". (Vendor additionally pinned its bodies to a fixed
`w-[600px]`, compounding it.)
- **FIX applied:**
- `vendors.clj` `vendor-timeline` + `clients.clj` `client-timeline`: use
`timeline/vertical-timeline` + `timeline/vertical-timeline-step` (the components already existed)
instead of the horizontal pair.
- `vendors.clj`: the 5 step bodies' `w-[600px] h-[350px]` → `w-full h-[350px]` so the body fills
the remaining width next to the now-narrow vertical timeline.
- **Verified live:** Vendor 760×520 and Client 820×560 with a proper vertical timeline, **no
horizontal overflow**, fields fill the body; forward nav (Info→Terms) works, the vendor-name chip
renders, and the validation-error re-render also lays out correctly.
- Note: edits made with the plain `Edit` tool (exact-string) — the clojure-mcp editors reformat the
whole file against a stricter config than the project's `lein cljfmt`, producing large spurious
diffs; both files pass `lein cljfmt check`.
### P2 — robustness / cosmetic
**BUG D — Invoice Bulk Edit with no selection → 500** (global "Oh, drat!" toast). Works fine with
a selection. Should no-op or show a friendly "select invoices first" message.
- File: `src/clj/auto_ap/ssr/invoices.clj` (bulk-edit open handler).
**BUG B — Footer leaks a raw Hiccup attr-map as text** in the red "unexpected error" banner
(`{:x-show "unexpectedError", ...}`). **Pre-existing in master** (NOT a migration regression), but
visible whenever `unexpectedError` flips true (e.g. it showed during BUG A). Cause: `modal-footer-`
calls `(hx/alpine-appear {...})` **twice** — the 2nd return value lands in child position and
renders as literal EDN.
- File: `src/clj/auto_ap/ssr/components/dialog.clj` ~line 59.
- **FIX:** delete the duplicate `(hx/alpine-appear ...)` line in `modal-footer-`.
**Cosmetic — over-tall empty modals.** Invoice/Transaction Bulk Edit and similar use fixed
`md:h-[650px]`; with little content they show a large empty lower region. Consider letting height
hug content (cap with `max-h`) rather than a fixed height. Low priority.
## 5. Suggested order of work
1. **§2 CSS rebuild** (unblocks all width checks; commit `output.css`; verify prod build does this).
2. **BUG A** + **BUG C** (the two functional 500s; one-liners each, REPL-verified).
3. **BUG E / BUG F** (wizard step-body layout rework; needs visual iteration per step).
4. **§3 animations** (add swap-transition classes to wizard step cards).
5. **BUG D**, **BUG B** (robustness / cosmetic).
6. Re-run the full agent-browser pass (all 9 modals) + the Playwright e2e suite as the parity gate.
## 6. What was verified vs. inferred
- **Verified in browser + REPL:** BUG A (500 + exact exception + fix direction), BUG C (500 +
exact exception + vector repro + line 661 cause), §2 CSS root cause (missing classes + rebuild
fixes Vendor 1257→760 and Client→820), BUG D (500 on empty selection), Vendor/Client inner
overflow (screenshots 16/17), and that Bulk Code / Sales Summary / Transaction Rule / Invoice
Pay / Transaction Edit-open all render without errors.
- **Inferred (static analysis, not yet visually A/B'd against master):** §3 animation regression
(step cards lack `htmx-swapping:`/`htmx-added:` classes) and BUG B being pre-existing.