Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Add scrolling case for longer user message",
"packageName": "@ni/spright-components",
"email": "163188334+Alexia-Claudia-Micu@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import {
// Distance from the bottom (px) within which the conversation is considered "at the bottom".
const scrollingPixelThreshold = 10;

// Maximum fraction of the viewport height that an anchored message may occupy after the
// post-send scroll. Messages taller than this are scrolled past so only this fraction
// remains visible at the top, leaving room for the AI response.
const anchoredMessageMaxViewportFraction = 0.2;

// Slot name for messages that precede the current turn's anchor message.
const historySlotName = 'history';

Expand Down Expand Up @@ -84,6 +89,7 @@ export class AutoScrollManager implements Subscriber {
this.resizeObserver = undefined;
this.setScrollAnchorMessage(undefined);
this.clearSlotAssignments();
this.conversation.anchoredContainer.style.minHeight = '';
this.anchorActive = false;
this.previousMessages = [];
}
Expand Down Expand Up @@ -142,24 +148,36 @@ export class AutoScrollManager implements Subscriber {
this.pendingAnchorInsert = false;
if (anchorInsert) {
this.anchorToLastInsertedMessage();
} else if (this.autoScrollEngaged) {
} else if (this.autoScrollEngaged && this.programmaticScrollTarget === undefined) {
this.followContent();
}
});
}

/**
* Pins the most recently inserted anchor message near the top of the
* viewport.
* viewport. Outbound messages taller than anchoredMessageMaxViewportFraction of
* the viewport are scrolled past so only that fraction remains visible
*/
private anchorToLastInsertedMessage(): void {
const message = this.getLastAnchorMessage();
if (message === undefined) {
return;
}

const anchored = this.conversation.anchoredContainer;
anchored.style.minHeight = '';

this.setScrollAnchorMessage(message);
this.autoScrollEngaged = true;
this.smoothScrollTo(this.getMaxScrollTop());

const target = this.getAnchorScrollTarget(message);
const shortfall = target - this.getMaxScrollTop();
if (shortfall > 0) {
anchored.style.minHeight = `${anchored.offsetHeight + shortfall}px`;
void this.conversation.messagesContainer.scrollHeight;
}
this.smoothScrollTo(Math.min(target, this.getMaxScrollTop()));
}

private followContent(): void {
Expand Down Expand Up @@ -206,6 +224,19 @@ export class AutoScrollManager implements Subscriber {
return Math.max(0, scrollHeight - clientHeight);
}

private getAnchorScrollTarget(message: ChatMessage): number {
const container = this.conversation.messagesContainer;
const containerRect = container.getBoundingClientRect();
const messageRect = message.getBoundingClientRect();
const messageTopInContainer = messageRect.top - containerRect.top + container.scrollTop;
const { clientHeight } = container;
const maxMessageCoverage = clientHeight * anchoredMessageMaxViewportFraction;
if (messageRect.height > maxMessageCoverage) {
return messageTopInContainer + messageRect.height - maxMessageCoverage;
}
return messageTopInContainer;
}

Comment on lines +227 to +239

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple of concerns with this PR:

  • If you notice from the previous refactor, a significant effort was put in to avoid manual calculations like this to measure the dom and instead rely on native CSS browser features. Please take some time to brainstorm and consider how this can be done in CSS with the browser native layout system instead of manually calculating layout. Things like this should be a last resort with comments describing approaches considered.
  • This PR adds no tests. Notice the previous refactor includes tests to cover behavior. Please review and add tests
  • This change is not covered by storybook matrix tests. Make sure this case is covered in storybook matrix snapshot testing to prevent regressions.

private smoothScrollTo(scrollTop: number): void {
const container = this.conversation.messagesContainer;
if (Math.abs(container.scrollTop - scrollTop) <= 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ A specific type of user scrolling that takes place when the user scrolls while A
Scrolling should animate when possible to be smooth rather than instant.

**Post send auto-scroll**
Post send auto-scrolling should occur any time the user sends a new message (regardless of where they are in the chat history at the time)
Post send auto-scrolling should occur any time the user sends a new message (regardless of where they are in the chat history at the time). The user message should be scrolled such that it occupies at most 20% of the viewport.

**AI response auto-scroll**
AI response auto-scroll should occur only if the AI streams responses. Long responses should auto scroll because streaming should be happening at a human readable pace and therefore start removing the old (assumed read) content off screen. If we are not streaming the response and instead returning a block of text, we should not autoscroll because this would take unread content off the screen.
Expand Down
Loading