diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index b0783202a1..c22dd6314f 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -300,6 +300,49 @@ class _MessageListState extends State with PerAccountStoreAwareStat Widget _buildListView(BuildContext context) { final length = model!.items.length; const centerSliverKey = ValueKey('center sliver'); + + Widget sliver = SliverStickyHeaderList( + headerPlacement: HeaderPlacement.scrollingStart, + delegate: SliverChildBuilderDelegate( + // To preserve state across rebuilds for individual [MessageItem] + // widgets as the size of [MessageListView.items] changes we need + // to match old widgets by their key to their new position in + // the list. + // + // The keys are of type [ValueKey] with a value of [Message.id] + // and here we use a O(log n) binary search method. This could + // be improved but for now it only triggers for materialized + // widgets. As a simple test, flinging through Combined feed in + // CZO on a Pixel 5, this only runs about 10 times per rebuild + // and the timing for each call is <100 microseconds. + // + // Non-message items (e.g., start and end markers) that do not + // have state that needs to be preserved have not been given keys + // and will not trigger this callback. + findChildIndexCallback: (Key key) { + final valueKey = key as ValueKey; + final index = model!.findItemWithMessageId(valueKey.value); + if (index == -1) return null; + return length - 1 - (index - 2); + }, + childCount: length + 2, + (context, i) { + // To reinforce that the end of the feed has been reached: + // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603 + if (i == 0) return const SizedBox(height: 36); + + if (i == 1) return MarkAsReadWidget(narrow: widget.narrow); + + final data = model!.items[length - 1 - (i - 2)]; + return _buildItem(data, i); + })); + + if (widget.narrow is CombinedFeedNarrow) { + // TODO(#311) If we have a bottom nav, it will pad the bottom + // inset, and this shouldn't be necessary + sliver = SliverSafeArea(sliver: sliver); + } + return CustomScrollView( // TODO: Offer `ScrollViewKeyboardDismissBehavior.interactive` (or // similar) if that is ever offered: @@ -318,41 +361,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat center: centerSliverKey, slivers: [ - SliverStickyHeaderList( - headerPlacement: HeaderPlacement.scrollingStart, - delegate: SliverChildBuilderDelegate( - // To preserve state across rebuilds for individual [MessageItem] - // widgets as the size of [MessageListView.items] changes we need - // to match old widgets by their key to their new position in - // the list. - // - // The keys are of type [ValueKey] with a value of [Message.id] - // and here we use a O(log n) binary search method. This could - // be improved but for now it only triggers for materialized - // widgets. As a simple test, flinging through Combined feed in - // CZO on a Pixel 5, this only runs about 10 times per rebuild - // and the timing for each call is <100 microseconds. - // - // Non-message items (e.g., start and end markers) that do not - // have state that needs to be preserved have not been given keys - // and will not trigger this callback. - findChildIndexCallback: (Key key) { - final valueKey = key as ValueKey; - final index = model!.findItemWithMessageId(valueKey.value); - if (index == -1) return null; - return length - 1 - (index - 2); - }, - childCount: length + 2, - (context, i) { - // To reinforce that the end of the feed has been reached: - // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603 - if (i == 0) return const SizedBox(height: 36); - - if (i == 1) return MarkAsReadWidget(narrow: widget.narrow); - - final data = model!.items[length - 1 - (i - 2)]; - return _buildItem(data, i); - })), + sliver, // This is a trivial placeholder that occupies no space. Its purpose is // to have the key that's passed to [ScrollView.center], and so to cause diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index aa7cd1fa3f..7f25cb10d4 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -86,6 +86,22 @@ void main() { return scrollView.controller; } + group('presents message content appropriately', () { + // regression test for https://github.com/zulip/zulip-flutter/issues/736 + testWidgets('content in "Combined feed" not asked to consume insets (including bottom)', (tester) async { + const fakePadding = FakeViewPadding(left: 10, top: 10, right: 10, bottom: 10); + tester.view.viewInsets = fakePadding; + tester.view.padding = fakePadding; + + await setupMessageListPage(tester, narrow: const CombinedFeedNarrow(), + messages: [eg.streamMessage(content: ContentExample.codeBlockPlain.html)]); + + final element = tester.element(find.byType(CodeBlock)); + final padding = MediaQuery.of(element).padding; + check(padding).equals(EdgeInsets.zero); + }); + }); + group('fetch older messages on scroll', () { int? itemCount(WidgetTester tester) => tester.widget(find.byType(CustomScrollView)).semanticChildCount;