diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index a678d80cf0..72322dd71a 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -247,6 +247,30 @@ class _MessageListState extends State with PerAccountStoreAwareStat // TODO: Offer `ScrollViewKeyboardDismissBehavior.interactive` (or // similar) if that is ever offered: // https://github.com/flutter/flutter/issues/57609#issuecomment-1355340849 + findChildIndexCallback: (Key key) { + // 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, scrolling through All Messages in + // CZO on a Pixel 5, this only runs about 10 times per rebuild + // and the timing for each call is measured in 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. + final valueKey = key as ValueKey; + final index = model!.findItemWithMessageId(valueKey.value); + if (index == -1) { + return null; + } + // Note `itemBuilder` pulls from `model.items` in reverse order. + return length - index - 1; + }, keyboardDismissBehavior: switch (Theme.of(context).platform) { // This seems to offer the only built-in way to close the keyboard // on iOS. It's not ideal; see TODO above. @@ -283,6 +307,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat header: header, child: header); case MessageListMessageItem(): return MessageItem( + key: ValueKey(data.message.id), trailing: i == 0 ? const SizedBox(height: 8) : const SizedBox(height: 11), item: data); } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index c54d3cb763..a8cb7507c5 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -344,5 +344,52 @@ void main() { check(getAnimation(tester).value).equals(0); }); + + testWidgets('animation state persistence', (WidgetTester tester) async { + // Check that _UnreadMarker maintains its in-progress animation + // as the number of items change in MessageList. + + // TODO: pull this out into group + Animation getAnimation(WidgetTester tester, int messageId) { + final widget = tester.widget(find.descendant( + of: find.byKey(ValueKey(messageId)), + matching: find.byType(FadeTransition))); + return widget.opacity; + } + + final message = eg.streamMessage(id: 1, flags: []); + await setupMessageListPage(tester, messages: [message]); + + // initial state: marker is visible + check(getAnimation(tester, message.id).value).equals(1.0); + + store.handleEvent(UpdateMessageFlagsAddEvent( + id: 0, + flag: MessageFlag.read, + messages: [message.id], + all: false, + )); + await tester.pump(); // process handleEvent + await tester.pump(const Duration(milliseconds: 30)); // run animation partially + + final newMessage = eg.streamMessage(id: 2, flags:[MessageFlag.read]); + store.handleEvent(MessageEvent(id: 0, message: newMessage)); + await tester.pump(); // process handleEvent + + // in-progress state: ensure original message marker is still + // running through its animation but new message is not animating. + check(find.byType(MessageItem).evaluate()).length.equals(2); + check(getAnimation(tester, message.id).status) + .equals(AnimationStatus.forward); + check(getAnimation(tester, newMessage.id).status) + .equals(AnimationStatus.dismissed); + + // If we introduce an animation bug (remove findChildIndexCallback + // on StickyHeaderListView.builder) this returns 1. + final frames = await tester.pumpAndSettle(); + check(frames).isGreaterThan(1); + check(getAnimation(tester, message.id).status) + .equals(AnimationStatus.completed); + }); }); }