From 01d16bbcf0485b3ca90228c67ca78e8599f9495b Mon Sep 17 00:00:00 2001 From: bowenliang123 Date: Wed, 26 Aug 2026 16:39:19 +0800 Subject: [PATCH] fix(client): re-anchor the trend chart only on real data pushes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chart's layout effect ran after every render (no deps), so any hover/select re-render near the right edge re-ran the stick-to-end check — and by the time it read el.scrollWidth the value was already the post-append width, which could misfire the anchor. The effect now runs only when granularity, focusTurn, or the request list changes, and the near-end check compares against the PREVIOUS pass's scrollWidth (kept in a ref), so a reader near the right edge still follows an appended bar while a mid-chart reader is never yanked back, and a hover/select render never touches the scroll at all. --- src/client/components/trendChart.tsx | 28 +++++++++++---- tests/client/components/trendChart.spec.ts | 42 ++++++++++++++++++---- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/client/components/trendChart.tsx b/src/client/components/trendChart.tsx index 602ac3a..c8566fd 100644 --- a/src/client/components/trendChart.tsx +++ b/src/client/components/trendChart.tsx @@ -251,10 +251,17 @@ export function makeTrendChart(kit: ViewKit): (props: TrendChartProps) => ReactN const scrollRef = React.useRef(null) const scrolledOnce = React.useRef(false) const lastGranRef = React.useRef(props.granularity) + // The newest bar's seq (or 0 when the log is empty): the layout effect only re-runs when the right edge genuinely + // moves (new bar appended, granularity switched, focus turn set) — hover/select changes keep their scroll position + // so the chart does not flash with every keystroke. + const lastSeqRef = React.useRef(0) + // The scrollWidth measured during the PREVIOUS effect pass. The "was the reader near the right edge?" check + // has to compare against the width as it was BEFORE the new bar landed — by the time the layout effect runs, + // `el.scrollWidth` is already the new (wider) value, so a near-edge check against it would miss the auto-follow. + const prevScrollWidthRef = React.useRef(0) const [edges, setEdges] = React.useState<{ left: boolean; right: boolean }>({ left: false, right: false }) - // Mirror of the last computed fades: the layout effect runs after EVERY render (no deps), so it must dispatch setState only on true - // change — a same-value dispatch during a granularity switch's pending lanes disables React's eager bailout and the queue grows - // unbounded (React error #185). + // Mirror of the last computed fades: the layout effect dispatches setState only on true change, so a same-value + // dispatch during a granularity switch's pending lanes does not disable React's eager bailout (React error #185). const edgesRef = React.useRef(edges) const updateEdges = (el: HTMLDivElement): void => { const left = el.scrollLeft > 4 @@ -298,6 +305,9 @@ export function makeTrendChart(kit: ViewKit): (props: TrendChartProps) => ReactN /* v8 ignore next 1 -- the scroll div renders unconditionally and React attaches refs before layout effects run; el is never null here. */ if (el === null) return + const newestSeq = requests.length === 0 ? 0 : requests[requests.length - 1].seq + const grew = newestSeq !== lastSeqRef.current + const widthBeforeAppend = prevScrollWidthRef.current if (props.granularity !== lastGranRef.current) { lastGranRef.current = props.granularity scrolledOnce.current = false @@ -311,16 +321,20 @@ export function makeTrendChart(kit: ViewKit): (props: TrendChartProps) => ReactN el.scrollLeft = Math.max(0, gi * (BAR_W + BAR_GAP) + BAR_W / 2 - el.clientWidth / 2) } props.onFocusTurnHandled() - } - if (!scrolledOnce.current) { + } else if (!scrolledOnce.current) { scrolledOnce.current = true el.scrollLeft = el.scrollWidth - } else if (el.scrollLeft + el.clientWidth >= el.scrollWidth - 24) { + } else if (grew && el.scrollLeft + el.clientWidth >= widthBeforeAppend - 24) { + // Only follow the latest bar when the right edge actually moved AND the reader was already near it + // BEFORE the new bar landed — comparing against the new scrollWidth would silently drop the stick for a + // mid-chart reader whose viewport just slid past the near-end threshold. el.scrollLeft = el.scrollWidth } + lastSeqRef.current = newestSeq + prevScrollWidthRef.current = el.scrollWidth updateEdges(el) updateTurnLabels(el) - }) + }, [props.granularity, props.focusTurn, requests]) // Compact single-line hover tooltip, shown instantly by the custom `.lc-chart-tip` (the native title is delayed): position, time, // estimated total, provider prompt when available; the per-category breakdown lives in the detail panel below. diff --git a/tests/client/components/trendChart.spec.ts b/tests/client/components/trendChart.spec.ts index 4e64a2e..b9c24ea 100644 --- a/tests/client/components/trendChart.spec.ts +++ b/tests/client/components/trendChart.spec.ts @@ -501,19 +501,21 @@ describe('TrendChart edge fades and scroll anchoring', () => { assert.equal(queryAll(m.container, '.lc-chart-fade-l').length, 1) assert.equal(queryAll(m.container, '.lc-chart-fade-r').length, 1) - // An unrelated update mid-scroll does NOT re-anchor (100 + 400 < 640 - 24). + // An unrelated (selection-only) update mid-scroll does NOT re-anchor (100 + 400 < 640 - 24), and the chart + // also stays put when the same selection-only update fires NEAR the right edge — only a real data push + // (a new request appended, or a granularity/focus switch) may follow the newest bar, so a hover/select + // change never flashes the column. await m.update(h(TrendChart, propsOf(reqs, { ...handlers, selectedSeq: 1 }))) assert.equal(scroll.scrollLeft, 100) - // …but near the end (230 + 400 >= 640 - 24) the next render sticks to the newest edge. await scrollTo(scroll, 230) await m.update(h(TrendChart, propsOf(reqs, { ...handlers, selectedSeq: 2 }))) await flush() - assert.equal(scroll.scrollLeft, 240) - assert.equal(queryAll(m.container, '.lc-chart-fade-l').length, 1) - assert.equal(queryAll(m.container, '.lc-chart-fade-r').length, 0) + assert.equal(scroll.scrollLeft, 230, 'selection-only update near the end does not stick') - // Turn labels re-center within their visible slice: T2 is half-clipped at the left → shifted right; - // T3 is fully visible and centered → no transform; T1 is fully out of view → untouched. + // Turn labels re-center within their visible slice: with the reader at scrollLeft 240, T2 is half-clipped + // at the left → shifted right; T3 is fully visible and centered → no transform; T1 is fully out of view + // → untouched. + await scrollTo(scroll, 240) const labels = queryAll(m.container, '.lc-turn-label') assert.equal(labels.length, 4) assert.equal(labels[1].style.transform, 'translateX(40px)') @@ -524,6 +526,32 @@ describe('TrendChart edge fades and scroll anchoring', () => { await m.unmount() }) + test('a data push follows the newest bar only when the reader was near the right edge', async () => { + const reqs = manySteps() + const { handlers } = makeSpies() + const m = await mount(h(TrendChart, propsOf(reqs, handlers))) + await flush() + const scroll = query(m.container, '.lc-chart-scroll') + // Mount anchors at the right edge (scrollLeft = 240 over scrollWidth 640 / clientWidth 400). + assert.equal(scroll.scrollLeft, 240) + + // A new request appended while the reader sits at the right edge → follows the newest bar. + const grown = [...reqs, req(reqs.length + 1, { turn: 1 + Math.floor(reqs.length / 10), step: reqs.length % 10 })] + await m.update(h(TrendChart, propsOf(grown, handlers))) + await flush() + assert.equal(scroll.scrollLeft, 256, 'near-edge reader follows the appended bar') + assert.equal(queryAll(m.container, '.lc-chart-fade-l').length, 1) + assert.equal(queryAll(m.container, '.lc-chart-fade-r').length, 0) + + // A push while the reader is mid-chart leaves them where they were — no auto-follow yank. + await scrollTo(scroll, 100) + const grown2 = [...grown, req(grown.length + 1, { turn: 1 + Math.floor(grown.length / 10), step: grown.length % 10 })] + await m.update(h(TrendChart, propsOf(grown2, handlers))) + await flush() + assert.equal(scroll.scrollLeft, 100, 'mid-scroll reader is not yanked back to the newest') + await m.unmount() + }) + test('a granularity switch re-anchors and flips overflow for real (React #185 regression)', async () => { const reqs = manySteps() const { handlers } = makeSpies()