Skip to content

Commit f673745

Browse files
ncamerancamera
authored
[ch-chat] Fix scroll flickering when updating the last chat message and the scroll is not at the bottom (#490)
* Fix ch-chat scroll flickering when streaming a new message * Set loadingState === "initial" by default in the ch-smart-grid * Add unit tests * Improve showcase * Add more unit tests * Add more unit tests * Add more unit tests
1 parent 92a740b commit f673745

File tree

14 files changed

+849
-22
lines changed

14 files changed

+849
-22
lines changed

‎src/components/chat/chat.tsx

+4-2
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ export class ChChat {
8282
*/
8383
@Prop() readonly isMobile?: boolean = false;
8484

85+
// TODO: Add support for undefined messages.
8586
/**
8687
* Specifies the items that the chat will display.
8788
*/
@@ -90,7 +91,7 @@ export class ChChat {
9091
/**
9192
* Specifies if the chat is waiting for the data to be loaded.
9293
*/
93-
@Prop({ mutable: true }) loadingState?: SmartGridDataState = "initial";
94+
@Prop({ mutable: true }) loadingState: SmartGridDataState = "initial";
9495

9596
/**
9697
* Specifies the theme to be used for rendering the markdown.
@@ -403,7 +404,7 @@ export class ChChat {
403404
<slot name="empty-chat"></slot>
404405
) : (
405406
<ch-smart-grid
406-
dataProvider
407+
dataProvider={this.loadingState === "more-data-to-fetch"}
407408
loadingState={
408409
this.virtualItems.length === 0 ? "initial" : this.loadingState
409410
}
@@ -514,6 +515,7 @@ export class ChChat {
514515
{this.theme && <ch-theme model={this.theme}></ch-theme>}
515516

516517
{this.loadingState === "initial" ? (
518+
// TODO: Improve this slot name
517519
<div class="loading-chat" slot="empty-chat"></div>
518520
) : (
519521
this.#renderChatOrEmpty()

‎src/components/chat/tests/scroll.e2e.ts

+246
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
import { E2EElement, E2EPage, newE2EPage } from "@stencil/core/testing";
2+
import {
3+
FIFTEEN_ITEMS,
4+
LOADING_STATE_VALUES,
5+
LONG_STRING,
6+
TWENTY_FIVE_ITEMS,
7+
TWENTY_ITEMS
8+
} from "./utils.e2e";
9+
import { SmartGridDataState } from "../../smart-grid/internal/infinite-scroll/types";
10+
import { ChatMessage } from "../types";
11+
12+
const FLOATING_POINT_ERROR_PRECISION = 1;
13+
14+
const valuesAreEqualInMarginOfError = (a: number, b: number) =>
15+
Math.abs(a - b) <= FLOATING_POINT_ERROR_PRECISION;
16+
17+
// eslint-disable-next-line @typescript-eslint/ban-types
18+
type ArgumentTypes<F extends Function> = F extends (...args: infer A) => any
19+
? A
20+
: never;
21+
22+
type UpdateLastChatMessageArgTypes = ArgumentTypes<
23+
HTMLChChatElement["updateLastMessage"]
24+
>;
25+
26+
describe("[ch-chat][scroll]", () => {
27+
let page: E2EPage;
28+
let chatRef: E2EElement;
29+
30+
beforeEach(async () => {
31+
page = await newE2EPage({
32+
html: `
33+
<div style="display: grid; block-size: 500px; inline-size: 500px">
34+
<ch-chat></ch-chat>
35+
</div>
36+
`,
37+
failOnConsoleError: true
38+
});
39+
40+
chatRef = await page.find("ch-chat");
41+
});
42+
43+
const scrollIsAtTheBottom = (sizes: {
44+
scrollTop: number;
45+
scrollHeight: number;
46+
offsetHeight: number;
47+
}) =>
48+
sizes.scrollTop !== 0 &&
49+
sizes.offsetHeight !== sizes.scrollHeight &&
50+
sizes.scrollHeight <= sizes.scrollTop + sizes.offsetHeight;
51+
52+
const getContentScrollPosition = (): Promise<{
53+
scrollTop: number;
54+
scrollHeight: number;
55+
offsetHeight: number;
56+
}> =>
57+
page.evaluate(() => {
58+
const chatRef = document.querySelector("ch-chat");
59+
60+
const smartGridRef = chatRef.shadowRoot.querySelector(
61+
"ch-smart-grid"
62+
) as HTMLChSmartGridElement;
63+
64+
return {
65+
scrollTop: smartGridRef.scrollTop,
66+
scrollHeight: smartGridRef.scrollHeight,
67+
offsetHeight: smartGridRef.offsetHeight
68+
};
69+
});
70+
71+
const updateScrollPosition = async scrollTop => {
72+
await page.evaluate((scrollTop: number) => {
73+
document
74+
.querySelector("ch-chat")
75+
.shadowRoot.querySelector("ch-smart-grid").scrollTop = scrollTop;
76+
}, scrollTop);
77+
78+
// Just in case, wait for changes to be completed
79+
await page.waitForChanges();
80+
};
81+
82+
const setInitialModel = async (
83+
loadingState: SmartGridDataState,
84+
model: ChatMessage[]
85+
) => {
86+
chatRef.setProperty("loadingState", loadingState);
87+
chatRef.setProperty("items", model);
88+
await page.waitForChanges();
89+
90+
// We need to wait an extra frame to compute the sizes, because the
91+
// infinite scroll and virtual scroller can not do the rendering and
92+
// positioning in only one frame
93+
await page.waitForChanges();
94+
};
95+
96+
const scrollPositionIsAtBottomOnInitialLoadTest = (
97+
loadingState: SmartGridDataState
98+
) => {
99+
it(`[loadingState = "${loadingState}"] should set scroll at the bottom when items are rendered for the first time`, async () => {
100+
await setInitialModel(loadingState, TWENTY_ITEMS);
101+
102+
const sizes = await getContentScrollPosition();
103+
104+
expect(scrollIsAtTheBottom(sizes)).toBe(true);
105+
});
106+
};
107+
108+
const shouldCorrectlySetTheScrollTopPositionTest = (
109+
loadingState: SmartGridDataState
110+
) => {
111+
it(`[loadingState = "${loadingState}"] should work setting the scrollTop`, async () => {
112+
await setInitialModel(loadingState, TWENTY_ITEMS);
113+
114+
const sizes = await getContentScrollPosition();
115+
const newScrollPosition = sizes.scrollTop - 200;
116+
117+
await updateScrollPosition(newScrollPosition);
118+
119+
const sizesAfterUpdate = await getContentScrollPosition();
120+
121+
expect(
122+
valuesAreEqualInMarginOfError(
123+
sizesAfterUpdate.scrollTop,
124+
newScrollPosition
125+
)
126+
).toBe(true);
127+
});
128+
};
129+
130+
const shouldKeepTheScrollAtTheBottomWhenSwitchingModelsTest = (
131+
loadingState: SmartGridDataState
132+
) => {
133+
it(`[loadingState = "${loadingState}"] switching between models should re position the scroll at the bottom (model with more items)`, async () => {
134+
await setInitialModel(loadingState, TWENTY_ITEMS);
135+
136+
const sizes = await getContentScrollPosition();
137+
const newScrollPosition = sizes.scrollTop - 200;
138+
139+
// Re position the scroll
140+
await updateScrollPosition(newScrollPosition);
141+
142+
// Set a model with more items
143+
await setInitialModel(loadingState, TWENTY_FIVE_ITEMS);
144+
145+
const sizesAfterUpdate = await getContentScrollPosition();
146+
147+
expect(scrollIsAtTheBottom(sizesAfterUpdate)).toBe(true);
148+
});
149+
150+
it(`[loadingState = "${loadingState}"] switching between models should re position the scroll at the bottom (model with less items)`, async () => {
151+
await setInitialModel(loadingState, TWENTY_ITEMS);
152+
153+
const sizes = await getContentScrollPosition();
154+
const newScrollPosition = sizes.scrollTop - 200;
155+
156+
// Re position the scroll
157+
await updateScrollPosition(newScrollPosition);
158+
159+
// Set a model with more items
160+
await setInitialModel(loadingState, FIFTEEN_ITEMS);
161+
162+
const sizesAfterUpdate = await getContentScrollPosition();
163+
164+
expect(scrollIsAtTheBottom(sizesAfterUpdate)).toBe(true);
165+
});
166+
};
167+
168+
const shouldNotMoveTheScrollTopWhenUpdatingTheLastChatMessage = (
169+
loadingState: SmartGridDataState
170+
) => {
171+
it(`[loadingState = "${loadingState}"] if the scroll is not at the bottom, it should not change the scrollTop position when updating the last chat message`, async () => {
172+
await setInitialModel(loadingState, TWENTY_ITEMS);
173+
174+
const sizes = await getContentScrollPosition();
175+
const newScrollPosition = sizes.scrollTop - 200;
176+
177+
// Re position the scroll
178+
await updateScrollPosition(newScrollPosition);
179+
180+
// Update message content
181+
await chatRef.callMethod(
182+
"updateLastMessage",
183+
{
184+
role: TWENTY_ITEMS[19].role,
185+
content: LONG_STRING + LONG_STRING + LONG_STRING + LONG_STRING
186+
} satisfies UpdateLastChatMessageArgTypes[0],
187+
"concat" satisfies UpdateLastChatMessageArgTypes[1]
188+
);
189+
await page.waitForChanges();
190+
191+
const sizesAfterUpdate = await getContentScrollPosition();
192+
193+
// Just in case, ensure the scrollHeight is larger than before
194+
expect(sizes.scrollHeight).toBeLessThan(sizesAfterUpdate.scrollHeight);
195+
expect(
196+
valuesAreEqualInMarginOfError(
197+
sizesAfterUpdate.scrollTop,
198+
newScrollPosition
199+
)
200+
).toBe(true);
201+
});
202+
};
203+
204+
const shouldKeepTheScrollAtTheBottomWhenUpdatingTheLastChatMessage = (
205+
loadingState: SmartGridDataState
206+
) => {
207+
it(`[loadingState = "${loadingState}"] if the scroll is at the bottom, it should keep it at the bottom when updating the last chat message`, async () => {
208+
await setInitialModel(loadingState, TWENTY_ITEMS);
209+
210+
const sizes = await getContentScrollPosition();
211+
212+
// Update message content
213+
await chatRef.callMethod(
214+
"updateLastMessage",
215+
{
216+
role: TWENTY_ITEMS[19].role,
217+
content: LONG_STRING + LONG_STRING + LONG_STRING + LONG_STRING
218+
} satisfies UpdateLastChatMessageArgTypes[0],
219+
"concat" satisfies UpdateLastChatMessageArgTypes[1]
220+
);
221+
await page.waitForChanges();
222+
223+
const sizesAfterUpdate = await getContentScrollPosition();
224+
225+
// Just in case, ensure the scrollHeight is larger than before
226+
expect(sizes.scrollHeight).toBeLessThan(sizesAfterUpdate.scrollHeight);
227+
expect(scrollIsAtTheBottom(sizesAfterUpdate)).toBe(true);
228+
});
229+
};
230+
231+
LOADING_STATE_VALUES.forEach(loadingState => {
232+
scrollPositionIsAtBottomOnInitialLoadTest(loadingState);
233+
shouldCorrectlySetTheScrollTopPositionTest(loadingState);
234+
235+
// TODO: Fix these tests
236+
if (loadingState !== "more-data-to-fetch") {
237+
shouldKeepTheScrollAtTheBottomWhenSwitchingModelsTest(loadingState);
238+
239+
// This test fails because the infinite scroll repositions the chat
240+
// message, even if no data was loaded
241+
shouldNotMoveTheScrollTopWhenUpdatingTheLastChatMessage(loadingState);
242+
}
243+
244+
shouldKeepTheScrollAtTheBottomWhenUpdatingTheLastChatMessage(loadingState);
245+
});
246+
});

0 commit comments

Comments
 (0)