diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index daa68cf86f..3050dac91c 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -53,6 +53,12 @@ class _MessageListPageState extends State { Widget build(BuildContext context) { return Scaffold( appBar: AppBar(title: MessageListAppBarTitle(narrow: widget.narrow)), + // TODO question for Vlad: for a stream view, should we set + // [backgroundColor] based on stream color, as in this frame: + // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=132%3A9684&mode=dev + // That's not obviously preferred over the default background that + // we matched to the Figma in 21dbae120. See another frame, which uses that: + // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=147%3A9088&mode=dev body: Builder( builder: (BuildContext context) => Center( child: Column(children: [ @@ -229,35 +235,33 @@ class _MessageListState extends State with PerAccountStoreAwareStat // TODO figure out text color -- web is supposedly hsl(0deg 0% 20%), // but seems much darker than that style: const TextStyle(color: Color.fromRGBO(0, 0, 0, 1)), - child: ColoredBox( - color: Colors.white, - // Pad the left and right insets, for small devices in landscape. - child: SafeArea( - // Don't let this be the place we pad the bottom inset. When there's - // no compose box, we want to let the message-list content pad it. - // TODO(#311) Remove as unnecessary if we do a bottom nav. - // The nav will pad the bottom inset, and an ancestor of this widget - // will have a `MediaQuery.removePadding` with `removeBottom: true`. - bottom: false, - - child: Center( - child: ConstrainedBox( - constraints: const BoxConstraints(maxWidth: 760), - child: NotificationListener( - onNotification: _metricsChanged, - child: Stack( - children: [ - _buildListView(context), - Positioned( - bottom: 0, - right: 0, - // TODO(#311) SafeArea shouldn't be needed if we have a - // bottom nav. That will pad the bottom inset. - child: SafeArea( - child: ScrollToBottomButton( - scrollController: scrollController, - visibleValue: _scrollToBottomVisibleValue))), - ]))))))); + // Pad the left and right insets, for small devices in landscape. + child: SafeArea( + // Don't let this be the place we pad the bottom inset. When there's + // no compose box, we want to let the message-list content pad it. + // TODO(#311) Remove as unnecessary if we do a bottom nav. + // The nav will pad the bottom inset, and an ancestor of this widget + // will have a `MediaQuery.removePadding` with `removeBottom: true`. + bottom: false, + + child: Center( + child: ConstrainedBox( + constraints: const BoxConstraints(maxWidth: 760), + child: NotificationListener( + onNotification: _metricsChanged, + child: Stack( + children: [ + _buildListView(context), + Positioned( + bottom: 0, + right: 0, + // TODO(#311) SafeArea shouldn't be needed if we have a + // bottom nav. That will pad the bottom inset. + child: SafeArea( + child: ScrollToBottomButton( + scrollController: scrollController, + visibleValue: _scrollToBottomVisibleValue))), + ])))))); } Widget _buildListView(context) { @@ -293,10 +297,10 @@ class _MessageListState extends State with PerAccountStoreAwareStat final valueKey = key as ValueKey; final index = model!.findItemWithMessageId(valueKey.value); if (index == -1) return null; - return length - 1 - (index - 1); + return length - 1 - (index - 2); }, controller: scrollController, - itemCount: length + 1, + itemCount: length + 2, // Setting reverse: true means the scroll starts at the bottom. // Flipping the indexes (in itemBuilder) means the start/bottom // has the latest messages. @@ -305,9 +309,13 @@ class _MessageListState extends State with PerAccountStoreAwareStat // TODO on new message when scrolled up, anchor scroll to what's in view reverse: true, itemBuilder: (context, i) { - if (i == 0) return MarkAsReadWidget(narrow: widget.narrow); + // 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 - 1)]; + final data = model!.items[length - 1 - (i - 2)]; switch (data) { case MessageListHistoryStartItem(): return const Center( @@ -326,7 +334,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat case MessageListMessageItem(): return MessageItem( key: ValueKey(data.message.id), - trailing: i == 1 ? const SizedBox(height: 8) : const SizedBox(height: 11), + trailingWhitespace: i == 1 ? 8 : 11, item: data); } }); @@ -403,26 +411,23 @@ class MarkAsReadWidget extends StatelessWidget { secondChild: SizedBox(width: double.infinity, // Design referenced from: // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=132-9684&mode=design&t=jJwHzloKJ0TMOG4M-0 - child: ColoredBox( - // TODO(#368): this should pull from stream color - color: Colors.transparent, - child: Padding( - // vertical padding adjusted for tap target height (48px) of button - padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)), - child: FilledButton.icon( - style: FilledButton.styleFrom( - backgroundColor: _UnreadMarker.color, - minimumSize: const Size.fromHeight(38), - textStyle: const TextStyle( - fontFamily: 'Source Sans 3', - fontSize: 18, - height: (23 / 18), - ).merge(weightVariableTextStyle(context)), - shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)), - ), - onPressed: () => _handlePress(context), - icon: const Icon(Icons.playlist_add_check), - label: Text(zulipLocalizations.markAsReadLabel(unreadCount))))))); + child: Padding( + // vertical padding adjusted for tap target height (48px) of button + padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)), + child: FilledButton.icon( + style: FilledButton.styleFrom( + backgroundColor: _UnreadMarker.color, + minimumSize: const Size.fromHeight(38), + textStyle: const TextStyle( + fontFamily: 'Source Sans 3', + fontSize: 18, + height: (23 / 18), + ).merge(weightVariableTextStyle(context)), + shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)), + ), + onPressed: () => _handlePress(context), + icon: const Icon(Icons.playlist_add_check), + label: Text(zulipLocalizations.markAsReadLabel(unreadCount)))))); } } @@ -446,11 +451,11 @@ class MessageItem extends StatelessWidget { const MessageItem({ super.key, required this.item, - this.trailing, + this.trailingWhitespace, }); final MessageListMessageItem item; - final Widget? trailing; + final double? trailingWhitespace; @override Widget build(BuildContext context) { @@ -460,10 +465,12 @@ class MessageItem extends StatelessWidget { header: RecipientHeader(message: message), child: _UnreadMarker( isRead: message.flags.contains(MessageFlag.read), - child: Column(children: [ - MessageWithPossibleSender(item: item), - if (trailing != null && item.isLastInBlock) trailing!, - ]))); + child: ColoredBox( + color: Colors.white, + child: Column(children: [ + MessageWithPossibleSender(item: item), + if (trailingWhitespace != null && item.isLastInBlock) SizedBox(height: trailingWhitespace!), + ])))); } } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 1de30a719c..c8555e2b74 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -81,7 +81,7 @@ void main() { testWidgets('basic', (tester) async { await setupMessageListPage(tester, foundOldest: false, messages: List.generate(200, (i) => eg.streamMessage(id: 950 + i, sender: eg.selfUser))); - check(itemCount(tester)).equals(202); + check(itemCount(tester)).equals(203); // Fling-scroll upward... await tester.fling(find.byType(MessageListPage), const Offset(0, 300), 8000); @@ -94,7 +94,7 @@ void main() { await tester.pump(Duration.zero); // Allow a frame for the response to arrive. // Now we have more messages. - check(itemCount(tester)).equals(302); + check(itemCount(tester)).equals(303); }); testWidgets('observe double-fetch glitch', (tester) async {