Add scrolling behavior to chat#2975
Open
Alexia-Claudia-Micu wants to merge 24 commits into
Open
Conversation
jattasNI
reviewed
Jun 11, 2026
jattasNI
left a comment
Contributor
There was a problem hiding this comment.
I pushed some clean ups and left comments describing them. I still have some more to review.
jattasNI
reviewed
Jun 11, 2026
| @@ -0,0 +1,224 @@ | |||
| import { mediumPadding } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; | |||
Contributor
There was a problem hiding this comment.
@Alexia-Claudia-Micu After the changes I pushed, the biggest remaining open issue I have is about the architecture of the scroll manager class. There are a few things I would like to improve about it. I ran out of time today to try these refactors myself / with copilot so I'm not sure how achievable they are.
- we should look at using intersection observer to detect when scrolling is necessary
- we should try to find a cleaner API contract between the scroll manager and the conversation component. Instead of the scroll manager gaining access to the conversation's internal elements, ideally the conversation would expose an API that the scroll manager could use. This would contain things like:
- fire events when new messages are added or the user scrolls
- properties to get the sizes of certain children
- methods to set the scroll offset and the bottom padding
- the unit tests seem to be mocking a lot of the functionality of the conversation which makes it harder to ensure that the actual conversation is behaving as we want it to. It would be good to explore whether we can write reliable tests that use the real conversation. Alternatively, if we do 2 above then we could write tests for the new conversation API and also write unit tests for the scroll manager which mock out the conversation via a clean contract
I recognize that this is a big refactor so I'm happy to have a discussion about ROI or about me/others helping do it if you find that you + copilot can't get anywhere with it.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull Request
🤨 Rationale
https://dev.azure.com/ni/DevCentral/_workitems/edit/3897966
👩💻 Implementation
Added a new class
ChatConversationScrollManagerwhich contains the scrolling behavior.Added
auto-scrollattribute for consumers to opt out of the scrolling behaviour.User message pinning: when a new outbound message is added,
scrollToLastMessageTop()scrolls it to the top of the viewport.For messages shorter than half the viewport, the message top is pinned with padding-bottom added to extend the scrollable range.
We use a
preventClampPaddingto prevent a visual jump from the new padding immediately changing the scroll position. For messages taller than half the viewport, only the last 20% of the message is shown at the top, leaving the rest of the viewport as whitespace for the advisor response.Auto-scroll: every time the conversation updates,
updatePaddingAndScroll()is called viarequestAnimationFrame. If a user message is pinned, it shrinks the padding-bottom as the advisor response grows (keeping the message at the top). It scrolls to the bottom once the padding is fully consumed.Added
ResizeObserverfor viewport resize and to include copy/regenerate buttons in the scroll.Also use
scrollTo({ behavior: 'smooth' })when scrolling the user message to the top. This means we need aprogrammaticScrollingflag (cleared on scrollend) so it is not mistaken for a user scroll-up.🧪 Testing
Added tests and manual testing in the example app.
✅ Checklist