Skip to content

Commit

Permalink
msglist: Maintain _UnreadMarker animation state when MessageListView.…
Browse files Browse the repository at this point in the history
…items changes size

There is an improved `getAnimation` helper (that easily extracts
animation state of a specific message due to the newly introduced
keys added to messages) in the new "animation state persistence"
test that will be refactored in the next commit.
  • Loading branch information
sirpengi committed Oct 25, 2023
1 parent 10365fc commit e5cb492
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 0 deletions.
25 changes: 25 additions & 0 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,30 @@ class _MessageListState extends State<MessageList> 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.
Expand Down Expand Up @@ -283,6 +307,7 @@ class _MessageListState extends State<MessageList> 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);
}
Expand Down
47 changes: 47 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<FadeTransition>(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);
});
});
}

0 comments on commit e5cb492

Please sign in to comment.