diff --git a/src/component_framework/static/component_framework/js/component-client.js b/src/component_framework/static/component_framework/js/component-client.js index 862e5f2..e369344 100644 --- a/src/component_framework/static/component_framework/js/component-client.js +++ b/src/component_framework/static/component_framework/js/component-client.js @@ -281,7 +281,8 @@ class ComponentClient { * attribute too, since it was captured as part of that HTML string. * Morphing (rather than a full replace) means nodes unaffected by the * rollback keep their identity, including any already-bound event - * listeners. + * listeners, currently-focused input, and (via the scroll capture/restore + * below) scroll position. * * @param {string} componentId - ID of the component to roll back. */ @@ -292,7 +293,9 @@ class ComponentClient { const element = document.getElementById(componentId); if (!element) return; - Idiomorph.morph(element, snapshot.html, { morphStyle: 'outerHTML' }); + const scrollPositions = this._captureScrollPositions(element); + Idiomorph.morph(element, snapshot.html, this._morphConfig()); + this._restoreScrollPositions(element, scrollPositions); this.bind(element); this._snapshots.delete(componentId); @@ -303,11 +306,15 @@ class ComponentClient { * * Morphs the element (in place, via Idiomorph) to match the new HTML from * the server, instead of a full outerHTML replace — nodes unaffected by the - * update (and their attached listeners, focus, scroll position) keep their - * identity. Updates the `data-state` attribute so the next dispatch sends - * the correct state back, then re-binds declarative event handlers (a - * no-op for nodes idiomorph preserved, since their listeners already - * survived the morph; only newly-inserted nodes actually need it). + * update (and their attached listeners) keep their identity. Focus and + * in-flight input are preserved via the `ignoreActiveValue`/`restoreFocus` + * morph config (see `_morphConfig()`); scroll position — which idiomorph + * has no concept of — is preserved via explicit capture/restore around the + * morph call (see `_captureScrollPositions()`/`_restoreScrollPositions()`). + * Updates the `data-state` attribute so the next dispatch sends the + * correct state back, then re-binds declarative event handlers (a no-op + * for nodes idiomorph preserved, since their listeners already survived + * the morph; only newly-inserted nodes actually need it). * * @param {Element} element - The component root element to morph in place. * @param {string} html - New component HTML from the server. @@ -322,7 +329,9 @@ class ComponentClient { return; } - Idiomorph.morph(element, html, { morphStyle: 'outerHTML' }); + const scrollPositions = this._captureScrollPositions(element); + Idiomorph.morph(element, html, this._morphConfig()); + this._restoreScrollPositions(element, scrollPositions); // Persist state so subsequent dispatches send the correct value back. if (state !== null) { @@ -509,6 +518,91 @@ class ComponentClient { } } + /** + * Idiomorph config shared by `update()` and `rollback()` (B2, #22). + * + * `restoreFocus` already defaults to `true` upstream (see + * `vendor/idiomorph.js`): if the focused input/textarea gets recreated + * rather than patched in place, idiomorph re-finds it by `id` and restores + * focus + selection. That alone is *not* sufficient to preserve in-flight + * input, though — without `ignoreActiveValue`, idiomorph still overwrites + * the focused element's `value` with whatever the server rendered, + * clobbering keystrokes the user made while the request was in flight. + * `ignoreActiveValue: true` skips only that value sync for the + * currently-focused element; every other attribute/child still morphs + * normally, so server-driven changes (e.g. a validation error class) still + * apply around the input the user is actively editing. + * + * @returns {{morphStyle: string, ignoreActiveValue: boolean}} + */ + _morphConfig() { + return { morphStyle: 'outerHTML', ignoreActiveValue: true }; + } + + /** + * Capture scroll offsets that would otherwise be lost when idiomorph + * recreates a scrollable element instead of patching it in place. + * Idiomorph has no concept of scroll position at all — this is bespoke + * bookkeeping this framework owns around `Idiomorph.morph()`, not morph + * configuration. + * + * Only the root element and descendants carrying a stable `id` can be + * re-located after the morph (mirrors idiomorph's own `restoreFocus`, + * which likewise needs an `id` to re-find a recreated focused element), and + * only non-zero offsets are recorded, to keep the common case a no-op. + * + * @param {Element} element - The component root element about to be morphed. + * @returns {Array<{id: string|null, isRoot: boolean, top: number, left: number}>} + * Captured offsets to pass to `_restoreScrollPositions()`. + */ + _captureScrollPositions(element) { + const positions = []; + + const record = (el, isRoot) => { + const top = el.scrollTop || 0; + const left = el.scrollLeft || 0; + if (!top && !left) return; + positions.push({ id: isRoot ? null : el.id || null, isRoot, top, left }); + }; + + record(element, true); + if (typeof element.querySelectorAll === 'function') { + for (const el of element.querySelectorAll('[id]')) { + record(el, false); + } + } + + return positions; + } + + /** + * Restore scroll offsets captured by `_captureScrollPositions()` after a + * morph. The root element keeps its identity across an outerHTML morph + * (see `update()`/`rollback()`), so it is restored via the direct + * reference; descendants are re-located by `id`, since idiomorph may have + * recreated rather than patched them. Descendants with no `id` cannot be + * re-located and are silently skipped — a documented limitation shared + * with idiomorph's own focus restoration. + * + * @param {Element} element - The component root element that was morphed. + * @param {Array<{id: string|null, isRoot: boolean, top: number, left: number}>} positions + * Offsets returned by `_captureScrollPositions()`. + */ + _restoreScrollPositions(element, positions) { + for (const pos of positions) { + let target = null; + if (pos.isRoot) { + target = element; + } else if (pos.id && typeof element.querySelector === 'function') { + target = element.querySelector(`[id="${pos.id}"]`); + } + if (target) { + target.scrollTop = pos.top; + target.scrollLeft = pos.left; + } + } + } + /** * Save a DOM snapshot for `componentId` before any mutation. * diff --git a/tests/js/morph-preservation.test.mjs b/tests/js/morph-preservation.test.mjs new file mode 100644 index 0000000..9be23fa --- /dev/null +++ b/tests/js/morph-preservation.test.mjs @@ -0,0 +1,293 @@ +/** + * Node-native tests for focus / in-flight-input / scroll preservation across + * morph patches (Epic B, B2 — #22). + * + * Run with: node --test "tests/js/**\/*.test.mjs" + * (`just test-js` has a pre-existing cmd.exe glob-expansion bug on Windows — + * documented as out of scope in PR #43 — so invoke node directly.) + * + * Split into two concerns, tested at two different levels, matching this + * repo's no-jsdom, hand-stub philosophy (see morph.test.mjs's header): + * + * 1. Focus / in-flight input value: idiomorph itself already implements + * this (restoreFocus defaults to true; ignoreActiveValue protects the + * focused element's value). We can't exercise idiomorph's internal + * browser-API-dependent logic (document.activeElement, + * HTMLInputElement, etc.) without jsdom, so — like morph.test.mjs — + * these tests only verify the *integration seam*: that + * update()/rollback() actually pass `ignoreActiveValue: true` in the + * config handed to Idiomorph.morph(). Without that flag (confirmed by + * reading vendor/idiomorph.js's ignoreAttribute()/syncInputValue() + * logic around line 751-761 and 834-846), idiomorph overwrites a + * focused input's `value` with the server's render even though + * restoreFocus keeps the element focused — i.e. focus survives by + * default, but the user's in-flight keystrokes don't, unless this flag + * is set. + * + * 2. Scroll position: idiomorph has no concept of scroll at all (pure DOM + * patching), so this is bespoke bookkeeping this repo owns outright, + * not idiomorph configuration. These tests exercise the real capture/ + * restore logic end to end against hand-rolled DOM stubs. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +// --- Minimal DOM stubs (installed before importing the module) ------------ + +function makeEl(dataset = {}, extra = {}) { + return { + dataset, + id: extra.id ?? 'c1', + tagName: extra.tagName ?? 'DIV', + outerHTML: extra.outerHTML ?? '
', + scrollTop: extra.scrollTop ?? 0, + scrollLeft: extra.scrollLeft ?? 0, + _attrs: {}, + _replacedWith: null, + setAttribute(k, v) { + this._attrs[k] = v; + }, + getAttribute(k) { + return k in this._attrs ? this._attrs[k] : null; + }, + getAttributeNames() { + return Object.keys(this._attrs); + }, + removeAttribute(k) { + delete this._attrs[k]; + }, + replaceWith(n) { + this._replacedWith = n; + }, + closest() { + return null; + }, + querySelectorAll() { + return []; + }, + querySelector() { + return null; + }, + matches() { + return false; + }, + }; +} + +const registry = {}; +globalThis.document = { + querySelector: () => null, + cookie: '', + getElementById: (id) => registry[id] ?? null, + addEventListener: () => {}, + createElement: () => makeEl({}), +}; + +const { ComponentClient } = await import( + '../../src/component_framework/static/component_framework/js/component-client.js' +); +const { Idiomorph } = await import( + '../../src/component_framework/static/component_framework/js/vendor/idiomorph.js' +); + +// --- Focus / in-flight input value (integration seam) ---------------------- + +test('update() passes ignoreActiveValue: true so a focused input\'s in-flight text is not clobbered', () => { + const client = new ComponentClient(); + const el = makeEl({ state: '{"count":1}' }); + registry.c1 = el; + + let morphArgs = null; + const originalMorph = Idiomorph.morph; + Idiomorph.morph = (...args) => { + morphArgs = args; + }; + try { + client.update(el, '
2
', '{"count":2}'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.ok(morphArgs, 'Idiomorph.morph should have been called'); + assert.equal( + morphArgs[2].ignoreActiveValue, + true, + 'update() must protect the focused element\'s value across the morph', + ); + delete registry.c1; +}); + +test('rollback() passes ignoreActiveValue: true so a focused input\'s in-flight text is not clobbered', () => { + const client = new ComponentClient(); + const el = makeEl( + { state: '{"count":2}' }, + { outerHTML: '
1
' }, + ); + registry.c1 = el; + client._snapshot(el, 'c1'); + + let morphArgs = null; + const originalMorph = Idiomorph.morph; + Idiomorph.morph = (...args) => { + morphArgs = args; + }; + try { + client.rollback('c1'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.ok(morphArgs, 'Idiomorph.morph should have been called'); + assert.equal( + morphArgs[2].ignoreActiveValue, + true, + 'rollback() must protect the focused element\'s value across the morph', + ); + delete registry.c1; +}); + +// --- Scroll position (bespoke, not idiomorph) ------------------------------- + +test('update() restores the root element\'s own scroll offsets when the morph resets them', () => { + const client = new ComponentClient(); + const el = makeEl({ state: '{"count":1}' }, { scrollTop: 120, scrollLeft: 30 }); + registry.c1 = el; + + const originalMorph = Idiomorph.morph; + Idiomorph.morph = () => { + // Simulate idiomorph patching the element in a way that resets scroll. + el.scrollTop = 0; + el.scrollLeft = 0; + }; + try { + client.update(el, '
2
', '{"count":2}'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.equal(el.scrollTop, 120); + assert.equal(el.scrollLeft, 30); + delete registry.c1; +}); + +test('update() restores a scrollable descendant\'s scroll offset even when idiomorph recreates that node', () => { + const client = new ComponentClient(); + + const oldList = makeEl({}, { id: 'msg-list', tagName: 'DIV', scrollTop: 250 }); + const newList = makeEl({}, { id: 'msg-list', tagName: 'DIV', scrollTop: 0 }); + + const el = makeEl({ state: '{"count":1}' }); + el.querySelectorAll = (selector) => (selector === '[id]' ? [oldList] : []); + el.querySelector = (selector) => (selector === '[id="msg-list"]' ? newList : null); + registry.c1 = el; + + const originalMorph = Idiomorph.morph; + // Idiomorph is stubbed out entirely here; `newList` already stands in for + // whatever node idiomorph would have produced, wired up via the + // querySelector stub above so restore can re-locate it by id. + Idiomorph.morph = () => {}; + try { + client.update(el, '
', '{"count":2}'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.equal(newList.scrollTop, 250, 'recreated descendant should have its scroll offset restored'); + assert.equal(oldList.scrollTop, 250, 'captured snapshot should reflect the pre-morph offset'); + delete registry.c1; +}); + +test('update() does not restore scroll for a descendant with no id (cannot be re-located, mirrors idiomorph\'s own restoreFocus limitation)', () => { + const client = new ComponentClient(); + + // No `id` set -> selector '[id]' would not match this in a real DOM; the + // stub simulates that filtering by only returning matches for '[id]'. + const noIdDescendant = makeEl({}, { id: '', tagName: 'DIV', scrollTop: 80 }); + + const el = makeEl({ state: '{"count":1}' }); + el.querySelectorAll = () => []; + registry.c1 = el; + + const originalMorph = Idiomorph.morph; + Idiomorph.morph = () => { + noIdDescendant.scrollTop = 0; + }; + try { + client.update(el, '
', '{"count":2}'); + } finally { + Idiomorph.morph = originalMorph; + } + + // Nothing should have thrown, and there is no mechanism to restore an + // unidentified node's scroll position -- documented limitation. + assert.equal(noIdDescendant.scrollTop, 0); + delete registry.c1; +}); + +test('update() does not capture or restore scroll when nothing is scrolled (no-op fast path)', () => { + const client = new ComponentClient(); + const el = makeEl({ state: '{"count":1}' }); + registry.c1 = el; + + const originalMorph = Idiomorph.morph; + let morphCalled = false; + Idiomorph.morph = () => { + morphCalled = true; + }; + try { + client.update(el, '
2
', '{"count":2}'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.equal(morphCalled, true); + assert.equal(el.scrollTop, 0); + assert.equal(el.scrollLeft, 0); + delete registry.c1; +}); + +test('rollback() restores the root element\'s own scroll offsets when the morph resets them', () => { + const client = new ComponentClient(); + const el = makeEl( + { state: '{"count":2}' }, + { outerHTML: '
1
', scrollTop: 60 }, + ); + registry.c1 = el; + client._snapshot(el, 'c1'); + + const originalMorph = Idiomorph.morph; + Idiomorph.morph = () => { + el.scrollTop = 0; + }; + try { + client.rollback('c1'); + } finally { + Idiomorph.morph = originalMorph; + } + + assert.equal(el.scrollTop, 60); + delete registry.c1; +}); + +test('_captureScrollPositions ignores elements with zero scroll offsets', () => { + const client = new ComponentClient(); + const el = makeEl({}, { scrollTop: 0, scrollLeft: 0 }); + + const positions = client._captureScrollPositions(el); + + assert.deepEqual(positions, []); +}); + +test('_captureScrollPositions records the root plus any scrolled descendants with an id', () => { + const client = new ComponentClient(); + const descendant = makeEl({}, { id: 'panel', scrollTop: 40, scrollLeft: 5 }); + const el = makeEl({}, { scrollTop: 100, scrollLeft: 0 }); + el.querySelectorAll = (selector) => (selector === '[id]' ? [descendant] : []); + + const positions = client._captureScrollPositions(el); + + assert.equal(positions.length, 2); + assert.ok(positions.some((p) => p.isRoot && p.top === 100 && p.left === 0)); + assert.ok(positions.some((p) => !p.isRoot && p.id === 'panel' && p.top === 40 && p.left === 5)); +});