Skip to content

Commit ad2e892

Browse files
Chat: the scroll position shouldn't change when a new message is rendered from the other participant (#28461)
1 parent 097faf6 commit ad2e892

File tree

7 files changed

+167
-46
lines changed

7 files changed

+167
-46
lines changed

packages/devextreme/js/__internal/ui/chat/messagelist.ts

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { noop } from '@js/core/utils/common';
99
import dateUtils from '@js/core/utils/date';
1010
import dateSerialization from '@js/core/utils/date_serialization';
1111
import { isElementInDom } from '@js/core/utils/dom';
12-
import { getHeight } from '@js/core/utils/size';
1312
import { isDate, isDefined } from '@js/core/utils/type';
1413
import type { Format } from '@js/localization';
1514
import dateLocalization from '@js/localization/date';
@@ -393,34 +392,56 @@ class MessageList extends Widget<Properties> {
393392
}
394393

395394
_renderMessage(message: Message): void {
396-
const { author, timestamp } = message;
397-
398-
const lastMessageGroup = this._getLastMessageGroup();
395+
const { timestamp } = message;
399396
const shouldCreateDayHeader = this._shouldAddDayHeader(timestamp);
400397

401-
if (lastMessageGroup) {
402-
const { items } = lastMessageGroup.option();
403-
const lastMessageGroupItem = items[items.length - 1];
404-
const lastMessageGroupUserId = lastMessageGroupItem.author?.id;
405-
const isTimeoutExceeded = this._isTimeoutExceeded(lastMessageGroupItem, message);
406-
407-
if (author?.id === lastMessageGroupUserId && !isTimeoutExceeded && !shouldCreateDayHeader) {
408-
lastMessageGroup.renderMessage(message);
398+
if (shouldCreateDayHeader) {
399+
this._createDayHeader(timestamp);
400+
this._renderMessageIntoGroup(message);
401+
return;
402+
}
409403

410-
this._scrollDownContent();
404+
const lastMessageGroup = this._getLastMessageGroup();
411405

412-
return;
413-
}
406+
if (!lastMessageGroup) {
407+
this._renderMessageIntoGroup(message);
408+
return;
414409
}
415410

416-
if (shouldCreateDayHeader) {
417-
this._createDayHeader(timestamp);
411+
const lastMessageGroupMessage = this._getLastMessageGroupItem(lastMessageGroup);
412+
const isTimeoutExceeded = this._isTimeoutExceeded(lastMessageGroupMessage, message);
413+
414+
if (this._isSameAuthor(message, lastMessageGroupMessage) && !isTimeoutExceeded) {
415+
this._renderMessageIntoGroup(message, lastMessageGroup);
416+
return;
418417
}
419418

420-
this._createMessageGroupComponent([message], author?.id);
421-
this._setLastMessageGroupClasses();
419+
this._renderMessageIntoGroup(message);
420+
}
422421

423-
this._scrollDownContent();
422+
_getLastMessageGroupItem(lastMessageGroup: MessageGroup): Message {
423+
const { items } = lastMessageGroup.option();
424+
425+
return items[items.length - 1];
426+
}
427+
428+
_isSameAuthor(lastMessageGroupMessage: Message, message: Message): boolean {
429+
return lastMessageGroupMessage.author?.id === message.author?.id;
430+
}
431+
432+
_renderMessageIntoGroup(message: Message, messageGroup?: MessageGroup): void {
433+
const { author } = message;
434+
435+
this._setIsReachedBottom();
436+
437+
if (messageGroup) {
438+
messageGroup.renderMessage(message);
439+
} else {
440+
this._createMessageGroupComponent([message], author?.id);
441+
this._setLastMessageGroupClasses();
442+
}
443+
444+
this._processScrollDownContent(this._isCurrentUser(author?.id));
424445
}
425446

426447
_getMessageData(message: Element): Message {
@@ -571,19 +592,12 @@ class MessageList extends Widget<Properties> {
571592
}
572593

573594
_setIsReachedBottom(): void {
574-
const contentHeight = getHeight(this._scrollView.content());
575595
// @ts-expect-error
576-
const containerHeight = getHeight(this._scrollView.container());
577-
const heightDifference = Math.floor(contentHeight - containerHeight);
578-
const scrollOffsetTop = this._scrollView.scrollOffset()?.top ?? 0;
579-
580-
const isBottomReached = heightDifference <= scrollOffsetTop;
581-
582-
this._isBottomReached = isBottomReached;
596+
this._isBottomReached = this._scrollView.isBottomReached();
583597
}
584598

585-
_processScrollDownContent(): void {
586-
if (this._isBottomReached) {
599+
_processScrollDownContent(shouldForceProcessing = false): void {
600+
if (this._isBottomReached || shouldForceProcessing) {
587601
this._scrollDownContent();
588602
}
589603

packages/devextreme/js/__internal/ui/scroll_view/m_scroll_view.native.pull_down.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ const PullDownNativeScrollViewStrategy = NativeStrategy.inherit({
165165
},
166166

167167
_isReachBottom() {
168-
return this._reachBottomEnabled && Math.round(this._bottomBoundary + Math.floor(this._location)) <= 1;
168+
return this._reachBottomEnabled && this.isBottomReached();
169+
},
170+
171+
isBottomReached() {
172+
return Math.round(this._bottomBoundary + Math.floor(this._location)) <= 1;
169173
},
170174

171175
_reachBottom() {

packages/devextreme/js/__internal/ui/scroll_view/m_scroll_view.native.swipe_down.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,11 @@ const SwipeDownNativeScrollViewStrategy = NativeStrategy.inherit({
181181
},
182182

183183
_isReachBottom() {
184-
return this._reachBottomEnabled && Math.round(this._bottomBoundary + Math.floor(this._location)) <= 1;
184+
return this._reachBottomEnabled && this.isBottomReached();
185+
},
186+
187+
isBottomReached() {
188+
return Math.round(this._bottomBoundary + Math.floor(this._location)) <= 1;
185189
},
186190

187191
_reachBottom() {

packages/devextreme/js/__internal/ui/scroll_view/m_scroll_view.simulated.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,13 @@ const ScrollViewScroller = Scroller.inherit({
107107
},
108108

109109
_isReachBottom() {
110+
return this._reachBottomEnabled && this.isBottomReached();
111+
},
112+
113+
isBottomReached() {
110114
const containerEl = this._$container.get(0);
111115

112-
return this._reachBottomEnabled && Math.round(this._bottomBoundary - Math.ceil(containerEl.scrollTop)) <= 1;
116+
return Math.round(this._bottomBoundary - Math.ceil(containerEl.scrollTop)) <= 1;
113117
},
114118

115119
_scrollComplete() {
@@ -322,6 +326,10 @@ const SimulatedScrollViewStrategy = SimulatedStrategy.inherit({
322326
return location;
323327
},
324328

329+
isBottomReached() {
330+
return this._scrollers.vertical.isBottomReached();
331+
},
332+
325333
dispose() {
326334
each(this._scrollers, function () {
327335
this.dispose();

packages/devextreme/js/__internal/ui/scroll_view/m_scroll_view.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const scrollViewServerConfig = {
4040
release: noop,
4141
refresh: noop,
4242
scrollOffset: () => ({ top: 0, left: 0 }),
43+
isBottomReached: () => false,
4344
_optionChanged(args) {
4445
if (args.name !== 'onUpdated') {
4546
return this.callBase.apply(this, arguments);
@@ -308,6 +309,10 @@ const ScrollView = Scrollable.inherit(isServerSide ? scrollViewServerConfig : {
308309
this._unlock();
309310
},
310311

312+
isBottomReached() {
313+
return this._strategy.isBottomReached();
314+
},
315+
311316
_dispose() {
312317
this._strategy.dispose();
313318
this.callBase();

packages/devextreme/testing/tests/DevExpress.ui.widgets/chatParts/messageGroup.tests.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import $ from 'jquery';
22

33
import MessageGroup from '__internal/ui/chat/messagegroup';
4-
import MessageBubble from '__internal/ui/chat/messagebubble';
54
import ChatAvatar from '__internal/ui/chat/avatar';
65
import dateLocalization from 'localization/date';
76

packages/devextreme/testing/tests/DevExpress.ui.widgets/chatParts/messageList.tests.js

Lines changed: 99 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,7 @@ QUnit.module('MessageList', () => {
11551155
});
11561156

11571157
QUnit.test('should be scrolled down if typingUsers changed at runtime if scroll position at the bottom', function(assert) {
1158+
const done = assert.async();
11581159
this.reinit({
11591160
width: 300,
11601161
height: 500,
@@ -1165,12 +1166,16 @@ QUnit.module('MessageList', () => {
11651166

11661167
assert.roughEqual(scrollTopBefore, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom before updating typingUsers');
11671168

1168-
this.instance.option({ typingUsers: [{ name: 'User' }] });
1169+
setTimeout(() => {
1170+
this.instance.option({ typingUsers: [{ name: 'User' }] });
11691171

1170-
const scrollTop = this.getScrollView().scrollTop();
1172+
const scrollTop = this.getScrollView().scrollTop();
1173+
1174+
assert.notEqual(scrollTop, 0, 'scroll position should not be 0 after items are updated at runtime');
1175+
assert.roughEqual(scrollTop, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom after typingUsers are updated at runtime');
11711176

1172-
assert.notEqual(scrollTop, 0, 'scroll position should not be 0 after items are updated at runtime');
1173-
assert.roughEqual(scrollTop, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom after typingUsers are updated at runtime');
1177+
done();
1178+
});
11741179
});
11751180

11761181
QUnit.test('should not be scroll down if typingUsers changed at runtime if scroll position not at the bottom', function(assert) {
@@ -1227,10 +1232,9 @@ QUnit.module('MessageList', () => {
12271232
[MOCK_CURRENT_USER_ID, MOCK_COMPANION_USER_ID].forEach(id => {
12281233
const isCurrentUser = id === MOCK_CURRENT_USER_ID;
12291234

1230-
QUnit.test(`ScrollView should be scrolled down after render ${isCurrentUser ? 'current user' : 'companion'} message`, function(assert) {
1235+
QUnit.test(`should be scrolled down after render ${isCurrentUser ? 'current user' : 'companion'} message`, function(assert) {
12311236
const done = assert.async();
1232-
assert.expect(2);
1233-
const items = generateMessages(31);
1237+
const items = generateMessages(52);
12341238

12351239
this.reinit({
12361240
width: 300,
@@ -1245,14 +1249,97 @@ QUnit.module('MessageList', () => {
12451249
text: 'NEW MESSAGE',
12461250
};
12471251

1248-
this.instance.option('items', [...items, newMessage]);
1252+
const scrollTopBefore = this.getScrollView().scrollTop();
1253+
assert.roughEqual(scrollTopBefore, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom before render new message');
12491254

12501255
setTimeout(() => {
1251-
const scrollTop = this.getScrollView().scrollTop();
1256+
this.instance.option('items', [...items, newMessage]);
12521257

1253-
assert.notEqual(scrollTop, 0, 'scroll position should not be 0 after a new message is rendered');
1254-
assert.roughEqual(scrollTop, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom after rendering the new message');
1255-
done();
1258+
setTimeout(() => {
1259+
const scrollTop = this.getScrollView().scrollTop();
1260+
1261+
assert.notEqual(scrollTop, 0, 'scroll position should not be 0 after a new message is rendered');
1262+
assert.roughEqual(scrollTop, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom after rendering the new message');
1263+
done();
1264+
});
1265+
});
1266+
});
1267+
});
1268+
1269+
QUnit.test('should be scroll down after render current user message if scroll position not at the bottom', function(assert) {
1270+
const done = assert.async();
1271+
const items = generateMessages(52);
1272+
1273+
this.reinit({
1274+
width: 300,
1275+
height: 500,
1276+
items,
1277+
currentUserId: MOCK_CURRENT_USER_ID,
1278+
});
1279+
1280+
const author = { id: MOCK_CURRENT_USER_ID };
1281+
const newMessage = {
1282+
author,
1283+
timestamp: NOW,
1284+
text: 'NEW MESSAGE',
1285+
};
1286+
setTimeout(() => {
1287+
const initialScrollTop = this.getScrollOffsetMax() - 100;
1288+
this.getScrollView().scrollTo({ top: initialScrollTop });
1289+
1290+
setTimeout(() => {
1291+
const scrollTopBefore = this.getScrollView().scrollTop();
1292+
assert.roughEqual(scrollTopBefore, this.getScrollOffsetMax() - 100, 1, 'scroll position should not be at the bottom before rendering the message');
1293+
1294+
setTimeout(() => {
1295+
this.instance.option('items', [...items, newMessage]);
1296+
1297+
setTimeout(() => {
1298+
const scrollTop = this.getScrollView().scrollTop();
1299+
1300+
assert.notEqual(scrollTop, 0, 'scroll position should not be 0 after a new message is rendered');
1301+
assert.roughEqual(scrollTop, this.getScrollOffsetMax(), 1, 'scroll position should be at the bottom after rendering the new message');
1302+
done();
1303+
});
1304+
});
1305+
});
1306+
});
1307+
});
1308+
1309+
QUnit.test('should not be scroll down after render companion message if scroll position not at the bottom', function(assert) {
1310+
const done = assert.async();
1311+
const items = generateMessages(52);
1312+
1313+
this.reinit({
1314+
width: 300,
1315+
height: 500,
1316+
items,
1317+
currentUserId: MOCK_CURRENT_USER_ID
1318+
});
1319+
1320+
const author = { id: MOCK_COMPANION_USER_ID };
1321+
const newMessage = {
1322+
author,
1323+
timestamp: NOW,
1324+
text: 'NEW MESSAGE',
1325+
};
1326+
setTimeout(() => {
1327+
this.getScrollView().scrollBy({ top: -100 });
1328+
setTimeout(() => {
1329+
1330+
const scrollTopBefore = this.getScrollView().scrollTop();
1331+
assert.roughEqual(scrollTopBefore, this.getScrollOffsetMax() - 100, 1, 'scroll position should not be at the bottom before rendering the message');
1332+
1333+
setTimeout(() => {
1334+
this.instance.option('items', [...items, newMessage]);
1335+
1336+
const scrollTop = this.getScrollView().scrollTop();
1337+
1338+
assert.notEqual(scrollTop, 0, 'scroll position should not be 0 after a new message is rendered');
1339+
assert.roughEqual(scrollTop, scrollTopBefore, 1, 'scroll position should be at the bottom after rendering the new message');
1340+
done();
1341+
1342+
});
12561343
});
12571344
});
12581345
});

0 commit comments

Comments
 (0)