Skip to content

Commit f180667

Browse files
msglist: Separate messages after 10-min gap, with a sender row
Fixes: #1773
1 parent 9cc6700 commit f180667

File tree

2 files changed

+79
-14
lines changed

2 files changed

+79
-14
lines changed

lib/model/message_list.dart

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,10 @@ mixin _MessageSequence {
459459
if (!messagesSameDay(prevMessageItem.message, message)) {
460460
items.add(MessageListDateSeparatorItem(message));
461461
canShareSender = false;
462+
} else if (prevMessageItem.message.senderId == message.senderId) {
463+
canShareSender = messagesWithinTenMinutes(prevMessage, message);
462464
} else {
463-
canShareSender = prevMessageItem.message.senderId == message.senderId;
465+
canShareSender = false;
464466
}
465467
}
466468
final item = buildItem(canShareSender);
@@ -568,6 +570,13 @@ bool messagesSameDay(MessageBase prevMessage, MessageBase message) {
568570
return true;
569571
}
570572

573+
@visibleForTesting
574+
bool messagesWithinTenMinutes(MessageBase prevMessage, MessageBase message) {
575+
final diffSeconds = (message.timestamp - prevMessage.timestamp).abs();
576+
if (diffSeconds <= 10 * 60) return true;
577+
return false;
578+
}
579+
571580
bool _sameDay(DateTime date1, DateTime date2) {
572581
if (date1.year != date2.year) return false;
573582
if (date1.month != date2.month) return false;

test/model/message_list_test.dart

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2948,7 +2948,14 @@ void main() {
29482948

29492949
final now = clock.now();
29502950
final t1 = eg.utcTimestamp(now.subtract(Duration(days: 1)));
2951-
final t2 = eg.utcTimestamp(now);
2951+
final t2 = eg.utcTimestamp(
2952+
now.subtract(Duration(days: 1))
2953+
.add(Duration(minutes: 1)));
2954+
final t3 = eg.utcTimestamp(
2955+
now.subtract(Duration(days: 1))
2956+
.add(Duration(minutes: 1))
2957+
.add(Duration(minutes: 10, seconds: 1)));
2958+
final t4 = eg.utcTimestamp(now);
29522959
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
29532960
Message streamMessage(int id, int timestamp, User sender) =>
29542961
eg.streamMessage(id: id, sender: sender,
@@ -2964,15 +2971,17 @@ void main() {
29642971
streamMessage(1, t1, eg.selfUser), // first message, so show sender
29652972
streamMessage(2, t1, eg.selfUser), // hide sender
29662973
streamMessage(3, t1, eg.otherUser), // no recipient header, but new sender
2967-
dmMessage(4, t1, eg.otherUser), // same sender, but new recipient
2968-
dmMessage(5, t2, eg.otherUser), // same sender/recipient, but new day
2974+
streamMessage(5, t2, eg.otherUser), // same sender, but within 10 mins of last message
2975+
streamMessage(6, t3, eg.otherUser), // same sender, after 10 mins of last message
2976+
dmMessage(7, t3, eg.otherUser), // same sender, but new recipient
2977+
dmMessage(8, t4, eg.otherUser), // same sender/recipient, but new day
29692978
]);
29702979
await prepareOutboxMessagesTo([
29712980
dmDestination([eg.selfUser, eg.otherUser]), // same day, but new sender
29722981
dmDestination([eg.selfUser, eg.otherUser]), // hide sender
29732982
]);
29742983
assert(
2975-
store.outboxMessages.values.every((message) => message.timestamp == t2));
2984+
store.outboxMessages.values.every((message) => message.timestamp == t4));
29762985
async.elapse(kLocalEchoDebounceDuration);
29772986

29782987
// We check showSender has the right values in [checkInvariants],
@@ -2982,6 +2991,8 @@ void main() {
29822991
(it) => it.isA<MessageListMessageItem>().showSender.isTrue(),
29832992
(it) => it.isA<MessageListMessageItem>().showSender.isFalse(),
29842993
(it) => it.isA<MessageListMessageItem>().showSender.isTrue(),
2994+
(it) => it.isA<MessageListMessageItem>().showSender.isFalse(),
2995+
(it) => it.isA<MessageListMessageItem>().showSender.isTrue(),
29852996
(it) => it.isA<MessageListRecipientHeaderItem>(),
29862997
(it) => it.isA<MessageListMessageItem>().showSender.isTrue(),
29872998
(it) => it.isA<MessageListDateSeparatorItem>(),
@@ -3072,14 +3083,14 @@ void main() {
30723083
});
30733084
});
30743085

3075-
test('messagesSameDay', () {
3076-
// These timestamps will differ depending on the timezone of the
3077-
// environment where the tests are run, in order to give the same results
3078-
// in the code under test which is also based on the ambient timezone.
3079-
// TODO(dart): It'd be great if tests could control the ambient timezone,
3080-
// so as to exercise cases like where local time falls back across midnight.
3081-
int timestampFromLocalTime(String date) => DateTime.parse(date).millisecondsSinceEpoch ~/ 1000;
3086+
// These timestamps will differ depending on the timezone of the
3087+
// environment where the tests are run, in order to give the same results
3088+
// in the code under test which is also based on the ambient timezone.
3089+
// TODO(dart): It'd be great if tests could control the ambient timezone,
3090+
// so as to exercise cases like where local time falls back across midnight.
3091+
int timestampFromLocalTime(String date) => DateTime.parse(date).millisecondsSinceEpoch ~/ 1000;
30823092

3093+
test('messagesSameDay', () {
30833094
const t111a = '2021-01-01 00:00:00';
30843095
const t111b = '2021-01-01 12:00:00';
30853096
const t111c = '2021-01-01 23:59:58';
@@ -3118,6 +3129,46 @@ void main() {
31183129
}
31193130
}
31203131
});
3132+
3133+
group('messagesWithinTenMinutes', () {
3134+
final stream = eg.stream();
3135+
void doTest(String time0, String time1, bool expected) {
3136+
test('$time0 $time1', () {
3137+
check(messagesWithinTenMinutes(
3138+
eg.streamMessage(stream: stream, topic: 'foo', timestamp: timestampFromLocalTime(time0)),
3139+
eg.streamMessage(stream: stream, topic: 'foo', timestamp: timestampFromLocalTime(time1)),
3140+
)).equals(expected);
3141+
check(messagesWithinTenMinutes(
3142+
eg.dmMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time0)),
3143+
eg.dmMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time1)),
3144+
)).equals(expected);
3145+
check(messagesWithinTenMinutes(
3146+
eg.streamOutboxMessage(timestamp: timestampFromLocalTime(time0)),
3147+
eg.streamOutboxMessage(timestamp: timestampFromLocalTime(time1)),
3148+
)).equals(expected);
3149+
check(messagesWithinTenMinutes(
3150+
eg.dmOutboxMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time0)),
3151+
eg.dmOutboxMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time1)),
3152+
)).equals(expected);
3153+
});
3154+
}
3155+
3156+
const time = '2021-01-01 00:30:00';
3157+
3158+
doTest('2021-01-01 00:19:59', time, false);
3159+
doTest('2021-01-01 00:20:00', time, true);
3160+
doTest('2021-01-01 00:20:01', time, true);
3161+
doTest('2021-01-01 00:25:00', time, true);
3162+
doTest('2021-01-01 00:29:59', time, true);
3163+
3164+
doTest(time, '2021-01-01 00:30:00', true);
3165+
3166+
doTest(time, '2021-01-01 00:30:01', true);
3167+
doTest(time, '2021-01-01 00:35:00', true);
3168+
doTest(time, '2021-01-01 00:39:59', true);
3169+
doTest(time, '2021-01-01 00:40:00', true);
3170+
doTest(time, '2021-01-01 00:40:01', false);
3171+
});
31213172
}
31223173

31233174
MessageListView? _lastModel;
@@ -3234,6 +3285,12 @@ void checkInvariants(MessageListView model) {
32343285
check(model.items[i++]).isA<MessageListDateSeparatorItem>()
32353286
.message.identicalTo(allMessages[j]);
32363287
forcedShowSender = true;
3288+
} else if (allMessages[j-1].senderId == allMessages[j].senderId) {
3289+
if (!messagesWithinTenMinutes(allMessages[j-1], allMessages[j])) {
3290+
forcedShowSender = true;
3291+
}
3292+
} else if (allMessages[j-1].senderId != allMessages[j].senderId) {
3293+
forcedShowSender = true;
32373294
}
32383295
if (j < model.messages.length) {
32393296
check(model.items[i]).isA<MessageListMessageItem>()
@@ -3244,8 +3301,7 @@ void checkInvariants(MessageListView model) {
32443301
.message.identicalTo(model.outboxMessages[j-model.messages.length]);
32453302
}
32463303
check(model.items[i++]).isA<MessageListMessageBaseItem>()
3247-
..showSender.equals(
3248-
forcedShowSender || allMessages[j].senderId != allMessages[j-1].senderId)
3304+
..showSender.equals(forcedShowSender)
32493305
..isLastInBlock.equals(
32503306
i == model.items.length || switch (model.items[i]) {
32513307
MessageListMessageItem()

0 commit comments

Comments
 (0)