From e8517307ce416ef01a890fc652b34025f1cfab5d Mon Sep 17 00:00:00 2001 From: Adnaan Badr Date: Sat, 20 Jun 2026 07:03:36 +0000 Subject: [PATCH 1/2] fix(morphdom): make lvt-on:click checkboxes/radios server-authoritative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A checkbox or radio whose toggle is routed to the server via lvt-on:click is server-authoritative: the click is dispatched and the server echoes back the new checked state in its re-render. The morphdom pass, however, defaults to "user selection wins" — it copies the live control's checked state onto the incoming element (toEl.checked = fromEl.checked) so scan-loop refreshes don't clobber pending user input. For an lvt-on:click control that's backwards: when the live state is stale relative to the server's authoritative value, the server's toggle never reflects in the DOM and the box appears stuck. Extend the server-authoritative predicate in onBeforeElUpdated to treat an lvt-on:click checkbox/radio like data-lvt-force-update — the server's rendered checked attribute wins. data-lvt-force-update remains the explicit one-shot override (and is still stripped after applying); lvt-on:click is a durable handler and is left in place, so the control stays authoritative on every update. This removes the need to sprinkle data-lvt-force-update on every server-driven toggle checkbox (lvt's toggle component, datatable row/select-all, tinkerdown auto-tasks, etc. all use this pattern without it and were latently affected). Tests: handler ⇒ server wins; handler-less ⇒ user state still preserved (default unchanged); explicit data-lvt-force-update still overrides. Full suite green (737). --- livetemplate-client.ts | 28 ++++++++++++++++++++------- tests/preserve.test.ts | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/livetemplate-client.ts b/livetemplate-client.ts index 1639a80..7cc4a3c 100644 --- a/livetemplate-client.ts +++ b/livetemplate-client.ts @@ -1823,22 +1823,36 @@ export class LiveTemplateClient { // state is user input that must survive scan-loop refreshes. Use // data-lvt-force-update to let the server override the user state. // - // Known limitation: force-update on one radio can uncheck a sibling - // that was already processed earlier in the same morphdom pass, since - // browser mutual exclusion fires synchronously mid-loop. To safely - // reset a radio group, send data-lvt-force-update on ALL radios in - // the group, not just the one being checked. + // Exception: a control with an lvt-on:click handler routes its own + // toggle to the server, which echoes back the authoritative state. + // There is no pending local-only selection to protect, so the server's + // rendered checked attribute must win — otherwise a server-driven + // toggle never reflects in the DOM. This makes lvt-on:click checkboxes + // server-authoritative without needing an explicit data-lvt-force-update. + // + // Known limitation: forcing one radio can uncheck a sibling that was + // already processed earlier in the same morphdom pass, since browser + // mutual exclusion fires synchronously mid-loop. To safely reset a + // radio group, make the server send the authoritative state (or + // data-lvt-force-update) on ALL radios in the group, not just one. if ( fromEl instanceof HTMLInputElement && toEl instanceof HTMLInputElement && (fromEl.type === "checkbox" || fromEl.type === "radio") ) { - if (toEl.hasAttribute("data-lvt-force-update")) { + const explicitForce = toEl.hasAttribute("data-lvt-force-update"); + const serverAuthoritative = + explicitForce || toEl.hasAttribute("lvt-on:click"); + if (serverAuthoritative) { fromEl.checked = toEl.checked; if (fromEl.type === "checkbox") { fromEl.indeterminate = toEl.indeterminate; } - fromEl.removeAttribute("data-lvt-force-update"); + // data-lvt-force-update is a one-shot signal; strip it so it does + // not persist. lvt-on:click is a durable handler — leave it. + if (explicitForce) { + fromEl.removeAttribute("data-lvt-force-update"); + } } else { toEl.checked = fromEl.checked; // Align the checked attribute with the property so morphdom's diff --git a/tests/preserve.test.ts b/tests/preserve.test.ts index 8f0e18a..440b014 100644 --- a/tests/preserve.test.ts +++ b/tests/preserve.test.ts @@ -231,6 +231,50 @@ describe("lvt-ignore and lvt-ignore-attrs", () => { expect(afterUpdate[1].checked).toBe(false); }); + it("server wins for a checkbox with an lvt-on:click handler", () => { + // A checkbox whose toggle is routed to the server via lvt-on:click is + // server-authoritative: the click is dispatched and the server echoes back + // the new state. At morph time the live control's checked is still false + // (the toggle is owned by the server, not local optimistic state), and the + // server's re-render carries the checked attribute. Server must win without + // needing an explicit data-lvt-force-update — issue tinkerdown#292. + const uncheckedTree = { + s: [`
`, `
`], + 0: ``, + }; + client.updateDOM(wrapper, uncheckedTree); + + const cb = wrapper.querySelector('input[type="checkbox"]')!; + expect(cb.checked).toBe(false); + + // Server responds to the click with the toggled (checked) state. + const checkedTree = { + 0: ``, + }; + client.updateDOM(wrapper, checkedTree); + + const cbAfter = wrapper.querySelector('input[type="checkbox"]')!; + expect(cbAfter.checked).toBe(true); + }); + + it("still preserves user state for a checkbox with NO server-bound handler", () => { + // Control: a plain form checkbox (no lvt-on:click) keeps the user's + // selection across a server refresh — the default must not regress. + const tree = { + s: [`
`, `
`], + 0: ``, + }; + client.updateDOM(wrapper, tree); + + const cb = wrapper.querySelector('input[type="checkbox"]')!; + cb.checked = true; // user checks it + + client.updateDOM(wrapper, tree); // server refresh, no checked attribute + + const cbAfter = wrapper.querySelector('input[type="checkbox"]')!; + expect(cbAfter.checked).toBe(true); + }); + it("preserves radio button checked state across morphdom updates", () => { const initialTree = { s: [ From 606b4f44a6ecd02322b692b034814a522669303b Mon Sep 17 00:00:00 2001 From: Adnaan Badr Date: Sat, 20 Jun 2026 07:23:59 +0000 Subject: [PATCH 2/2] test(morphdom): cover radio + inverse cases for server-authoritative checkboxes Addresses review feedback on PR #135: - add a server-authoritative radio test (the predicate covers radios too) - the checkbox test now also asserts the inverse direction (server re-affirms checked over a stale user-unchecked state) using a co-changing sibling slot so each push is a real morphdom pass, not a short-circuited no-op --- tests/preserve.test.ts | 50 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/tests/preserve.test.ts b/tests/preserve.test.ts index 440b014..4f92752 100644 --- a/tests/preserve.test.ts +++ b/tests/preserve.test.ts @@ -238,9 +238,12 @@ describe("lvt-ignore and lvt-ignore-attrs", () => { // (the toggle is owned by the server, not local optimistic state), and the // server's re-render carries the checked attribute. Server must win without // needing an explicit data-lvt-force-update — issue tinkerdown#292. + // Slot 1 is a co-rendered label that changes each push, so every update is + // a real morphdom pass (an identical tree short-circuits in updateDOM). const uncheckedTree = { - s: [`
`, `
`], + s: [`
`, ``, `
`], 0: ``, + 1: `v1`, }; client.updateDOM(wrapper, uncheckedTree); @@ -248,13 +251,52 @@ describe("lvt-ignore and lvt-ignore-attrs", () => { expect(cb.checked).toBe(false); // Server responds to the click with the toggled (checked) state. - const checkedTree = { + client.updateDOM(wrapper, { 0: ``, + 1: `v2`, + }); + expect( + wrapper.querySelector('input[type="checkbox"]')!.checked + ).toBe(true); + + // Inverse: the user optimistically unchecks it (live state now stale), but + // the server's next render re-affirms checked. Server authority must win in + // both directions — the local control never overrides the server value. + wrapper.querySelector('input[type="checkbox"]')!.checked = false; + client.updateDOM(wrapper, { + 0: ``, + 1: `v3`, + }); + expect( + wrapper.querySelector('input[type="checkbox"]')!.checked + ).toBe(true); + }); + + it("server wins for a radio with an lvt-on:click handler", () => { + // The server-authoritative path also applies to radios. When every radio in + // the group is server-bound, the server sends authoritative state for ALL of + // them (here: r1 unchecked, r2 checked), which sidesteps the documented + // mid-pass mutual-exclusion limitation. + const uncheckedTree = { + s: [`
`, `
`], + 0: `` + + ``, + }; + client.updateDOM(wrapper, uncheckedTree); + + const radios = wrapper.querySelectorAll('input[type="radio"]'); + expect(radios[0].checked).toBe(false); + expect(radios[1].checked).toBe(false); + + const checkedTree = { + 0: `` + + ``, }; client.updateDOM(wrapper, checkedTree); - const cbAfter = wrapper.querySelector('input[type="checkbox"]')!; - expect(cbAfter.checked).toBe(true); + const after = wrapper.querySelectorAll('input[type="radio"]'); + expect(after[0].checked).toBe(false); + expect(after[1].checked).toBe(true); }); it("still preserves user state for a checkbox with NO server-bound handler", () => {