From 6512c1e36e800b53aa15cbf5a83165144f153ebc Mon Sep 17 00:00:00 2001 From: E-m-i-n-e-n-c-e Date: Fri, 17 Jan 2025 22:29:04 +0530 Subject: [PATCH] Finished the job but idk why the tests are failing.Have to troubleshoot --- lib/model/message_list.dart | 37 ++++----- lib/widgets/message_list.dart | 151 ++++++++++++++++++++++++++-------- 2 files changed, 133 insertions(+), 55 deletions(-) diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index c5567251aa..5da716c56a 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -167,7 +167,8 @@ mixin _MessageSequence { /// before, between, or after the messages. /// /// This information is completely derived from [messages] and - /// the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown]. + /// the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown] + /// and [haveNewest], [fetchingNewer] and [fetchNewerCoolingDown]. /// It exists as an optimization, to memoize that computation. final QueueList items = QueueList(); @@ -476,16 +477,20 @@ bool _sameDay(DateTime date1, DateTime date2) { /// * Add listeners with [addListener]. /// * Fetch messages with [fetchInitial]. When the fetch completes, this object /// will notify its listeners (as it will any other time the data changes.) -/// * Fetch more messages as needed with [fetchOlder]. +/// * Fetch more messages as needed with [fetchOlder] or [fetchNewer]. /// * On reassemble, call [reassemble]. /// * When the object will no longer be used, call [dispose] to free /// resources on the [PerAccountStore]. class MessageListView with ChangeNotifier, _MessageSequence { - MessageListView._({required this.store, required this.narrow}); + MessageListView._({required this.store, required this.narrow, this.anchorMessageId}); + // Anchor message ID is used to fetch messages from a specific point in the list. + // It is set when the user navigates to a message list page with a specific anchor message. + int? anchorMessageId; + int? get anchorIndex => anchorMessageId != null ? findItemWithMessageId(anchorMessageId!) : null; factory MessageListView.init( - {required PerAccountStore store, required Narrow narrow}) { - final view = MessageListView._(store: store, narrow: narrow); + {required PerAccountStore store, required Narrow narrow, int? anchorMessageId}) { + final view = MessageListView._(store: store, narrow: narrow, anchorMessageId: anchorMessageId); store.registerMessageList(view); return view; } @@ -564,15 +569,13 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } - /// The message ID to use as an anchor when fetching messages. - /// If null, the latest messages will be fetched. - int? anchorMessageId; + /// Fetch messages, starting from scratch. Future fetchInitial() async { // TODO(#80): fetch from anchor firstUnread, instead of newest - assert(!fetched && !haveOldest && !fetchingOlder && !fetchOlderCoolingDown); + assert(!fetched && !haveOldest && !fetchingOlder && !fetchOlderCoolingDown && !fetchingNewer && !fetchNewerCoolingDown && !haveNewest); assert(messages.isEmpty && contents.isEmpty); // TODO schedule all this in another isolate final generation = this.generation; @@ -585,9 +588,11 @@ class MessageListView with ChangeNotifier, _MessageSequence { ? kMessageListFetchBatchSize ~/ 2 // Fetch messages before and after anchor : kMessageListFetchBatchSize, // Fetch only older messages when no anchor numAfter: anchorMessageId != null - ? kMessageListFetchBatchSize ~/ 2 // Fetch messages before and after anchor - : 0, // Don't fetch newer messages when no anchor + ? kMessageListFetchBatchSize ~/2 // Fetch messages before and after anchor + : 0, // Don't fetch newer messages when no anchor ); + anchorMessageId ??= result.messages.last.id; + if (this.generation > generation) return; store.reconcileMessages(result.messages); store.recentSenders.handleMessages(result.messages); // TODO(#824) @@ -598,16 +603,11 @@ class MessageListView with ChangeNotifier, _MessageSequence { } _fetched = true; _haveOldest = result.foundOldest; + _haveNewest = result.foundNewest; _updateEndMarkers(); notifyListeners(); } - /// Initialize the view with a specific anchor message. - void setAnchorMessage(int messageId) { - anchorMessageId = messageId; - _reset(); - fetchInitial(); - } /// Fetch the next batch of older messages, if applicable. Future fetchOlder() async { @@ -720,8 +720,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { _insertAllMessages(messages.length, fetchedMessages); - // No foundNewest in API, assume we're caught up if we get fewer messages - _haveNewest = result.messages.length < kMessageListFetchBatchSize; + _haveNewest = result.foundNewest; } finally { if (this.generation == generation) { diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 8e139bee22..abed7727db 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -182,12 +182,12 @@ abstract class MessageListPageState { } class MessageListPage extends StatefulWidget { - const MessageListPage({super.key, required this.initNarrow}); - + const MessageListPage({super.key, required this.initNarrow, this.anchorMessageId}); + final int? anchorMessageId; static Route buildRoute({int? accountId, BuildContext? context, - required Narrow narrow}) { + required Narrow narrow, int? anchorMessageId}) { return MaterialAccountWidgetRoute(accountId: accountId, context: context, - page: MessageListPage(initNarrow: narrow)); + page: MessageListPage(initNarrow: narrow, anchorMessageId: anchorMessageId)); } /// The [MessageListPageState] above this context in the tree. @@ -302,7 +302,7 @@ class _MessageListPageState extends State implements MessageLis removeBottom: ComposeBox.hasComposeBox(narrow), child: Expanded( - child: MessageList(narrow: narrow, onNarrowChanged: _narrowChanged))), + child: MessageList(narrow: narrow, onNarrowChanged: _narrowChanged, anchorMessageId: widget.anchorMessageId))), if (ComposeBox.hasComposeBox(narrow)) ComposeBox(key: _composeBoxKey, narrow: narrow) ])))); @@ -443,11 +443,11 @@ const _kShortMessageHeight = 80; const kFetchMessagesBufferPixels = (kMessageListFetchBatchSize / 2) * _kShortMessageHeight; class MessageList extends StatefulWidget { - const MessageList({super.key, required this.narrow, required this.onNarrowChanged}); + const MessageList({super.key, required this.narrow, required this.onNarrowChanged, this.anchorMessageId}); final Narrow narrow; final void Function(Narrow newNarrow) onNarrowChanged; - + final int? anchorMessageId; @override State createState() => _MessageListState(); } @@ -456,6 +456,8 @@ class _MessageListState extends State with PerAccountStoreAwareStat MessageListView? model; final ScrollController scrollController = ScrollController(); final ValueNotifier _scrollToBottomVisibleValue = ValueNotifier(false); + List newItems = []; + List oldItems = []; @override void initState() { @@ -476,10 +478,14 @@ class _MessageListState extends State with PerAccountStoreAwareStat super.dispose(); } - void _initModel(PerAccountStore store) { - model = MessageListView.init(store: store, narrow: widget.narrow); + void _initModel(PerAccountStore store) async{ + model = MessageListView.init(store: store, narrow: widget.narrow, anchorMessageId: widget.anchorMessageId); model!.addListener(_modelChanged); - model!.fetchInitial(); + await model!.fetchInitial(); + setState(() { + oldItems = model!.items.sublist(0, model!.anchorIndex!+1); + newItems = model!.items.sublist(model!.anchorIndex!+1, model!.items.length); + }); } void _modelChanged() { @@ -488,10 +494,54 @@ class _MessageListState extends State with PerAccountStoreAwareStat // [PropagateMode.changeAll] or [PropagateMode.changeLater]. widget.onNarrowChanged(model!.narrow); } + + final previousLength = oldItems.length + newItems.length; + setState(() { + oldItems = model!.items.sublist(0, model!.anchorIndex!+1); + newItems = model!.items.sublist(model!.anchorIndex!+1, model!.items.length); // The actual state lives in the [MessageListView] model. // This method was called because that just changed. }); + + + // Auto-scroll when new messages arrive if we're already near the bottom + if (model!.items.length > previousLength && // New messages were added + scrollController.hasClients) { + // Use post-frame callback to ensure scroll metrics are up to date + WidgetsBinding.instance.addPostFrameCallback((_) async { + // This is to prevent auto-scrolling when fetching newer messages + if(model!.fetchingNewer || model!.fetchingOlder || model!.fetchNewerCoolingDown || model!.fetchOlderCoolingDown || !model!.haveNewest ){ + return; + } + + final viewportDimension = scrollController.position.viewportDimension; + final maxScrollExtent = scrollController.position.maxScrollExtent; + final currentScroll = scrollController.position.pixels; + + // If we're within 300px of the bottommost viewport, auto-scroll + if (maxScrollExtent - currentScroll - viewportDimension < 300) { + + final distance = scrollController.position.pixels; + final durationMsAtSpeedLimit = (1000 * distance / 8000).ceil(); + final durationMs = max(300, durationMsAtSpeedLimit); + + await scrollController.animateTo( + scrollController.position.maxScrollExtent, + duration: Duration(milliseconds: durationMs), + curve: Curves.ease); + + + + if (scrollController.position.pixels + 40 < scrollController.position.maxScrollExtent ) { + await scrollController.animateTo( + scrollController.position.maxScrollExtent, + duration: Duration(milliseconds: durationMs), + curve: Curves.ease); + } + } + }); + } } void _handleScrollMetrics(ScrollMetrics scrollMetrics) { @@ -510,6 +560,11 @@ class _MessageListState extends State with PerAccountStoreAwareStat // still not yet updated to account for the newly-added messages. model?.fetchOlder(); } + + // Check for fetching newer messages when near the bottom + if (scrollMetrics.extentAfter < kFetchMessagesBufferPixels) { + model?.fetchNewer(); + } } void _scrollChanged() { @@ -562,7 +617,8 @@ class _MessageListState extends State with PerAccountStoreAwareStat } Widget _buildListView(BuildContext context) { - final length = model!.items.length; + final length = oldItems.length; + final newLength = newItems.length; const centerSliverKey = ValueKey('center sliver'); Widget sliver = SliverStickyHeaderList( @@ -587,22 +643,32 @@ 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 - 3); + return length - 1 - index; }, - childCount: length + 3, + childCount: length, (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); - - if (i == 2) return TypingStatusWidget(narrow: widget.narrow); - - final data = model!.items[length - 1 - (i - 3)]; + final data = oldItems[length - 1 - i]; return _buildItem(data, i); })); + Widget newMessagesSliver = SliverStickyHeaderList( + headerPlacement: HeaderPlacement.scrollingStart, + delegate: SliverChildBuilderDelegate( + findChildIndexCallback: (Key key) { + final valueKey = key as ValueKey; + final index = model!.findItemWithMessageId(valueKey.value); + if (index == -1) return null; + return index-3; + }, + childCount: newLength+3, + (context, i) { + if (i == newLength) return TypingStatusWidget(narrow: widget.narrow); + if (i == newLength+1) return MarkAsReadWidget(narrow: widget.narrow); + if (i == newLength+2) return const SizedBox(height: 36); + final data = newItems[i]; + return _buildItem(data, i-newLength); + })); + if (!ComposeBox.hasComposeBox(widget.narrow)) { // TODO(#311) If we have a bottom nav, it will pad the bottom // inset, and this shouldn't be necessary @@ -623,16 +689,16 @@ class _MessageListState extends State with PerAccountStoreAwareStat controller: scrollController, semanticChildCount: length + 2, - anchor: 1.0, + anchor: 0.85, center: centerSliverKey, slivers: [ - 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 - // the above [SliverStickyHeaderList] to run from bottom to top. + sliver, // Main message list (grows upward) + // Center point - everything before this grows up, everything after grows down const SliverToBoxAdapter(key: centerSliverKey), + // Static widgets and new messages (will grow downward) + newMessagesSliver, // New messages list (will grow downward) + ]); } @@ -674,14 +740,28 @@ class ScrollToBottomButton extends StatelessWidget { final ValueNotifier visibleValue; final ScrollController scrollController; - Future _navigateToBottom() { + Future _navigateToBottom() async { + // Calculate initial scroll parameters final distance = scrollController.position.pixels; final durationMsAtSpeedLimit = (1000 * distance / 8000).ceil(); final durationMs = max(300, durationMsAtSpeedLimit); - return scrollController.animateTo( - 0, + + // Do a single scroll attempt with a completion check + await scrollController.animateTo( + scrollController.position.maxScrollExtent, duration: Duration(milliseconds: durationMs), curve: Curves.ease); + var count =1; + // Check if we actually reached bottom, if not try again + // This handles cases where content was loaded during scroll + while (scrollController.position.pixels + 40 < scrollController.position.maxScrollExtent) { + await scrollController.animateTo( + scrollController.position.maxScrollExtent, + duration: const Duration(milliseconds: 300), + curve: Curves.ease); + count++; + } + print("count: $count"); } @override @@ -728,6 +808,7 @@ class _TypingStatusWidgetState extends State with PerAccount } void _modelChanged() { + setState(() { // The actual state lives in [model]. // This method was called because that just changed. @@ -1358,11 +1439,9 @@ class MessageWithPossibleSender extends StatelessWidget { selfUserId: store.selfUserId), }; Navigator.push(context, - MessageListPage.buildRoute(context: context, narrow: narrow)); - final messageListState = context.findAncestorStateOfType<_MessageListState>(); - if (messageListState != null) { - messageListState.model?.setAnchorMessage(message.id); - } + MessageListPage.buildRoute(context: context, narrow: narrow, anchorMessageId: message.id)); + + } }, child: Padding(