diff --git a/client/src/components/Chat/Messages/MessageNav.tsx b/client/src/components/Chat/Messages/MessageNav.tsx index e7a4cd2c86..afb3a9aad9 100644 --- a/client/src/components/Chat/Messages/MessageNav.tsx +++ b/client/src/components/Chat/Messages/MessageNav.tsx @@ -146,6 +146,31 @@ function computeTargetScroll( return Math.max(0, Math.min(target, max)); } +/** + * An entry's top edge in the scroll container's content space — the same space + * as `container.scrollTop`. A single `offsetTop` is measured from the nearest + * positioned ancestor, which for an in-thread steer is the response's `relative` + * content column, not the scroll content. Mixing that local value with the + * content-space `offsetTop` of top-level rows compares different origins and + * breaks every rail decision (jump targets, current row, fisheye centering) the + * moment the viewport reaches a steer. Summing `offsetTop` up the offsetParent + * chain until it leaves the container folds any nesting back into one origin; + * top-level rows collapse to a single hop. + */ +function entryTop(el: HTMLElement, container: HTMLElement): number { + let top = 0; + let node: Element | null = el; + while (node instanceof HTMLElement) { + top += node.offsetTop; + const parent = node.offsetParent; + if (!(parent instanceof HTMLElement) || parent === container || !container.contains(parent)) { + break; + } + node = parent; + } + return top; +} + type RibDims = { baseW: number; baseH: number; peakW: number; peakH: number }; const RIB_END: RibDims = { baseW: 3, baseH: 3, peakW: 4.5, peakH: 4.5 }; @@ -289,18 +314,26 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject { + const container = scrollableRef.current; + if (!container) { + return null; + } let nextId: string | null = null; let nextTop = Number.POSITIVE_INFINITY; for (const id of visibleSetRef.current) { const el = observedRef.current.get(id); - if (!el || el.offsetTop >= nextTop) { + if (!el) { + continue; + } + const top = entryTop(el, container); + if (top >= nextTop) { continue; } nextId = id; - nextTop = el.offsetTop; + nextTop = top; } return nextId; - }, []); + }, [scrollableRef]); useEffect(() => { messagesByIdRef.current = messagesById; @@ -862,8 +895,14 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject { for (let i = 0; i < entries.length; i++) { const el = resolveEntryEl(entries[i].id); - offsetsTop[i] = el ? el.offsetTop : Number.POSITIVE_INFINITY; - offsetsBottom[i] = el ? el.offsetTop + el.offsetHeight : Number.POSITIVE_INFINITY; + if (!el) { + offsetsTop[i] = Number.POSITIVE_INFINITY; + offsetsBottom[i] = Number.POSITIVE_INFINITY; + continue; + } + const top = entryTop(el, container); + offsetsTop[i] = top; + offsetsBottom[i] = top + el.offsetHeight; } }; recomputeOffsets(); @@ -1128,7 +1167,7 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject scrollTop + JUMP_EPS) { + if (entryTop(el, container) - scrollMargin > scrollTop + JUMP_EPS) { scrollToStart(entries[i].id); return; } diff --git a/client/src/components/Chat/Messages/__tests__/MessageNav.spec.tsx b/client/src/components/Chat/Messages/__tests__/MessageNav.spec.tsx index 921e5776c8..5cd92eb8a0 100644 --- a/client/src/components/Chat/Messages/__tests__/MessageNav.spec.tsx +++ b/client/src/components/Chat/Messages/__tests__/MessageNav.spec.tsx @@ -476,6 +476,56 @@ describe('MessageNav', () => { expect(steerRib?.getAttribute('aria-label')).not.toContain('Danny'); }); + it('places a nested steer at its content-space position, not its offset-parent-local offset', () => { + // A steer renders inside the response's `relative` content column, so its + // raw offsetTop is local to that column, not its true position in the + // thread. The rail must sum the offsetParent chain — otherwise the steer's + // small local offset reads as the topmost row and hijacks the current + // indicator (and, with it, the up/down chevrons) whenever it is on screen. + const messages = [ + buildMessage({ messageId: 'u1', text: 'first ask', isCreatedByUser: true }), + buildMessage({ messageId: 'a1', text: 'long tool run' }), + buildMessage({ messageId: 'u2', text: 'follow-up', isCreatedByUser: true }), + buildMessage({ messageId: 'a2', text: 'second reply' }), + ]; + mockUseGetMessagesByConvoId.mockReturnValue({ data: messages }); + const { scrollable } = buildDom(messages); + const response = scrollable.querySelector('#a1') as HTMLElement; + + // Nest the steer in a positioned column of its own; the steer's offsetTop + // (40) is local to that column (380), so its content-space top is 420 — + // below a1 (300) and above u2 (500). jsdom leaves offsetParent null, so + // the nesting has to be declared for the chain-walk to have anything to + // sum. + const column = document.createElement('div'); + column.className = 'relative'; + Object.defineProperty(column, 'offsetTop', { value: 380, configurable: true }); + response.appendChild(column); + const steer = appendSteerNode(column, 's1', 'steer mid-run words', 40); + Object.defineProperty(steer, 'offsetParent', { value: column, configurable: true }); + + const scrollableRef = { current: scrollable } as RefObject; + const { container } = render(); + act(() => { + jest.advanceTimersByTime(250); + }); + + const io = MockIntersectionObserver.last(); + act(() => { + io!.trigger([ + { target: document.getElementById('a1')!, isIntersecting: true }, + { target: document.getElementById('steer-s1')!, isIntersecting: true }, + ]); + jest.advanceTimersByTime(32); + }); + + // Topmost-by-content-space is the response, not the steer with the smaller + // local offset. Before the chain-walk this landed on 'steer-s1'. + const current = container.querySelectorAll('[aria-current="true"]'); + expect(current).toHaveLength(1); + expect(current[0]).toHaveAttribute('data-msg-id', 'a1'); + }); + it('un-lights a steer rib when its DOM node is replaced (pending → applied swap)', async () => { const messages = [ buildMessage({ messageId: 'u1', text: 'first ask', isCreatedByUser: true }),