Skip to content

Commit c5e6957

Browse files
committed
msglist [nfc]: Handle start markers within widget code, not model
The view-model already exposes flags that express all the same information. By leaving these markers out of the list of items, we can save ourselves a bunch of stateful updates.
1 parent 4b69adf commit c5e6957

File tree

3 files changed

+17
-64
lines changed

3 files changed

+17
-64
lines changed

lib/model/message_list.dart

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -63,21 +63,6 @@ class MessageListMessageItem extends MessageListMessageBaseItem {
6363
});
6464
}
6565

66-
/// Indicates the app is loading more messages at the top.
67-
// TODO(#80): or loading at the bottom, by adding a [MessageListDirection.newer]
68-
class MessageListLoadingItem extends MessageListItem {
69-
final MessageListDirection direction;
70-
71-
const MessageListLoadingItem(this.direction);
72-
}
73-
74-
enum MessageListDirection { older }
75-
76-
/// Indicates we've reached the oldest message in the narrow.
77-
class MessageListHistoryStartItem extends MessageListItem {
78-
const MessageListHistoryStartItem();
79-
}
80-
8166
/// The sequence of messages in a message list, and how to display them.
8267
///
8368
/// This comprises much of the guts of [MessageListView].
@@ -161,11 +146,6 @@ mixin _MessageSequence {
161146

162147
static int _compareItemToMessageId(MessageListItem item, int messageId) {
163148
switch (item) {
164-
case MessageListHistoryStartItem(): return -1;
165-
case MessageListLoadingItem():
166-
switch (item.direction) {
167-
case MessageListDirection.older: return -1;
168-
}
169149
case MessageListRecipientHeaderItem(:var message):
170150
case MessageListDateSeparatorItem(:var message):
171151
if (message.id == null) return 1; // TODO(#1441): test
@@ -328,37 +308,12 @@ mixin _MessageSequence {
328308
showSender: !canShareSender, isLastInBlock: true));
329309
}
330310

331-
/// Update [items] to include markers at start and end as appropriate.
332-
void _updateEndMarkers() {
333-
assert(fetched);
334-
assert(!(fetchingOlder && fetchOlderCoolingDown));
335-
final effectiveFetchingOlder = fetchingOlder || fetchOlderCoolingDown;
336-
assert(!(effectiveFetchingOlder && haveOldest));
337-
final startMarker = switch ((effectiveFetchingOlder, haveOldest)) {
338-
(true, _) => const MessageListLoadingItem(MessageListDirection.older),
339-
(_, true) => const MessageListHistoryStartItem(),
340-
(_, _) => null,
341-
};
342-
final hasStartMarker = switch (items.firstOrNull) {
343-
MessageListLoadingItem() => true,
344-
MessageListHistoryStartItem() => true,
345-
_ => false,
346-
};
347-
switch ((startMarker != null, hasStartMarker)) {
348-
case (true, true): items[0] = startMarker!;
349-
case (true, _ ): items.addFirst(startMarker!);
350-
case (_, true): items.removeFirst();
351-
case (_, _ ): break;
352-
}
353-
}
354-
355311
/// Recompute [items] from scratch, based on [messages], [contents], and flags.
356312
void _reprocessAll() {
357313
items.clear();
358314
for (var i = 0; i < messages.length; i++) {
359315
_processMessage(i);
360316
}
361-
_updateEndMarkers();
362317
}
363318
}
364319

@@ -508,7 +463,6 @@ class MessageListView with ChangeNotifier, _MessageSequence {
508463
}
509464
_fetched = true;
510465
_haveOldest = result.foundOldest;
511-
_updateEndMarkers();
512466
notifyListeners();
513467
}
514468

@@ -555,7 +509,6 @@ class MessageListView with ChangeNotifier, _MessageSequence {
555509
|| (narrow as TopicNarrow).with_ == null);
556510
assert(messages.isNotEmpty);
557511
_fetchingOlder = true;
558-
_updateEndMarkers();
559512
notifyListeners();
560513
final generation = this.generation;
561514
bool hasFetchError = false;
@@ -601,13 +554,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
601554
.wait().then((_) {
602555
if (this.generation != generation) return;
603556
_fetchOlderCoolingDown = false;
604-
_updateEndMarkers();
605557
notifyListeners();
606558
}));
607559
} else {
608560
_fetchOlderCooldownBackoffMachine = null;
609561
}
610-
_updateEndMarkers();
611562
notifyListeners();
612563
}
613564
}

lib/widgets/message_list.dart

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,10 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
606606
if (childIndex < 0) return null;
607607
return childIndex;
608608
},
609-
childCount: topItems,
609+
childCount: topItems + 1,
610610
(context, childIndex) {
611+
if (childIndex == topItems) return _buildStartCap();
612+
611613
final itemIndex = totalItems - 1 - (childIndex + bottomItems);
612614
final data = model.items[itemIndex];
613615
final item = _buildItem(data);
@@ -688,12 +690,21 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
688690
]);
689691
}
690692

693+
Widget _buildStartCap() {
694+
// These assertions are invariants of [MessageListView].
695+
assert(!(model.fetchingOlder && model.fetchOlderCoolingDown));
696+
final effectiveFetchingOlder =
697+
model.fetchingOlder || model.fetchOlderCoolingDown;
698+
assert(!(model.haveOldest && effectiveFetchingOlder));
699+
return switch ((effectiveFetchingOlder, model.haveOldest)) {
700+
(true, _) => const _MessageListLoadingMore(),
701+
(_, true) => const _MessageListHistoryStart(),
702+
(_, _) => const SizedBox.shrink(),
703+
};
704+
}
705+
691706
Widget _buildItem(MessageListItem data) {
692707
switch (data) {
693-
case MessageListHistoryStartItem():
694-
return const _MessageListHistoryStart();
695-
case MessageListLoadingItem():
696-
return const _MessageListLoadingMore();
697708
case MessageListRecipientHeaderItem():
698709
final header = RecipientHeader(message: data.message, narrow: widget.narrow);
699710
return StickyHeaderItem(allowOverflow: true,

test/model/message_list_test.dart

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,7 +1791,6 @@ void main() {
17911791
// We check showSender has the right values in [checkInvariants],
17921792
// but to make this test explicit:
17931793
check(model.items).deepEquals(<void Function(Subject<Object?>)>[
1794-
(it) => it.isA<MessageListHistoryStartItem>(),
17951794
(it) => it.isA<MessageListRecipientHeaderItem>(),
17961795
(it) => it.isA<MessageListMessageItem>().showSender.isTrue(),
17971796
(it) => it.isA<MessageListMessageItem>().showSender.isFalse(),
@@ -1964,12 +1963,6 @@ void checkInvariants(MessageListView model) {
19641963
}
19651964

19661965
int i = 0;
1967-
if (model.haveOldest) {
1968-
check(model.items[i++]).isA<MessageListHistoryStartItem>();
1969-
}
1970-
if (model.fetchingOlder || model.fetchOlderCoolingDown) {
1971-
check(model.items[i++]).isA<MessageListLoadingItem>();
1972-
}
19731966
for (int j = 0; j < model.messages.length; j++) {
19741967
bool forcedShowSender = false;
19751968
if (j == 0
@@ -1991,9 +1984,7 @@ void checkInvariants(MessageListView model) {
19911984
i == model.items.length || switch (model.items[i]) {
19921985
MessageListMessageItem()
19931986
|| MessageListDateSeparatorItem() => false,
1994-
MessageListRecipientHeaderItem()
1995-
|| MessageListHistoryStartItem()
1996-
|| MessageListLoadingItem() => true,
1987+
MessageListRecipientHeaderItem() => true,
19971988
});
19981989
}
19991990
check(model.items).length.equals(i);

0 commit comments

Comments
 (0)