Skip to content

Commit e5983aa

Browse files
committed
msglist: Simplify computing semanticChildCount
This has already gotten out of sync with the total number of children in the list, after 5c70c76 added a TypingStatusWidget child while leaving this unchanged. On the other hand, its spec isn't to be the total number of children: it's to be the total number of children "that will contribute semantic information". So the spacer shouldn't count, and probably TypingStatusWidget and MarkAsReadWidget shouldn't either when they don't show anything. (Currently it effectively counts the spacer and MarkAsReadWidget, or one could retcon that to TypingStatusWidget and MarkAsReadWidget.) Further: > If the number is unknown or unbounded this should be left unset > or set to null. So it seems like when `haveOldest` is false, this should be null. Meanwhile, the exact value here seems to have little effect. I just tried the app out in TalkBack, and there are some other issues (when you scroll around, there's some glitchy behavior with focus moving to the sticky header and back, and even with the scroll position sometimes abruptly jumping back; and I couldn't find a way to navigate by a whole message or item at a time, though that might have been my inexperience with TalkBack) but it doesn't ever quote the number of list items as far as I could tell, and isn't bothered by this being a few more or less than the spec'd number. So for the moment let's do something simple: don't count any of the extra children at the end, the ones that are often or always empty. This also makes a couple of tests a bit simpler to follow. Thanks to Zixuan for raising the question. Discussion here: #1468 (comment)
1 parent 231b054 commit e5983aa

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

lib/widgets/message_list.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
641641
},
642642

643643
controller: scrollController,
644-
semanticChildCount: length + 2,
644+
semanticChildCount: length, // TODO(#537): what's the right value for this?
645645
center: centerSliverKey,
646646
paintOrder: SliverPaintOrder.firstIsTop,
647647

test/widgets/message_list_test.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ void main() {
375375
testWidgets('basic', (tester) async {
376376
await setupMessageListPage(tester, foundOldest: false,
377377
messages: List.generate(300, (i) => eg.streamMessage(id: 950 + i, sender: eg.selfUser)));
378-
check(itemCount(tester)).equals(303);
378+
check(itemCount(tester)).equals(301);
379379

380380
// Fling-scroll upward...
381381
await tester.fling(find.byType(MessageListPage), const Offset(0, 300), 8000);
@@ -388,7 +388,7 @@ void main() {
388388
await tester.pump(Duration.zero); // Allow a frame for the response to arrive.
389389

390390
// Now we have more messages.
391-
check(itemCount(tester)).equals(403);
391+
check(itemCount(tester)).equals(401);
392392
});
393393

394394
testWidgets('observe double-fetch glitch', (tester) async {
@@ -429,7 +429,7 @@ void main() {
429429
...List.generate(100, (i) => eg.streamMessage(id: 1302 + i)),
430430
]);
431431
final lastRequest = connection.lastRequest;
432-
check(itemCount(tester)).equals(404);
432+
check(itemCount(tester)).equals(402);
433433

434434
// Fling-scroll upward...
435435
await tester.fling(find.byType(MessageListPage), const Offset(0, 300), 8000);

0 commit comments

Comments
 (0)