Skip to content

refine scrolling #468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iceljc merged 1 commit into from
Jul 9, 2026
Merged

refine scrolling #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/lib/common/modals/DialogModal.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
cancel = () => {},
close = () => {},
disableConfirmBtn = false,
disableBackdropClick = false,
children,
titleIcon = undefined
} = $props();
Expand Down Expand Up @@ -51,6 +52,9 @@

/** @param {MouseEvent} e */
function handleBackdropClick(e) {
if (disableBackdropClick) {
return;
}
if (e.target === e.currentTarget) {
toggleModal();
}
Expand Down
18 changes: 5 additions & 13 deletions src/lib/styles/pages/_chat.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,13 +1439,14 @@

/* Scrollable thread area (replaces .chat-scrollbar.chat-content.scroll-bottom-to-top) */


.cb-msgs-scroll {
display: flex;
flex-direction: column-reverse;
overflow-y: auto;
overflow-y: scroll;
scrollbar-width: none;
scroll-behavior: smooth;

&::-webkit-scrollbar {
display: none;
}
}


Expand All @@ -1459,15 +1460,6 @@
}


/* Alias of .cb-msgs-scroll for clarity at the markup site */


.cb-msgs-scroll-rev {
display: flex;
flex-direction: column-reverse;
}


/* Conversation container (replaces .chat-conversation.p-3) */


Expand Down
104 changes: 79 additions & 25 deletions src/routes/chat/[agentId]/[conversationId]/chat-box.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script>
import { onMount, setContext, tick } from 'svelte';
import { onDestroy, onMount, setContext, tick } from 'svelte';
import { get } from 'svelte/store';
import { Pane, Splitpanes } from 'svelte-splitpanes';
import Viewport from 'svelte-viewport-info';
Expand Down Expand Up @@ -232,6 +232,10 @@
autoScrollToBottom: autoScrollToBottom
});

onDestroy(() => {
scrollbars.forEach(scrollbar => scrollbar?.destroy?.());
});

onMount(async () => {
disableSpeech = navigator.userAgent.includes('Firefox');
// @ts-ignore
Expand Down Expand Up @@ -268,10 +272,9 @@
// @ts-ignore
await signalr.start(page.params.conversationId);

scrollbars = [
document.querySelector('.cb-msgs-scroll')
].filter(Boolean);
refresh();
initScrollbar();
await refresh(true);
pinToBottomWhileSettling();

window.addEventListener('message', async (e) => {
if (e.data.action === ChatAction.Logout) {
Expand All @@ -284,6 +287,15 @@
});
});

function initScrollbar() {
/** @type {HTMLElement | null} */
const msgScrollElem = document.querySelector('.cb-msgs-scroll');
if (msgScrollElem) {
// @ts-ignore
scrollbars = [OverlayScrollbars(msgScrollElem, options)];
}
}

function handleLogoutAction() {
resetStorage(true);
}
Expand Down Expand Up @@ -443,7 +455,8 @@
return dialogs;
}

async function refresh() {
/** @param {boolean} stopScroll */
async function refresh(stopScroll = false) {
// trigger UI render
dialogs = await refreshDialogs();
lastBotMsg = null;
Expand All @@ -454,24 +467,64 @@
groupedDialogs = groupDialogs(dialogs);
await tick();

autoScrollToBottom();
if (!stopScroll) {
autoScrollToBottom();
}
}

let _autoScrollScheduled = false;
function autoScrollToBottom() {
if (_autoScrollScheduled) return;
_autoScrollScheduled = true;
requestAnimationFrame(() => {
scrollbars.forEach(scrollbar => {
if (!scrollbar) return;
setTimeout(() => {
scrollbar.scrollTo({ top: scrollbar.scrollHeight, behavior: 'smooth' });
}, 150);
});
_autoScrollScheduled = false;
const scrollToBottom = () => {
scrollbars.forEach(scrollbar => {
if (!scrollbar) return;
const { viewport } = scrollbar.elements();
viewport.scrollTo({ top: viewport.scrollHeight, behavior: 'smooth' });
});
_autoScrollScheduled = false;
};
scrollToBottom();
});
}

/**
* Keep the thread pinned to the very bottom while the initial layout settles.
* Async content (images, code blocks, mermaid diagrams) can grow the height
* after first paint, so a fixed delay isn't enough — a ResizeObserver re-pins
* on every height change using instant `scrollTop` writes (no animation, no
* visible scroll). Pinning stops as soon as the user interacts with the pane,
* or after a safety timeout, so it never fights manual scrolling.
* @param {number} timeoutMs
*/
function pinToBottomWhileSettling(timeoutMs = 3000) {
const scrollbar = scrollbars[0];
if (!scrollbar) return;

const { viewport } = scrollbar.elements();
const content = viewport.firstElementChild || viewport;
const pin = () => { viewport.scrollTop = viewport.scrollHeight; };
pin();

const observer = new ResizeObserver(pin);
observer.observe(content);

/** @type {ReturnType<typeof setTimeout>} */
let timer;
const stop = () => {
observer.disconnect();
clearTimeout(timer);
viewport.removeEventListener('wheel', stop);
viewport.removeEventListener('pointerdown', stop);
viewport.removeEventListener('keydown', stop);
};
viewport.addEventListener('wheel', stop, { passive: true });
viewport.addEventListener('pointerdown', stop);
viewport.addEventListener('keydown', stop);
timer = setTimeout(stop, timeoutMs);
}
Comment on lines +501 to +526

@qodo-code-review qodo-code-review Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Pin observer lacks teardown 🐞 Bug ☼ Reliability

In chat-box.svelte, pinToBottomWhileSettling() registers a ResizeObserver, timeout, and event
listeners but nothing calls its stop()/disconnect logic during component teardown, so callbacks can
run against detached DOM until the timeout fires. onDestroy() only destroys OverlayScrollbars
instances and does not stop the pinning observer/listeners.
Agent Prompt 
### Issue description
`pinToBottomWhileSettling()` creates a `ResizeObserver`, a timeout, and several event listeners, but its cleanup (`observer.disconnect()`, `clearTimeout`, `removeEventListener`) is only executed when `stop()` runs. If the route/component unmounts before that, these resources can remain active briefly and execute against DOM that’s no longer owned by the component.

### Issue Context
`onDestroy()` currently destroys OverlayScrollbars instances, but the pinning observer/listeners created by `pinToBottomWhileSettling()` are not referenced there.

### Fix Focus Areas
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[235-237]
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[492-526]

### Suggested fix
- Make `pinToBottomWhileSettling()` return a disposer (e.g., `return stop;`) or store `stop` in an outer-scope variable.
- Invoke that disposer from `onDestroy()` (and optionally before re-running pinning) to ensure the `ResizeObserver`, timer, and listeners are always cleaned up on unmount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


/** @param {import('$conversationTypes').ChatResponseModel[]} dialogs */
function assignMessageDisclaimer(dialogs) {
if (!dialogs) return null;
Expand Down Expand Up @@ -1699,6 +1752,7 @@
title={'Send message'}
size={'5xl'}
isOpen={isOpenBigMsgModal}
disableBackdropClick={true}
toggleModal={() => toggleBigMessageModal()}
confirm={() => sendBigMessage()}
cancel={() => toggleBigMessageModal()}
Expand Down Expand Up @@ -1899,17 +1953,17 @@
</div>
</div>

<StateModal
isOpen={isOpenUserAddStateModal}
inline
bind:states={userAddStates}
requireActiveRounds
toggleModal={() => toggleUserAddStateModal()}
confirm={() => handleConfirmUserAddStates()}
cancel={() => toggleUserAddStateModal()}
/>

<div class={`cb-msgs-scroll cb-msgs-content cb-msgs-scroll-rev ${!loadEditor ? 'cb-msgs-content-expand' : ''}`}>
<StateModal
isOpen={isOpenUserAddStateModal}
inline
bind:states={userAddStates}
requireActiveRounds
toggleModal={() => toggleUserAddStateModal()}
confirm={() => handleConfirmUserAddStates()}
cancel={() => toggleUserAddStateModal()}
/>

<div class={`cb-msgs-scroll cb-msgs-content ${!loadEditor ? 'cb-msgs-content-expand' : ''}`}>
<div class="cb-conv">
<ul class="cb-conv-list">
{#each Object.entries(groupedDialogs) as [createDate, dialogGroup]}
Expand Down
41 changes: 39 additions & 2 deletions src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@

onMount(() => {
// Load history, wait for Svelte to flush the new list items to the
// DOM, then attach OverlayScrollbars and scroll to bottom. Without
// DOM, then attach OverlayScrollbars and pin to the bottom. Without
// the tick() wait, scrollHeight is read before the rows render and
// the viewport ends up parked at the top.
(async () => {
await Promise.all([getChatContentLogs(), getChatStateLogs()]);
await tick();
initScrollbars();
scroll();
pinToBottomWhileSettling();
})();

return () => {
Expand Down Expand Up @@ -111,6 +111,43 @@
});
}

/**
* Keep each log panel pinned to the very bottom while the initial layout
* settles. Async content can grow the row heights after first paint, so a
* fixed delay isn't enough — a ResizeObserver re-pins on every height change
* using instant `scrollTop` writes (no animation, no visible scroll).
* Pinning stops as soon as the user interacts with a panel, or after a
* safety timeout, so it never fights manual scrolling.
* @param {number} timeoutMs
*/
function pinToBottomWhileSettling(timeoutMs = 3000) {
scrollbars.forEach(scrollbar => {
if (!scrollbar) return;

const { viewport } = scrollbar.elements();
const content = viewport.firstElementChild || viewport;
const pin = () => { viewport.scrollTop = viewport.scrollHeight; };
pin();

const observer = new ResizeObserver(pin);
observer.observe(content);

/** @type {ReturnType<typeof setTimeout>} */
let timer;
const stop = () => {
observer.disconnect();
clearTimeout(timer);
viewport.removeEventListener('wheel', stop);
viewport.removeEventListener('pointerdown', stop);
viewport.removeEventListener('keydown', stop);
};
viewport.addEventListener('wheel', stop, { passive: true });
viewport.addEventListener('pointerdown', stop);
viewport.addEventListener('keydown', stop);
timer = setTimeout(stop, timeoutMs);
});
Comment on lines +123 to +148

@qodo-code-review qodo-code-review Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Persist-log leaks scroll resources 🐞 Bug ☼ Reliability

In persist-log.svelte, pinToBottomWhileSettling() adds ResizeObserver/listeners per panel but the
onMount cleanup only calls cleanLogs(), so these resources may remain active after unmount until the
timeout fires. Additionally, initScrollbars() creates OverlayScrollbars instances but the component
never destroys them on teardown.
Agent Prompt 
### Issue description
`pinToBottomWhileSettling()` attaches `ResizeObserver` + event listeners + a timer for each OverlayScrollbars viewport, but the component’s teardown path (the `onMount` return cleanup) doesn’t call any disposer, so resources can outlive the component briefly. Separately, OverlayScrollbars instances created in `initScrollbars()` are never `.destroy()`’d.

### Issue Context
PersistLog is conditionally mounted/unmounted from chat-box; when it unmounts, it should release all observers/listeners and destroy OverlayScrollbars instances.

### Fix Focus Areas
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[69-84]
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[114-149]
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[151-160]

### Suggested fix
- Refactor `pinToBottomWhileSettling()` to return an array of disposer functions (one per scrollbar) or store them in component scope.
- In the `onMount` cleanup (or `onDestroy`), call those disposers to `disconnect()` observers, remove listeners, and clear timers immediately.
- Also destroy OverlayScrollbars instances in teardown (e.g., `scrollbars.forEach(sb => sb?.destroy?.()); scrollbars = [];`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

function initScrollbars() {
scrollbarElements.forEach(item => {
const elem = document.querySelector(item.id);
Expand Down
Loading