mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
📌 fix: Pin the PTC Trace Through Its Final Settle
The fourth Codex pass on `4bf68e1`, one P2 finding.
`useFollowScroll` returned early whenever `active` was false, so the one
change it most needed to follow was the one it skipped. A failing inner call
settles by appending its error line in the same commit that clears the last
running row: the content grows and the stream ends together, and the pin that
would have revealed that line never fired. On an expanded, bottom-pinned pane
the failure — the row a reader most wants — stayed below the fold.
The falling edge of `active` now pins too, but only when the content changed
with it. Ending a stream on its own still leaves the pane where the reader
left it, which is what the existing contract promises and what the sibling
code and command panes rely on; a reader who has scrolled up is untouched
either way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP
This commit is contained in:
parent
4bf68e18a4
commit
085a83fe5f
2 changed files with 48 additions and 3 deletions
|
|
@ -112,6 +112,34 @@ describe('useFollowScroll', () => {
|
||||||
expect(state.writes).toEqual([900]);
|
expect(state.writes).toEqual([900]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/** A failing PTC call settles by appending its error line in the same
|
||||||
|
* commit that clears the last running row, so the growth and the end of
|
||||||
|
* the stream arrive together. */
|
||||||
|
it('pins one last time when the change that ends the stream also adds content', () => {
|
||||||
|
const { rerender, state } = setup();
|
||||||
|
state.scrollHeight = 1200;
|
||||||
|
rerender(<Probe content="ab" active={false} expanded />);
|
||||||
|
expect(state.writes).toEqual([1200]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not pin the settling change for a reader who scrolled up', () => {
|
||||||
|
const { rerender, pane, state } = setup();
|
||||||
|
state.scrollTop = 100;
|
||||||
|
fireEvent.scroll(pane);
|
||||||
|
state.scrollHeight = 1200;
|
||||||
|
rerender(<Probe content="ab" active={false} expanded />);
|
||||||
|
expect(state.writes).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('pins only the settling change, not the ones after it', () => {
|
||||||
|
const { rerender, state } = setup();
|
||||||
|
rerender(<Probe content="ab" active={false} expanded />);
|
||||||
|
expect(state.writes).toEqual([900]);
|
||||||
|
state.scrollHeight = 1200;
|
||||||
|
rerender(<Probe content="abc" active={false} expanded />);
|
||||||
|
expect(state.writes).toEqual([900]);
|
||||||
|
});
|
||||||
|
|
||||||
it('detaches when the user scrolls up beyond the follow threshold', () => {
|
it('detaches when the user scrolls up beyond the follow threshold', () => {
|
||||||
const { rerender, pane, state } = setup();
|
const { rerender, pane, state } = setup();
|
||||||
state.scrollTop = 100;
|
state.scrollTop = 100;
|
||||||
|
|
|
||||||
|
|
@ -32,8 +32,16 @@ const FOLLOW_THRESHOLD_PX = 40;
|
||||||
* would open at the tail. Gated, it opens reading from the top, while an
|
* would open at the tail. Gated, it opens reading from the top, while an
|
||||||
* expand mid-stream pins immediately. The pin's own scroll event measures
|
* expand mid-stream pins immediately. The pin's own scroll event measures
|
||||||
* distance zero and keeps the attached state, so programmatic scrolls
|
* distance zero and keeps the attached state, so programmatic scrolls
|
||||||
* need no special-casing. Finished panes (`active === false`) are never
|
* need no special-casing.
|
||||||
* scrolled.
|
*
|
||||||
|
* A finished pane (`active === false`) is never scrolled, with one
|
||||||
|
* exception: the very change that ends the stream. That change often adds
|
||||||
|
* content — a failing PTC call settles by appending its error line in the
|
||||||
|
* same commit that clears the last running row — and gating on `active`
|
||||||
|
* alone would drop exactly the pin that content needs, leaving the final
|
||||||
|
* line below the fold. So the falling edge of `active` still pins, but
|
||||||
|
* only when the content changed with it; ending the stream on its own
|
||||||
|
* leaves the pane where the reader left it.
|
||||||
*/
|
*/
|
||||||
export default function useFollowScroll<T extends HTMLElement>(
|
export default function useFollowScroll<T extends HTMLElement>(
|
||||||
content: string | readonly ReactNode[],
|
content: string | readonly ReactNode[],
|
||||||
|
|
@ -42,6 +50,8 @@ export default function useFollowScroll<T extends HTMLElement>(
|
||||||
): { ref: RefObject<T>; onScroll: UIEventHandler<T> } {
|
): { ref: RefObject<T>; onScroll: UIEventHandler<T> } {
|
||||||
const ref = useRef<T>(null);
|
const ref = useRef<T>(null);
|
||||||
const followRef = useRef(true);
|
const followRef = useRef(true);
|
||||||
|
const wasActiveRef = useRef(active);
|
||||||
|
const previousContentRef = useRef(content);
|
||||||
|
|
||||||
const onScroll = useCallback<UIEventHandler<T>>((event) => {
|
const onScroll = useCallback<UIEventHandler<T>>((event) => {
|
||||||
const el = event.currentTarget;
|
const el = event.currentTarget;
|
||||||
|
|
@ -49,7 +59,14 @@ export default function useFollowScroll<T extends HTMLElement>(
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
useLayoutEffect(() => {
|
useLayoutEffect(() => {
|
||||||
if (!active || !expanded || !followRef.current) {
|
const settling = wasActiveRef.current && !Object.is(previousContentRef.current, content);
|
||||||
|
previousContentRef.current = content;
|
||||||
|
wasActiveRef.current = active;
|
||||||
|
|
||||||
|
if (!expanded || !followRef.current) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (!active && !settling) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const el = ref.current;
|
const el = ref.current;
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue