fix(observer): isolate chat and activity scrolling #456
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // @vitest-environment jsdom | ||
|
|
||
| import { act, cleanup, renderHook } from '@testing-library/react'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { useFollowLatest } from './use-follow-latest'; | ||
|
|
||
| let resized: () => void; | ||
| const disconnect = vi.fn(); | ||
|
|
||
| beforeEach(() => { | ||
| vi.stubGlobal('ResizeObserver', class { | ||
| constructor(callback: () => void) { resized = callback; } | ||
| observe() {} | ||
| disconnect = disconnect; | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| cleanup(); | ||
| vi.unstubAllGlobals(); | ||
| vi.restoreAllMocks(); | ||
| disconnect.mockClear(); | ||
| }); | ||
|
|
||
| function makePane() { | ||
| const pane = document.createElement('div'); | ||
| Object.defineProperties(pane, { | ||
| clientHeight: { value: 200, configurable: true }, | ||
| scrollHeight: { value: 1000, configurable: true }, | ||
| }); | ||
| return pane; | ||
| } | ||
|
|
||
| function scroll(pane: HTMLDivElement, top: number) { | ||
| act(() => { | ||
| pane.scrollTop = top; | ||
| pane.dispatchEvent(new Event('scroll')); | ||
| }); | ||
| } | ||
|
|
||
| describe('useFollowLatest', () => { | ||
| it('follows the initial and latest entries within its own pane', () => { | ||
| const pane = makePane(); | ||
| const other = makePane(); | ||
| other.scrollTop = 123; | ||
| const pageScroll = vi.spyOn(window, 'scrollTo'); | ||
| const ref = { current: pane }; | ||
| const { rerender } = renderHook(({ id }) => useFollowLatest(ref, id), { initialProps: { id: 'first' } }); | ||
| expect(pane.scrollTop).toBe(1000); | ||
| Object.defineProperty(pane, 'scrollHeight', { value: 1200 }); | ||
| rerender({ id: 'second' }); | ||
| expect(pane.scrollTop).toBe(1200); | ||
| expect(other.scrollTop).toBe(123); | ||
| expect(pageScroll).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('preserves the reading position and resumes when the reader returns near the bottom', () => { | ||
| const pane = makePane(); | ||
| const ref = { current: pane }; | ||
| const { rerender } = renderHook(({ id }) => useFollowLatest(ref, id), { initialProps: { id: 'first' } }); | ||
| scroll(pane, 300); | ||
| Object.defineProperty(pane, 'scrollHeight', { value: 1200 }); | ||
| rerender({ id: 'second' }); | ||
| expect(pane.scrollTop).toBe(300); | ||
| scroll(pane, 950); | ||
| Object.defineProperty(pane, 'scrollHeight', { value: 1400 }); | ||
| rerender({ id: 'third' }); | ||
| expect(pane.scrollTop).toBe(1400); | ||
| }); | ||
|
|
||
| it('does not jump when older entries are prepended without changing the latest id', () => { | ||
| const pane = makePane(); | ||
| const ref = { current: pane }; | ||
| const { rerender } = renderHook(() => useFollowLatest(ref, 'latest')); | ||
| scroll(pane, 200); | ||
| Object.defineProperty(pane, 'scrollHeight', { value: 1500 }); | ||
| // The pagination caller restores its anchor after prepending an older page. | ||
| pane.scrollTop += 500; | ||
| rerender(); | ||
| expect(pane.scrollTop).toBe(700); | ||
| }); | ||
|
|
||
| it('follows a pane becoming visible but preserves a reader position on resize', () => { | ||
| const pane = makePane(); | ||
| const ref = { current: pane }; | ||
| renderHook(() => useFollowLatest(ref, 'latest')); | ||
| Object.defineProperty(pane, 'clientHeight', { value: 0 }); | ||
| scroll(pane, 0); | ||
| Object.defineProperty(pane, 'clientHeight', { value: 200 }); | ||
| act(() => resized()); | ||
| expect(pane.scrollTop).toBe(1000); | ||
| scroll(pane, 200); | ||
| act(() => resized()); | ||
| expect(pane.scrollTop).toBe(200); | ||
| }); | ||
|
|
||
| it('waits for the first entry and cleans up the observer and listener', () => { | ||
| const pane = makePane(); | ||
| const removeListener = vi.spyOn(pane, 'removeEventListener'); | ||
| const ref = { current: pane }; | ||
| const { rerender, unmount } = renderHook(({ id }: { id?: string }) => useFollowLatest(ref, id), { initialProps: {} }); | ||
| expect(pane.scrollTop).toBe(0); | ||
| rerender({ id: 'first' }); | ||
| expect(pane.scrollTop).toBe(1000); | ||
| unmount(); | ||
| expect(disconnect).toHaveBeenCalledOnce(); | ||
| expect(removeListener).toHaveBeenCalledWith('scroll', expect.any(Function)); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| 'use client'; | ||
|
|
||
| import { useEffect, useLayoutEffect, useRef, type RefObject } from 'react'; | ||
|
|
||
| const useClientLayoutEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect; | ||
|
|
||
| /** Follow incoming entries within this pane until the reader scrolls away. */ | ||
| export function useFollowLatest(scrollRef: RefObject<HTMLDivElement>, latestId?: string) { | ||
| const following = useRef(true); | ||
|
|
||
| useClientLayoutEffect(() => { | ||
| const pane = scrollRef.current; | ||
| if (pane && latestId && following.current) pane.scrollTop = pane.scrollHeight; | ||
| }, [latestId, scrollRef]); | ||
|
|
||
| useEffect(() => { | ||
| const pane = scrollRef.current; | ||
| if (!pane) return; | ||
| const onScroll = () => { | ||
| // Hidden responsive panes must not change the reader's follow preference. | ||
| if (pane.clientHeight > 0) { | ||
| following.current = pane.scrollHeight - pane.clientHeight - pane.scrollTop < 80; | ||
| } | ||
|
Comment on lines
+19
to
+23
Contributor
There was a problem hiding this comment. 🟡 Cleared activity stops following new events After clearing activity while scrolled to the top, Learn moreThe activity panel stores a preference to follow incoming events in Example: With 300 events, a reader scrolls to the first event, then clicks Clear. Ten new events fill the pane and a twentieth goes below it; the panel remains at the top instead of showing event twenty. Recommended fix: Reset the follow preference when the feed is explicitly cleared, or when it transitions from a nonempty list to empty. Avoid resetting it during ordinary message pagination or a transient content update. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| }; | ||
| const resize = new ResizeObserver(() => { | ||
| if (following.current && pane.clientHeight > 0) pane.scrollTop = pane.scrollHeight; | ||
| }); | ||
| pane.addEventListener('scroll', onScroll, { passive: true }); | ||
| resize.observe(pane); | ||
| return () => { | ||
| pane.removeEventListener('scroll', onScroll); | ||
| resize.disconnect(); | ||
| }; | ||
| }, [scrollRef]); | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: AgentWorkforce/relaycast
Length of output: 7659
🏁 Script executed:
Repository: AgentWorkforce/relaycast
Length of output: 41128
Reset follow mode when the feed is cleared.
When a reader scrolls away from the bottom and clicks Clear,
ActivityLogstays mounted andfollowing.currentcan remain false. If the pane is already atscrollTop === 0, clearing the events does not need to emit a scroll event. Following events can then remain below the viewport.Reset
following.currentwhenlatestIdis absent.Suggested fix
useClientLayoutEffect(() => { const pane = scrollRef.current; - if (pane && latestId && following.current) pane.scrollTop = pane.scrollHeight; + if (!latestId) { + following.current = true; + return; + } + if (pane && following.current) pane.scrollTop = pane.scrollHeight; }, [latestId, scrollRef]);🤖 Prompt for AI Agents