From 87d88d69523c46195982a2521128d0c67356ce86 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 8 Sep 2026 18:07:47 +0300 Subject: [PATCH] fix(chat): prevent persistent gaps after activity collapse Upgrade LegendList to 3.3.10 and patch temporary padding cleanup to compare CSSOM-serialized values. Settle Activity height when React replays an interrupted layout effect. Validated with 16 padding regressions, 3 collapse lifecycle tests, the UI suite, workspace type-check and lint, and web and VS Code builds. --- bun-patches/@legendapp%2Flist@3.3.10.patch | 68 ++++++++++ bun.lock | 5 +- package.json | 3 +- packages/ui/package.json | 2 +- .../components/LiveActivityCollapse.test.tsx | 118 ++++++++++++++++++ .../chat/components/LiveActivityCollapse.tsx | 29 +++-- .../chat/message/parts/DOCUMENTATION.md | 15 +++ scripts/legend-list-padding.test.mjs | 116 +++++++++++++++++ 8 files changed, 342 insertions(+), 14 deletions(-) create mode 100644 bun-patches/@legendapp%2Flist@3.3.10.patch create mode 100644 packages/ui/src/components/chat/components/LiveActivityCollapse.test.tsx create mode 100644 scripts/legend-list-padding.test.mjs diff --git a/bun-patches/@legendapp%2Flist@3.3.10.patch b/bun-patches/@legendapp%2Flist@3.3.10.patch new file mode 100644 index 00000000..7d571a24 --- /dev/null +++ b/bun-patches/@legendapp%2Flist@3.3.10.patch @@ -0,0 +1,68 @@ +diff --git a/react-native.web.js b/react-native.web.js +index 5b171ebd6fa7ac86b0148fe17f897a0ca821d55e..954235cd2b970e0dbb6b78f650633e4f07242558 100644 +--- a/react-native.web.js ++++ b/react-native.web.js +@@ -6560,8 +6560,11 @@ function ScrollAdjust() { + window.getComputedStyle(contentNode)[axis.paddingEndProp] + ); + const temporaryPaddingEnd = `${(currentPaddingEnd || 0) + pad}px`; +- temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: temporaryPaddingEnd }; + contentNode.style[axis.paddingEndProp] = temporaryPaddingEnd; ++ // CSSOM rounds fractional pixels (2123.1875px becomes 2123.19px). ++ // Track the serialized value so ownership checks and cleanup ++ // match what the browser actually stored, not the input string. ++ temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: contentNode.style[axis.paddingEndProp] }; + void contentNode.offsetHeight; + scrollBy(); + if (resetPaddingRafRef.current !== void 0) { +diff --git a/react-native.web.mjs b/react-native.web.mjs +index cf549ca32946a9e35a883290c11423330bf8728b..d6508ee3b88e2f79c5ee9b994f6f93b6654e708c 100644 +--- a/react-native.web.mjs ++++ b/react-native.web.mjs +@@ -6539,8 +6539,11 @@ function ScrollAdjust() { + window.getComputedStyle(contentNode)[axis.paddingEndProp] + ); + const temporaryPaddingEnd = `${(currentPaddingEnd || 0) + pad}px`; +- temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: temporaryPaddingEnd }; + contentNode.style[axis.paddingEndProp] = temporaryPaddingEnd; ++ // CSSOM rounds fractional pixels (2123.1875px becomes 2123.19px). ++ // Track the serialized value so ownership checks and cleanup ++ // match what the browser actually stored, not the input string. ++ temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: contentNode.style[axis.paddingEndProp] }; + void contentNode.offsetHeight; + scrollBy(); + if (resetPaddingRafRef.current !== void 0) { +diff --git a/react.js b/react.js +index 5b171ebd6fa7ac86b0148fe17f897a0ca821d55e..954235cd2b970e0dbb6b78f650633e4f07242558 100644 +--- a/react.js ++++ b/react.js +@@ -6560,8 +6560,11 @@ function ScrollAdjust() { + window.getComputedStyle(contentNode)[axis.paddingEndProp] + ); + const temporaryPaddingEnd = `${(currentPaddingEnd || 0) + pad}px`; +- temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: temporaryPaddingEnd }; + contentNode.style[axis.paddingEndProp] = temporaryPaddingEnd; ++ // CSSOM rounds fractional pixels (2123.1875px becomes 2123.19px). ++ // Track the serialized value so ownership checks and cleanup ++ // match what the browser actually stored, not the input string. ++ temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: contentNode.style[axis.paddingEndProp] }; + void contentNode.offsetHeight; + scrollBy(); + if (resetPaddingRafRef.current !== void 0) { +diff --git a/react.mjs b/react.mjs +index cf549ca32946a9e35a883290c11423330bf8728b..d6508ee3b88e2f79c5ee9b994f6f93b6654e708c 100644 +--- a/react.mjs ++++ b/react.mjs +@@ -6539,8 +6539,11 @@ function ScrollAdjust() { + window.getComputedStyle(contentNode)[axis.paddingEndProp] + ); + const temporaryPaddingEnd = `${(currentPaddingEnd || 0) + pad}px`; +- temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: temporaryPaddingEnd }; + contentNode.style[axis.paddingEndProp] = temporaryPaddingEnd; ++ // CSSOM rounds fractional pixels (2123.1875px becomes 2123.19px). ++ // Track the serialized value so ownership checks and cleanup ++ // match what the browser actually stored, not the input string. ++ temporaryPaddingRef.current = { baseline: baselinePaddingEnd, value: contentNode.style[axis.paddingEndProp] }; + void contentNode.offsetHeight; + scrollBy(); + if (resetPaddingRafRef.current !== void 0) { diff --git a/bun.lock b/bun.lock index e2b18c35..db20dee6 100644 --- a/bun.lock +++ b/bun.lock @@ -166,7 +166,7 @@ "@dnd-kit/core": "^6.3.1", "@dnd-kit/sortable": "^10.0.0", "@dnd-kit/utilities": "^3.2.2", - "@legendapp/list": "3.3.8", + "@legendapp/list": "3.3.10", "@lezer/highlight": "^1.2.3", "@opencode-ai/sdk": "1.18.29", "@pierre/diffs": "1.3.0-beta.6", @@ -350,6 +350,7 @@ ], "patchedDependencies": { "@tanstack/virtual-core@3.17.3": "bun-patches/@tanstack+virtual-core+3.17.3.patch", + "@legendapp/list@3.3.10": "bun-patches/@legendapp%2Flist@3.3.10.patch", "bun-pty@0.4.8": "bun-patches/bun-pty@0.4.8.patch", }, "overrides": { @@ -917,7 +918,7 @@ "@kwsites/promise-deferred": ["@kwsites/promise-deferred@1.1.1", "", {}, "sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw=="], - "@legendapp/list": ["@legendapp/list@3.3.8", "", { "dependencies": { "use-sync-external-store": "^1.5.0" }, "peerDependencies": { "react": "*", "react-dom": "*", "react-native": "*" }, "optionalPeers": ["react-dom", "react-native"] }, "sha512-GM4Hca/6WDvcY33XXCieR9MaG9CoZmACzwqQwRhKFSaNKfQV1lTLiTOpWuA9wnE8n8+6WeA52DwNKC9yrnPPeg=="], + "@legendapp/list": ["@legendapp/list@3.3.10", "", { "dependencies": { "use-sync-external-store": "^1.5.0" }, "peerDependencies": { "react": "*", "react-dom": "*", "react-native": "*" }, "optionalPeers": ["react-dom", "react-native"] }, "sha512-S8cwV11oJJD2m47JoRnCmlgbUrnY/f48TPL1zl1C3wwyrvru53Zf1NtYFlMtAU6lUKh/cU9RIffLI7unbf3kmg=="], "@levischuck/tiny-cbor": ["@levischuck/tiny-cbor@0.2.11", "", {}, "sha512-llBRm4dT4Z89aRsm6u2oEZ8tfwL/2l6BwpZ7JcyieouniDECM5AqNgr/y08zalEIvW3RSK4upYyybDcmjXqAow=="], diff --git a/package.json b/package.json index a28116b4..4d9b3b78 100644 --- a/package.json +++ b/package.json @@ -198,6 +198,7 @@ }, "patchedDependencies": { "@tanstack/virtual-core@3.17.3": "bun-patches/@tanstack+virtual-core+3.17.3.patch", - "bun-pty@0.4.8": "bun-patches/bun-pty@0.4.8.patch" + "bun-pty@0.4.8": "bun-patches/bun-pty@0.4.8.patch", + "@legendapp/list@3.3.10": "bun-patches/@legendapp%2Flist@3.3.10.patch" } } diff --git a/packages/ui/package.json b/packages/ui/package.json index 76561272..66aed936 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -44,7 +44,7 @@ "@dnd-kit/core": "^6.3.1", "@dnd-kit/sortable": "^10.0.0", "@dnd-kit/utilities": "^3.2.2", - "@legendapp/list": "3.3.8", + "@legendapp/list": "3.3.10", "@lezer/highlight": "^1.2.3", "@opencode-ai/sdk": "1.18.29", "@pierre/diffs": "1.3.0-beta.6", diff --git a/packages/ui/src/components/chat/components/LiveActivityCollapse.test.tsx b/packages/ui/src/components/chat/components/LiveActivityCollapse.test.tsx new file mode 100644 index 00000000..158114a2 --- /dev/null +++ b/packages/ui/src/components/chat/components/LiveActivityCollapse.test.tsx @@ -0,0 +1,118 @@ +import React, { act, StrictMode, Suspense } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { Window } from 'happy-dom'; +import { LiveActivityCollapse } from './LiveActivityCollapse'; + +describe('live Activity collapse layout lifecycle', () => { + let root: Root; + let container: HTMLDivElement; + let restore: () => void; + + beforeEach(() => { + const win = new Window({ url: 'http://localhost' }); + const globals = { + window: win, document: win.document, HTMLElement: win.HTMLElement, + Element: win.Element, SVGElement: win.SVGElement, NodeList: win.NodeList, + requestAnimationFrame: win.requestAnimationFrame.bind(win), + cancelAnimationFrame: win.cancelAnimationFrame.bind(win), + getComputedStyle: win.getComputedStyle.bind(win), + IS_REACT_ACT_ENVIRONMENT: true, + }; + const previous = Object.keys(globals).map((name) => [name, Object.getOwnPropertyDescriptor(globalThis, name)] as const); + for (const [name, value] of Object.entries(globals)) { + Object.defineProperty(globalThis, name, { value, configurable: true, writable: true }); + } + restore = () => { + for (const [name, descriptor] of previous) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor); + else Reflect.deleteProperty(globalThis, name); + } + }; + container = document.createElement('div'); + document.body.append(container); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + restore(); + }); + + test('settles the target height when React cleans up and replays a collapse layout effect', async () => { + await act(async () => root.render( + + +
{ + if (!node?.parentElement) return; + // Happy DOM has no layout engine. Supply the height + // measured before a real historical turn collapses. + Object.defineProperty(node.parentElement, 'scrollHeight', { configurable: true, get: () => 2400 }); + }}>Historical activity
+ + , + )); + const region = container.querySelector('[data-live-activity-content]'); + expect(region?.style.height).toBe('0px'); + expect(region?.childElementCount).toBe(0); + }); + + test('settles a collapse after a Suspense hide/reveal in production lifecycle', async () => { + let suspended = false; + let release: () => void = () => undefined; + const pending = new Promise((resolve) => { release = resolve; }); + function LoadingSibling() { + if (suspended) throw pending; + return null; + } + const render = () => ( + Loading history}> + +
{ + if (!node?.parentElement) return; + Object.defineProperty(node.parentElement, 'scrollHeight', { configurable: true, get: () => 2400 }); + }}>Historical activity
+ + + + ); + await act(async () => root.render(render())); + suspended = true; + await act(async () => root.render(render())); + suspended = false; + await act(async () => { release(); }); + const region = container.querySelector('[data-live-activity-content]'); + expect(region?.style.height).toBe('0px'); + expect(region?.childElementCount).toBe(0); + }); + + test('restores natural height when an expansion is interrupted by Suspense', async () => { + let expanded = false; + let suspended = false; + let release: () => void = () => undefined; + const pending = new Promise((resolve) => { release = resolve; }); + function LoadingSibling() { + if (suspended) throw pending; + return null; + } + const render = () => ( + Loading history}> + +
Historical activity
+ + + + ); + await act(async () => root.render(render())); + expanded = true; + await act(async () => root.render(render())); + suspended = true; + await act(async () => root.render(render())); + suspended = false; + await act(async () => { release(); }); + const region = container.querySelector('[data-live-activity-content]'); + expect(region?.style.height).toBe('auto'); + expect(region?.style.overflow).toBe('visible'); + expect(region?.textContent).toBe('Historical activity'); + }); +}); diff --git a/packages/ui/src/components/chat/components/LiveActivityCollapse.tsx b/packages/ui/src/components/chat/components/LiveActivityCollapse.tsx index ba79de72..179caa25 100644 --- a/packages/ui/src/components/chat/components/LiveActivityCollapse.tsx +++ b/packages/ui/src/components/chat/components/LiveActivityCollapse.tsx @@ -16,13 +16,24 @@ export function LiveActivityCollapse({ expanded, children, id, animateOnMount = React.useLayoutEffect(() => { const element = ref.current; - if (!element || previousExpanded.current === expanded) return; - previousExpanded.current = expanded; - if (expanded) setRetained(true); - if (window.matchMedia('(prefers-reduced-motion: reduce)').matches) { + if (!element) return; + const settle = () => { element.style.height = expanded ? 'auto' : '0px'; element.style.overflow = expanded ? 'visible' : 'hidden'; setRetained(expanded); + }; + // Suspense can clean up a layout effect while retaining its DOM, then + // replay setup on reveal. The old animation was stopped, but the ref + // still records its target. Skipping setup here would freeze the + // measured pre-collapse height and retain an empty historical region. + if (previousExpanded.current === expanded) { + settle(); + return; + } + previousExpanded.current = expanded; + if (expanded) setRetained(true); + if (window.matchMedia('(prefers-reduced-motion: reduce)').matches) { + settle(); return; } element.style.height = expanded ? '0px' : `${element.scrollHeight}px`; @@ -32,12 +43,10 @@ export function LiveActivityCollapse({ expanded, children, id, animateOnMount = ease: [0.16, 1, 0.3, 1], }); let cancelled = false; - void animation.finished.then(() => { - if (cancelled) return; - element.style.height = expanded ? 'auto' : '0px'; - element.style.overflow = expanded ? 'visible' : 'hidden'; - setRetained(expanded); - }).catch(() => undefined); + const finish = () => { + if (!cancelled) settle(); + }; + void animation.finished.then(finish, finish); return () => { cancelled = true; animation.stop(); diff --git a/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md b/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md index 84cca7c2..b9a5393e 100644 --- a/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md +++ b/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md @@ -82,6 +82,21 @@ within the session. The disclosure uses a finite 180ms height transition, respects reduced motion, and delegates end pinning to the existing timeline. It never calls scroll-to-bottom. Collapsed history does not mount its hidden message bodies; initial history loads do not animate collapse. +Layout-effect replay after a Suspense hide/reveal must settle the requested +height and retained children even when the expanded target did not change. +Cleanup stops the animation, so a same-target early return can leave a cached +pre-collapse height on the DOM indefinitely. Failed animations also settle; +callbacks from cancelled, superseded animations never settle a newer target. + +The virtualizer also adds temporary end padding while compensating prepended +history. The Bun patch for `@legendapp/list@3.3.10` stores that padding's CSSOM +read-back value: Chromium rounds fractional pixel strings, so comparing the +original input with `style.paddingBottom` can skip cleanup permanently. This +leaves a phantom tail even when every Activity region is already zero-height. +The patch covers both web entry points in ESM and CJS; its installed-controller +regression tests live in `scripts/legend-list-padding.test.mjs`. Retain this +fix when updating the dependency unless upstream has equivalent ownership and +cleanup behavior. Chat padding and scroll policies do not compensate for it. The header retains its report when expanded and has no hover background. Its left inset matches sorted Activity. Diff deletions use the ASCII hyphen. diff --git a/scripts/legend-list-padding.test.mjs b/scripts/legend-list-padding.test.mjs new file mode 100644 index 00000000..f074a421 --- /dev/null +++ b/scripts/legend-list-padding.test.mjs @@ -0,0 +1,116 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; +import { runInNewContext } from 'node:vm'; + +const requireUI = createRequire(new URL('../packages/ui/package.json', import.meta.url)); +const packageDirectory = dirname(requireUI.resolve('@legendapp/list/react')); +const bundles = ['react.js', 'react.mjs', 'react-native.web.js', 'react-native.web.mjs']; + +// Exercise the installed dependency's actual private ScrollAdjust controller. +// Only its external hooks, DOM geometry and frame scheduler are supplied here. +// Keeping the controller in the package (rather than copying it into a test) +// makes a missing patch or a changed upstream implementation fail this check. +function controller(bundle, { horizontal = false, baseline = '' } = {}) { + const source = readFileSync(join(packageDirectory, bundle), 'utf8'); + const start = source.indexOf('function ScrollAdjust() {'); + const end = source.indexOf('var SnapWrapper', start); + assert.ok(start >= 0 && end > start, "Find the pinned package version's ScrollAdjust implementation"); + + const hooks = { useRef: (value) => ({ current: value }), useCallback: (callback) => callback }; + const signals = new Map([['scrollAdjust', 0], ['scrollAdjustUserOffset', 0]]); + const listeners = new Map(); + const frames = new Map(); + let frameId = 0; + const paddingKey = horizontal ? 'paddingRight' : 'paddingBottom'; + let padding = baseline; + const style = {}; + Object.defineProperty(style, paddingKey, { + get: () => padding, + set(value) { + // Observed in Chromium: assigning 2123.1875px reads back as + // 2123.19px. This normalization is what a plain JS style mock misses. + padding = value.endsWith('px') ? `${Number(Number.parseFloat(value).toPrecision(6))}px` : value; + }, + }); + const contentNode = { + style, + get scrollHeight() { return 300 + (Number.parseFloat(padding) || 0); }, + get scrollWidth() { return 300 + (Number.parseFloat(padding) || 0); }, + get offsetHeight() { return this.scrollHeight; }, + }; + const scrollElement = { + scrollTop: 0, scrollLeft: 0, clientHeight: 300, clientWidth: 300, + scrollBy({ left, top }) { this.scrollLeft += left; this.scrollTop += top; }, + }; + const ctx = { state: { props: { horizontal }, scroll: 0, adjustingFromInitialMount: false } }; + const ScrollAdjust = runInNewContext(`(${source.slice(start, end).trim()})`, { + React3: hooks, + React3__namespace: hooks, + useStateContext: () => ctx, + peek$: (_ctx, key) => signals.get(key), + useValueListener$: (key, callback) => listeners.set(key, callback), + getScrollAdjustTarget: () => ({ contentNode, scrollElement }), + getScrollAdjustAxis: () => ({ + x: horizontal ? 1 : 0, y: horizontal ? 0 : 1, + contentSizeKey: horizontal ? 'scrollWidth' : 'scrollHeight', + viewportSizeKey: horizontal ? 'clientWidth' : 'clientHeight', + paddingEndProp: paddingKey, + }), + scrollAdjustBy: (element, left, top) => element.scrollBy({ left, top }), + window: { getComputedStyle: () => style }, + requestAnimationFrame: (callback) => { frames.set(++frameId, callback); return frameId; }, + cancelAnimationFrame: (id) => frames.delete(id), + }); + ScrollAdjust(); + return { + get padding() { return padding; }, + set padding(value) { style[paddingKey] = value; }, + adjust(offset) { + signals.set('scrollAdjustUserOffset', offset); + listeners.get('scrollAdjustUserOffset')(); + }, + finishFrame() { + const callbacks = [...frames.values()]; + frames.clear(); + for (const callback of callbacks) callback(); + }, + }; +} + +for (const bundle of bundles) { + describe(`LegendList temporary padding: ${bundle}`, () => { + it('removes browser-rounded fractional padding on the next frame', () => { + const view = controller(bundle); + view.adjust(1061.59375); + assert.equal(view.padding, '2123.19px'); + view.finishFrame(); + assert.equal(view.padding, ''); + }); + + it('preserves the original baseline across overlapping adjustments', () => { + const view = controller(bundle, { baseline: '12px' }); + view.adjust(1061.59375); + view.adjust(5000.25); + view.finishFrame(); + assert.equal(view.padding, '12px'); + }); + + it('cleans up the horizontal web entry point too', () => { + const view = controller(bundle, { horizontal: true }); + view.adjust(1061.59375); + view.finishFrame(); + assert.equal(view.padding, ''); + }); + + it('does not clear padding changed by another owner', () => { + const view = controller(bundle); + view.adjust(1061.59375); + view.padding = '28px'; + view.finishFrame(); + assert.equal(view.padding, '28px'); + }); + }); +}