Files
integreat/docs/plans/2026-06-27-001-fix-ssr-modal-regressions-plan.md
Bryce 6c791efb06 feat(ssr): subtle fade-in on wizard step cards
§3 animations: the migrated wizard step cards had no transition, so step→step
swaps and modal open were flat. The old mm/* slide system was deleted in Phase
11 (and its classes purged from CSS), and the transaction-edit "reference" uses
an undefined `last-modal-step` no-op — so there was no clean slide to restore.

Apply the codebase's existing `fade-in transition-opacity duration-300`
primitive (`.htmx-added .fade-in` in input.css) to all three wizard step cards
(new-invoice basic-details + accounts, vendor step-card, client step-card). Each
card now fades in on open and on every step swap. Verified live: cards always
settle to opacity 1 (never stuck invisible) on both open and step navigation.

Richer directional (forward/back) slide transitions are left for a design pass.

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

221 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 (subtle fade) & verified:** added the codebase's existing `fade-in transition-opacity
duration-300` to all three wizard step cards (new-invoice basic-details + accounts, vendor
step-card, client step-card). `fade-in` is defined in `resources/input.css`
(`.htmx-added .fade-in { opacity:0 }` → transitions to 1 after htmx settle), so each card fades in
on open *and* on every step swap. Verified live: cards always settle to **opacity 1** (never stuck
invisible) on both open and step navigation — no functional risk.
Notes / intentionally deferred (needs design intent + visual sign-off, so not done autonomously):
- The transaction-edit "reference" card's `last-modal-step` class is **undefined** (a no-op); its
only real transition is `transition duration-150`. So there was no clean reference slide to copy.
- The richer **forward/backward slide** transitions (the old `mm/*` modal-stack system using
`group/transition` + `forward`/`backward` + `htmx-*:translate-x-2/3`) were deliberately deleted in
Phase 11 and purged from the CSS. Re-introducing directional slides is a larger design decision
(which element carries `htmx-swapping`/`htmx-added` — the form vs the card — plus settle timing)
and is left for a human-in-the-loop pass if the subtle fade isn't enough.
## 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.