Files
integreat/docs/plans/2026-06-27-001-fix-ssr-modal-regressions-plan.md
Bryce b44213bffd feat(ssr): restore forward/back step slide in the wizard engine
The engine migration replaced the old mm/* modal-stack wizards (which slid
forward/back between steps) with wizard2, but never carried the slide over — step
transitions went flat. Restore the original mechanism in the shared engine so all
wizards (new-invoice, vendor, client, pay, transaction-rule) get it:

- wizard2/step-slide-classes: the group-[.forward]/transition:htmx-* and
  group-[.backward]/* slide variants, applied to the swapped <form>.
- wizard2/transitioner: the #transitioner wrapper whose @htmx:after-request hook
  reads the x-transition-type response header and toggles group/transition +
  forward|backward on itself. All 5 configs' :open-response now use it.
- wizard2/handle-step-submit sets x-transition-type (forward on advance, backward
  on Back, none on a same-step validation re-render) + HX-reswap "outerHTML
  swap:0.16s" so the slide-out plays before the swap. Direction computed from
  step order (transition-type).
- Removed the interim per-card fade-in in favor of this.
- Rebuilt output.css so the 16 fwd + 16 back slide variants are compiled.

REPL-verified: open-wizard emits the transitioner, the form carries the slide
classes, and submit responses carry the transition headers. Live verification
needs a server refresh (the dev server froze its route table at startup).

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

228 lines
15 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:** ALL identified items **FIXED + verified** (2026-06-27) — BUG A, BUG C, BUG D, BUG B,
> BUG E/F, the CSS size root cause, and a subtle wizard fade-in (§3). The only thing intentionally
> left is the optional richer forward/back *slide* transition, which needs design sign-off (§3).
> **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 — DONE (forward/back slide restored) in the shared `wizard2` engine.** The user confirmed the
old wizard had a directional **slide forward/back** between steps; the engine migration dropped it.
Restored the original mechanism (read out of the deleted `mm/multi_modal.clj`) in one shared place:
- `wizard2/step-slide-classes` — the `group-[.forward]/transition:htmx-*` + `group-[.backward]/…`
variants, now applied to the swapped wizard `<form>`.
- `wizard2/transitioner` — the `#transitioner` wrapper with the `@htmx:after-request` hook that reads
the `x-transition-type` response header and toggles `group/transition` + `forward|backward` on
itself (so the variants fire on the next swap). All 5 wizard configs' `:open-response` now wrap the
form in `wizard2/transitioner` instead of a plain `#transitioner` div.
- `wizard2/handle-step-submit` now sets `x-transition-type` (forward on advance, backward on Back,
`none` on a same-step validation re-render) + `HX-reswap: outerHTML swap:0.16s` (the swap delay
that lets the slide-out play). Direction is computed from the step order (`transition-type`).
- The earlier per-card `fade-in` (interim) was removed in favor of this.
- CSS rebuilt so the `group-[.forward]/transition:htmx-*` variants (16 fwd + 16 back) are compiled.
- Applies to ALL wizards (new-invoice, vendor, client, pay, transaction-rule) since it lives in the
engine. REPL-verified: `open-wizard` emits the transitioner, the form carries the slide classes,
and submit responses carry `x-transition-type` + the `HX-reswap` swap delay.
- **Live-verify caveat:** the long-lived dev server froze its route table at startup
(`auto-ap.handler/match->handler-lookup` is a `def` that merged the per-ns `key->handler` maps), so
an nREPL reload of leaf namespaces does NOT reach the running router — a server refresh
(`stop jetty → tools.namespace refresh → user/start-http`) or a fresh `lein run` is needed to see
it live. (Documented hazard; see the QA memory.)
## 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** — **DONE & verified live.** With no
selection, `selected->ids` returns `nil`, which `all-ids-not-locked` fed straight into a Datomic
`:in $ [?i ...]` query → "Unable to find data source" exception → global "Oh, drat!" toast.
- File: `src/clj/auto_ap/ssr/invoices.clj` — wrapped `all-ids-not-locked`'s body in
`(when (seq all-ids) ...)`, so an empty/nil selection yields `[]` and the modal opens cleanly
(also protects every other caller). Live: Bulk Edit with nothing selected now opens the modal,
no toast, no 500.
**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 — DONE & verified:** deleted the duplicate `(hx/alpine-appear ...)` line in `modal-footer-`.
Rendering the footer to HTML now shows a single `alpine-appear` (the legit attrs) and no literal
`{:x-show ...}` text child.
**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.