diff --git a/e2e/transaction-edit-morph.spec.ts b/e2e/transaction-edit-swap.spec.ts similarity index 81% rename from e2e/transaction-edit-morph.spec.ts rename to e2e/transaction-edit-swap.spec.ts index 6ee9ca62..27d79e0a 100644 --- a/e2e/transaction-edit-morph.spec.ts +++ b/e2e/transaction-edit-swap.spec.ts @@ -1,11 +1,19 @@ import { test, expect } from '@playwright/test'; -// These tests cover the "render the whole form via an htmx + alpine morph swap" -// behaviour on the transaction edit page. The whole-form approach exists so that -// any edit can hit its own route yet re-render the entire form, while keeping the -// user's focus and caret position intact (which a plain innerHTML swap destroys). +// These tests cover the "post the whole form, swap back only what changed" +// behaviour on the transaction edit page. Each edit hits its own route and the +// server re-renders the entire form, but the client swaps back a targeted slice: +// - discrete changes (vendor, account, location, mode, add/remove row) swap +// the #manual-coding-section fragment via hx-select (+ an OOB refresh of the +// #wizard-snapshot hidden field so the round-tripped wizard state stays in +// sync); +// - typed fields never swap the input the user is in -- the amount field +// OOB-swaps only the #total/#balance cells (hx-swap=none), and the memo +// posts with hx-swap=none. +// Because the active input is never part of a swapped region, focus and caret +// survive a plain swap with no morph extension involved. -// Collect any uncaught page errors or console errors so a morph that throws +// Collect any uncaught page errors or console errors so a swap that throws // (e.g. a tooltip callback dereferencing a stale $refs) fails the test loudly. function trackErrors(page: any): string[] { const errors: string[] = []; @@ -28,7 +36,7 @@ async function openManualAdvanced(page: any, transactionIndex = 0) { await page.click('button:has-text("Manual")'); // First transaction has no accounts so it opens in "simple" mode. Switch to - // advanced mode (a whole-form morph swap) so the account grid is present. + // advanced mode (a section swap) so the account grid is present. const advancedLink = page.locator('a:has-text("Switch to advanced mode")'); if (await advancedLink.count()) { await advancedLink.first().click(); @@ -37,7 +45,7 @@ async function openManualAdvanced(page: any, transactionIndex = 0) { } // Drives the vendor typeahead like a user: open the dropdown, inject a result -// (Solr is unavailable in tests), click it, and wait for the whole-form morph. +// (Solr is unavailable in tests), click it, and wait for the section swap. async function selectVendor(page: any, vendorId: number, label: string) { const vendor = page .locator('div[hx-post*="edit-vendor-changed"]') @@ -63,9 +71,9 @@ async function selectVendor(page: any, vendorId: number, label: string) { await page.waitForTimeout(400); } -// Removes every existing account row (each remove is its own whole-form morph), -// so a test starts from a known-empty state regardless of what earlier tests -// saved onto the shared transaction. +// Removes every existing account row (each remove is its own section swap), so a +// test starts from a known-empty state regardless of what earlier tests saved +// onto the shared transaction. async function clearAccounts(page: any) { // eslint-disable-next-line no-constant-condition while (true) { @@ -81,13 +89,13 @@ async function clearAccounts(page: any) { test.describe.configure({ mode: 'serial' }); -test.describe('Transaction Edit whole-form morph', () => { - test('morph swaps (toggle mode, add account) do not throw', async ({ page }) => { +test.describe('Transaction Edit whole-form swap', () => { + test('section swaps (toggle mode, add account) do not throw', async ({ page }) => { const errors = trackErrors(page); await openManualAdvanced(page, 0); - // Add an account row -- another whole-form morph swap. + // Add an account row -- another section swap. await page .locator('#account-grid-body') .locator('button:has-text("New account"), a:has-text("New account")') @@ -98,12 +106,12 @@ test.describe('Transaction Edit whole-form morph', () => { .poll(async () => page.locator('#account-grid-body tbody tr.account-row').count()) .toBeGreaterThan(0); - // The form must survive the morph intact. + // The form must survive the swap intact. await expect(page.locator('#wizard-form')).toHaveCount(1); expect(errors, errors.join('\n')).toEqual([]); }); - test('keeps focus and typed value in the amount field across a morph', async ({ page }) => { + test('keeps focus and typed value in the amount field across a swap', async ({ page }) => { const errors = trackErrors(page); await openManualAdvanced(page, 0); @@ -125,9 +133,10 @@ test.describe('Transaction Edit whole-form morph', () => { await amount.waitFor(); // Type a clean value via the keyboard. Typing fires the field's htmx trigger - // (keyup), which posts to a per-field route and morphs the whole form back - // in. The amount field is type=number (no text caret), so we assert focus + - // node identity + value -- the guarantees morph gives that innerHTML can't. + // (keyup), which posts the whole form but swaps back ONLY the total/balance + // cells out-of-band (hx-swap=none on the field itself). The amount field is + // type=number (no text caret), so we assert focus + node identity + value -- + // the input is never replaced, which is what makes that hold. await amount.click(); await amount.press('Control+a'); @@ -139,7 +148,7 @@ test.describe('Transaction Edit whole-form morph', () => { ); await amount.pressSequentially('150', { delay: 40 }); - // Identify the live focused node (before the debounced morph lands) so we can + // Identify the live focused node (before the debounced swap lands) so we can // prove the *same* node survives. await page.evaluate(() => { (window as any).__focusedAmount = document.activeElement; @@ -157,20 +166,21 @@ test.describe('Transaction Edit whole-form morph', () => { }; }); - // Focus must stay on the amount field after the morph... + // Focus must stay on the amount field after the swap... expect(state.isAmountField).toBe(true); - // ...on the very same DOM node (this is what morph buys us over innerHTML)... + // ...on the very same DOM node (the input is never part of the swapped region)... expect(state.sameNode).toBe(true); // ...with the value the user typed left intact. expect(state.value).toBe('150'); - // The TOTAL must have recomputed server-side from the posted amount. + // The TOTAL must have recomputed server-side from the posted amount and been + // applied via the out-of-band swap. await expect(page.locator('.account-total-row #total')).toContainText('150'); expect(errors, errors.join('\n')).toEqual([]); }); - test('preserves caret position in the memo text field across a morph', async ({ page }) => { + test('preserves caret position in the memo text field across a swap', async ({ page }) => { const errors = trackErrors(page); await page.goto('/transaction2'); @@ -182,7 +192,8 @@ test.describe('Transaction Edit whole-form morph', () => { await memo.waitFor(); // Clear any seeded memo text, then type "hello" via the keyboard (fires the - // field's htmx keyup trigger) and let that first whole-form morph settle. + // field's htmx keyup trigger) and let that first post settle. Memo posts with + // hx-swap=none, so nothing is swapped back into the field. await memo.click(); await memo.press('Control+a'); const firstSwap = page.waitForResponse( @@ -204,7 +215,7 @@ test.describe('Transaction Edit whole-form morph', () => { (window as any).__focusedMemo = document.activeElement; }); - // Insert a char at the caret -> "heXllo", caret moves to 3, fires the swap. + // Insert a char at the caret -> "heXllo", caret moves to 3, fires the post. const memoSwap = page.waitForResponse( (r: any) => r.url().includes('edit-form-changed') && @@ -239,7 +250,7 @@ test.describe('Transaction Edit whole-form morph', () => { await openManualAdvanced(page, 0); // Start from a clean, empty account row so selecting the account actually - // changes accountId (and fires the change-gated whole-form morph). + // changes accountId (and fires the change-gated section swap). await clearAccounts(page); await page .locator('#account-grid-body') @@ -260,7 +271,7 @@ test.describe('Transaction Edit whole-form morph', () => { // Account search is backed by Solr (unavailable in tests), so type under the // 3-char threshold and inject a clickable result into the typeahead state -- - // the click handler, tippy.hide(), Alpine reactivity and HTMX morph all run + // the click handler, tippy.hide(), Alpine reactivity and the HTMX swap all run // exactly as in production. await search.fill('te'); const testInfo = await (await page.request.get('/test-info')).json(); @@ -269,9 +280,8 @@ test.describe('Transaction Edit whole-form morph', () => { (window as any).Alpine.$data(el).elements = [{ value: id, label: 'Test Account' }]; }, accountId); - // Clicking the result runs `value = element; tippy.hide(); ...`. Before the - // fix this threw "tippy is null" because the cached tippy var was stale after - // an earlier morph. + // Clicking the result runs `value = element; tippy.hide(); ...` and dispatches + // the change that fires the section swap. const swap = page.waitForResponse( (r: any) => r.url().includes('edit-form-changed') && @@ -282,7 +292,7 @@ test.describe('Transaction Edit whole-form morph', () => { await swap; await page.waitForTimeout(300); - // The chosen account must survive the whole-form morph. + // The chosen account must survive the section swap. const hidden = page .locator('#account-grid-body tbody tr.account-row') .first() @@ -293,7 +303,7 @@ test.describe('Transaction Edit whole-form morph', () => { expect(errors, errors.join('\n')).toEqual([]); }); - test('selecting a vendor populates its default account across the morph', async ({ page }) => { + test('selecting a vendor populates its default account across the swap', async ({ page }) => { const errors = trackErrors(page); // Open the modal in simple mode (transaction 0 has no accounts). @@ -329,9 +339,9 @@ test.describe('Transaction Edit whole-form morph', () => { await swap; await page.waitForTimeout(400); - // The vendor's default account must now be reflected in the account field -- - // this is the bug the `key` re-init fixes: a server-driven value change into an - // Alpine-stateful typeahead that morph would otherwise preserve as empty. + // The vendor's default account must now be reflected in the account field. + // Because the section is rebuilt fresh from the server (no preserved Alpine + // state), the server-driven account value lands without any keying tricks. const accountHidden = page .locator('input[type="hidden"][name*="transaction-account/account"]') .first(); @@ -371,8 +381,9 @@ test.describe('Transaction Edit whole-form morph', () => { await expect(vendorLabel).toHaveText('Test Vendor'); await expect(accountHidden).toHaveValue(account1.toString()); - // Second vendor -- this is the regression: a morph-preserved typeahead lost its - // value watcher, so the second change fired no request at all. + // Second vendor -- the regression guard: the section (and its vendor + // typeahead) is rebuilt fresh on every swap, so a second change still fires + // its request and updates the default account. await selectVendor(page, vendor2, 'Second Vendor'); await expect(vendorLabel).toHaveText('Second Vendor'); await expect(accountHidden).toHaveValue(account2.toString()); diff --git a/src/clj/auto_ap/ssr/components/multi_modal.clj b/src/clj/auto_ap/ssr/components/multi_modal.clj index 898343bb..f7f76ec6 100644 --- a/src/clj/auto_ap/ssr/components/multi_modal.clj +++ b/src/clj/auto_ap/ssr/components/multi_modal.clj @@ -305,6 +305,11 @@ (list (fc/with-field :snapshot (com/hidden {:name (fc/field-name) + ;; Stable id so a partial swap (e.g. the transaction + ;; edit form swapping only #manual-coding-section) can + ;; refresh the encoded snapshot out-of-band and keep the + ;; round-tripped wizard state in sync. + :id "wizard-snapshot" :value (pr-str (fc/field-value))})) (fc/with-field :edit-path (com/hidden {:name (fc/field-name) diff --git a/src/clj/auto_ap/ssr/transaction/edit.clj b/src/clj/auto_ap/ssr/transaction/edit.clj index 924670a9..59fec8b2 100644 --- a/src/clj/auto_ap/ssr/transaction/edit.clj +++ b/src/clj/auto_ap/ssr/transaction/edit.clj @@ -233,8 +233,10 @@ :x-dispatch:changed "simpleAccountId" :hx-trigger "changed" :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) - :hx-target "#wizard-form" - :hx-swap "morph" + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML" :hx-include "closest form"} (location-select* {:name (fc/field-name) @@ -248,8 +250,10 @@ [:a.text-sm.text-blue-600.hover:underline.cursor-pointer {:hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-wizard-toggle-mode) :hx-include "closest form" - :hx-target "#wizard-form" - :hx-swap "morph"} + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML"} "Switch to advanced mode"]]])) (defn- manual-mode-initial @@ -264,11 +268,6 @@ (com/data-grid-row (-> {:class "account-row" :id (str "account-row-" index) - ;; Key the row by its account id (alpine-morph keys off `key`, not `id`) so - ;; a server-driven account change re-inits the row's x-data. Otherwise morph - ;; preserves the stale accountId and the account typeahead (x-model="accountId") - ;; snaps back to the old value. - :key (str "account-row-" index "--" (fc/field-value (:transaction-account/account value))) :x-data (hx/json {:show (boolean (not (fc/field-value (:new? value)))) :accountId (fc/field-value (:transaction-account/account value))}) :data-key "show" @@ -297,8 +296,10 @@ :x-dispatch:changed "accountId" :hx-trigger "changed" :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) - :hx-target "#wizard-form" - :hx-swap "morph" + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML" :hx-include "closest form"} (location-select* {:name (fc/field-name) :account-location (:account/location (cond->> (:transaction-account/account @value) @@ -311,16 +312,17 @@ {} (com/validated-field {:errors (fc/field-errors)} - ;; Editing an amount re-renders the whole form (so TOTAL/BALANCE recompute). - ;; The stable id lets alpine-morph match this exact input across the swap, - ;; keeping the user's focus and caret while they type. (let [amount-attrs {:name (fc/field-name) :id (str "account-amount-" index) :class "w-16 account-amount-field" :value (fc/field-value) :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) - :hx-target "#wizard-form" - :hx-swap "morph" + ;; Typing an amount posts the whole form but swaps NOTHING into the field + ;; itself (hx-swap=none). Only the TOTAL and BALANCE cells are pulled out of + ;; the response and applied out-of-band, so the amount input is never replaced + ;; and the user's focus + caret survive with no morph involved. + :hx-select-oob "#total,#balance" + :hx-swap "none" :hx-trigger "keyup changed delay:300ms" :hx-include "closest form"}] (if (= "%" amount-mode) @@ -329,8 +331,10 @@ (com/data-grid-cell {:class "align-top"} (com/a-icon-button {:hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-wizard-remove-account) :hx-vals (hx/json {:row-index (or index 0)}) - :hx-target "#wizard-form" - :hx-swap "morph" + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML" :hx-include "closest form" :class "account-remove-action"} svg/x)))) @@ -471,8 +475,10 @@ :name "step-params[amount-mode]" :orientation :horizontal :hx-post (bidi/path-for ssr-routes/only-routes ::route/toggle-amount-mode) - :hx-target "#wizard-form" - :hx-swap "morph" + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML" :hx-include "closest form"})) (com/data-grid-header {:class "w-16"})]} (fc/cursor-map (fn [cursor] @@ -485,8 +491,10 @@ (com/data-grid-row {:class "new-row"} (com/data-grid-cell {:colspan 4} (com/a-button {:hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-wizard-new-account) - :hx-target "#wizard-form" - :hx-swap "morph" + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML" :hx-include "closest form" :color :secondary} "New account"))) @@ -501,7 +509,7 @@ (com/data-grid-row {:class "account-balance-row"} (com/data-grid-cell {}) (com/data-grid-cell {:class "text-right"} [:span.font-bold.text-right "BALANCE"]) - (com/data-grid-cell {:id "total" + (com/data-grid-cell {:id "balance" :class "text-right"} (account-balance* request)) (com/data-grid-cell {})) @@ -528,8 +536,10 @@ (com/hidden {:name "step-params[mode]" :value (name mode)}) [:div {:hx-trigger "change" :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-vendor-changed) - :hx-target "#wizard-form" - :hx-swap "morph" + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML" :hx-sync "this:replace" :hx-include "closest form"} (fc/with-field :transaction/vendor @@ -537,11 +547,6 @@ {:label "Vendor" :errors (fc/field-errors)} [:div.w-96 (com/typeahead {:name (fc/field-name) - ;; Key the vendor typeahead by its value so alpine-morph re-creates - ;; it on each vendor change. A morph-preserved typeahead loses its - ;; value $watch -> change dispatch, so a *second* vendor change would - ;; otherwise never fire its htmx request. - :id (fc/field-name) :error? (fc/error?) :class "w-96" :placeholder "Search..." @@ -551,13 +556,7 @@ (if (= mode :simple) (let [simple-account-id (let [av (-> (first all-accounts) :transaction-account/account)] (if (map? av) (:db/id av) av))] - ;; Key this wrapper by the account id so alpine-morph re-inits its x-data - ;; when the server changes the account (e.g. a vendor selection populating - ;; its default account). Without a changing key, morph keeps the stale - ;; simpleAccountId and the nested typeahead's x-model="simpleAccountId" - ;; binds back to the empty value. alpine-morph keys off `key`, not `id`. - [:div {:key (str "simple-account-wrapper--" simple-account-id) - :x-data (hx/json {:simpleAccountId simple-account-id})} + [:div {:x-data (hx/json {:simpleAccountId simple-account-id})} (simple-mode-fields* request)]) [:div (when (<= row-count 1) @@ -565,8 +564,10 @@ [:a.text-sm.text-blue-600.hover:underline.cursor-pointer {:hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-wizard-toggle-mode) :hx-include "closest form" - :hx-target "#wizard-form" - :hx-swap "morph"} + :hx-target "#manual-coding-section" + :hx-select "#manual-coding-section" + :hx-select-oob "#wizard-snapshot" + :hx-swap "outerHTML"} "Switch to simple mode"]]) (fc/with-field :transaction/accounts (com/validated-field @@ -878,17 +879,17 @@ {:label "Memo" :errors (fc/field-errors)} [:div.w-96 - ;; Memo edits re-render the whole form via morph. The stable id - ;; lets alpine-morph match this input across the swap so the - ;; caret stays put while the user types. (com/text-input {:value (-> (fc/field-value)) :name (fc/field-name) :id "edit-memo" :error? (fc/field-errors) :placeholder "Optional note" :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) - :hx-target "#wizard-form" - :hx-swap "morph" + ;; Memo has no dependent UI, so it posts only to keep the + ;; server snapshot in sync and swaps nothing back in. With + ;; hx-swap=none the input is never touched, so the caret + ;; stays put with no morph. + :hx-swap "none" :hx-trigger "keyup changed delay:300ms" :hx-include "closest form"})])) [:div {:x-data (hx/json {:activeForm (if (:transaction/payment (:entity request)) @@ -1372,7 +1373,7 @@ (-> mm/default-form-props (assoc :hx-post (str (bidi/path-for ssr-routes/only-routes ::route/edit-submit)) - :hx-ext "response-targets,alpine-morph")) + :hx-ext "response-targets")) :render-timeline? false)) (steps [_] [:links]) diff --git a/src/clj/auto_ap/ssr/ui.clj b/src/clj/auto_ap/ssr/ui.clj index ca099d10..19cabe50 100644 --- a/src/clj/auto_ap/ssr/ui.clj +++ b/src/clj/auto_ap/ssr/ui.clj @@ -39,7 +39,6 @@ [:link {:rel "stylesheet" :href "https://cdn.jsdelivr.net/npm/vanillajs-datepicker@1.3.4/dist/css/datepicker.min.css"}] [:script {:type "text/javascript" :src "https://cdn.jsdelivr.net/npm/vanillajs-datepicker@1.3.4/dist/js/datepicker-full.min.js"}] [:script {:src "https://unpkg.com/htmx.org/dist/ext/response-targets.js" :defer true}] - [:script {:src "https://unpkg.com/htmx.org@2.0.10/dist/ext/alpine-morph.js"}] [:script {:src "https://cdn.jsdelivr.net/npm/date-fns@3.6.0/cdn.min.js" :defer true}] [:script {:src "https://cdnjs.cloudflare.com/ajax/libs/Chart.js/4.4.1/chart.umd.min.js" :integrity "sha512-CQBWl4fJHWbryGE+Pc7UAxWMUMNMWzWxF4SQo9CgkJIN1kx6djDQZjh3Y8SZ1d+6I+1zze6Z7kHXO7q3UyZAWw==" :crossorigin "anonymous" :referrerpolicy "no-referrer"}] @@ -48,7 +47,6 @@ [:script {:defer true :src "/js/alpine-vals.js"}] [:script {:defer true :src "https://cdn.jsdelivr.net/npm/@ryangjchandler/alpine-clipboard@2.x.x/dist/alpine-clipboard.js"}] [:script {:defer true :src "https://cdn.jsdelivr.net/npm/@alpinejs/focus@3.x.x/dist/cdn.min.js"}] - [:script {:defer true :src "https://cdn.jsdelivr.net/npm/@alpinejs/morph@3.x.x/dist/cdn.min.js"}] [:script {:defer true :src "https://cdn.jsdelivr.net/npm/alpinejs@3.x.x/dist/cdn.min.js"}] [:script {:src "https://cdn.jsdelivr.net/npm/signature_pad@4.1.7/dist/signature_pad.umd.min.js"}] [:script {:src "https://cdn.jsdelivr.net/npm/jdenticon@3.3.0/dist/jdenticon.min.js" :async true :defer true :integrity "sha384-LfouGM03m83ArVtne1JPk926e3SGD0Tz8XHtW2OKGsgeBU/UfR0Fa8eX+UlwSSAZ" :crossorigin "anonymous"}]