msglist: Maintain _UnreadMarker animation state when MessageListView.…

…items changes size
zulip · Oct 30, 2023 · bdba1f6 · bdba1f6
1 parent e5d1c2a
commit bdba1f6
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 0 deletions.
diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -255,6 +255,27 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
         _ => ScrollViewKeyboardDismissBehavior.manual,
       },
 
+      // 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 All Messages 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;
+      },
       controller: scrollController,
       itemCount: length,
       // Setting reverse: true means the scroll starts at the bottom.

diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart
@@ -355,5 +355,57 @@ void main() {
         ..value.equals(0.0)
         ..status.equals(AnimationStatus.completed);
     });
+
+    testWidgets('animation state persistence', (WidgetTester tester) async {
+      // Check that _UnreadMarker maintains its in-progress animation
+      // as the number of items changes in MessageList. See
+      // findChildIndexCallback on StickyHeaderListView.builder for
+      //
+      final message = eg.streamMessage(flags: []);
+      await setupMessageListPage(tester, messages: [message]);
+      check(getAnimation(tester, message.id))
+        ..value.equals(1.0)
+        ..status.equals(AnimationStatus.dismissed);
+
+      store.handleEvent(UpdateMessageFlagsAddEvent(
+        id: 0,
+        flag: MessageFlag.read,
+        messages: [message.id],
+        all: false,
+      ));
+      await tester.pump(); // process handleEvent
+      check(getAnimation(tester, message.id))
+        ..value.equals(1.0)
+        ..status.equals(AnimationStatus.forward);
+
+      // run animation partially
+      await tester.pump(const Duration(milliseconds: 30));
+      check(getAnimation(tester, message.id))
+        ..value.isGreaterThan(0.0)
+        ..value.isLessThan(1.0)
+        ..status.equals(AnimationStatus.forward);
+
+      // introduce new message
+      final newMessage = eg.streamMessage(flags:[MessageFlag.read]);
+      store.handleEvent(MessageEvent(id: 0, message: newMessage));
+      await tester.pump(); // process handleEvent
+      check(find.byType(MessageItem).evaluate()).length.equals(2);
+      check(getAnimation(tester, message.id))
+        ..value.isGreaterThan(0.0)
+        ..value.isLessThan(1.0)
+        ..status.equals(AnimationStatus.forward);
+      check(getAnimation(tester, newMessage.id))
+        ..value.equals(0.0)
+        ..status.equals(AnimationStatus.dismissed);
+
+      final frames = await tester.pumpAndSettle();
+      check(frames).isGreaterThan(1);
+      check(getAnimation(tester, message.id))
+        ..value.equals(0.0)
+        ..status.equals(AnimationStatus.completed);
+      check(getAnimation(tester, newMessage.id))
+        ..value.equals(0.0)
+        ..status.equals(AnimationStatus.dismissed);
+    });
   });
 }