Skip to content

Add scrolling behavior to chat#2975

Open
Alexia-Claudia-Micu wants to merge 24 commits into
mainfrom
users/cmicu/3897966
Open

Add scrolling behavior to chat#2975
Alexia-Claudia-Micu wants to merge 24 commits into
mainfrom
users/cmicu/3897966

Conversation

@Alexia-Claudia-Micu

@Alexia-Claudia-Micu Alexia-Claudia-Micu commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Pull Request

🤨 Rationale

https://dev.azure.com/ni/DevCentral/_workitems/edit/3897966

👩‍💻 Implementation

Added a new class ChatConversationScrollManager which contains the scrolling behavior.

Added auto-scroll attribute 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 preventClampPadding to 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 via requestAnimationFrame. 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 ResizeObserver for 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 a programmaticScrolling flag (cleared on scrollend) so it is not mistaken for a user scroll-up.

🧪 Testing

Added tests and manual testing in the example app.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@Alexia-Claudia-Micu Alexia-Claudia-Micu marked this pull request as ready for review June 11, 2026 07:46
@jattasNI jattasNI requested a review from msmithNI as a code owner June 11, 2026 19:16

@jattasNI jattasNI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I pushed some clean ups and left comments describing them. I still have some more to review.

Comment thread packages/spright-components/src/chat/conversation/index.ts Outdated
Comment thread packages/spright-components/src/chat/conversation/index.ts Outdated
Comment thread packages/spright-components/src/chat/conversation/index.ts Outdated
Comment thread change/@ni-spright-blazor-08d16ce6-2034-41f4-9374-2a2fed2ee178.json
@@ -0,0 +1,224 @@
import { mediumPadding } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@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.

  1. we should look at using intersection observer to detect when scrolling is necessary
  2. 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
  3. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants