msglist: Leave blank space for "mark as read" button so using it doesn't cause messages to shift

This commit addresses the issue of the message list shifting downwards when the "mark as read" button is pressed. The previous implementation would return an empty widget when the button was pressed and cause the message list to shift down. Now it takes a widget which will display the button based on the opacity level and will disable the region the button holds once the button is pressed, thus won't cause the message list to shift down.

Fixes: #562

Author: abelaba

lib/widgets/message_list.dart

@@ -461,34 +461,39 @@ class MarkAsReadWidget extends StatelessWidget {
     final zulipLocalizations = ZulipLocalizations.of(context);
     final store = PerAccountStoreWidget.of(context);
     final unreadCount = store.unreads.countInNarrow(narrow);
-    return AnimatedCrossFade(
-      duration: const Duration(milliseconds: 300),
-      crossFadeState: (unreadCount > 0) ? CrossFadeState.showSecond : CrossFadeState.showFirst,
-      firstChild: const SizedBox.shrink(),
-      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: 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:
-                // Restate [FilledButton]'s default, which inherits from
-                // [zulipTypography]…
-                Theme.of(context).textTheme.labelLarge!
-                // …then clobber some attributes to follow Figma:
-                .merge(const TextStyle(
-                  fontSize: 18,
-                  height: (23 / 18))
-                .merge(weightVariableTextStyle(context, wght: 400))),
-              shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
-            ),
-            onPressed: () => _handlePress(context),
-            icon: const Icon(Icons.playlist_add_check),
-            label: Text(zulipLocalizations.markAllAsReadLabel)))));
+    final areMessagesRead = unreadCount == 0;
+
+    return IgnorePointer(
+      ignoring: areMessagesRead,
+      child: AnimatedOpacity(
+        opacity: areMessagesRead ? 0 : 1,
+        duration: Duration(milliseconds: areMessagesRead ? 2000 : 300),
+        curve: Curves.easeOut,
+        child: 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: 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:
+                  // Restate [FilledButton]'s default, which inherits from
+                  // [zulipTypography]…
+                  Theme.of(context).textTheme.labelLarge!
+                  // …then clobber some attributes to follow Figma:
+                  .merge(const TextStyle(
+                    fontSize: 18,
+                    height: (23 / 18))
+                  .merge(weightVariableTextStyle(context, wght: 400))),
+                shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
+              ),
+              onPressed: () => _handlePress(context),
+              icon: const Icon(Icons.playlist_add_check),
+              label: Text(zulipLocalizations.markAllAsReadLabel))))),
+    );
   }
 }
 

test/widgets/message_list_test.dart

@@ -610,13 +610,25 @@ void main() {
 
   group('MarkAsReadWidget', () {
     bool isMarkAsReadButtonVisible(WidgetTester tester) {
-      // Zero height elements on the edge of a scrolling viewport
-      // are treated as invisible for hit-testing, see
-      // [SliverMultiBoxAdaptorElement.debugVisitOnstageChildren].
-      // Set `skipOffstage: false` here to safely target the
-      // [MarkAsReadWidget] even when it is inactive.
-      return tester.getSize(
-        find.byType(MarkAsReadWidget, skipOffstage: false)).height > 0;
+      final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
+      // Locate the [MarkAsReadWidget] and check its state
+      // to determine if it is visible.
+      var markAsReadWidget = find.byType(MarkAsReadWidget);
+
+      // Locate the [Text] widget with the markAllAsReadLabel
+      // to determine if the [MarkAsReadWidget] is visible.
+      // This is done by finding a descendant of the [MarkAsReadWidget]
+      // that is a [Text] widget with the markAllAsReadLabel.
+      // The hitTestable() method is used to make the descendant widget hit-testable,
+      // allowing it to be found and evaluated in the widget tree.
+      final markAsReadWidgetButtonDescendant = find.descendant(
+        of: markAsReadWidget,
+        matching: find.text(zulipLocalizations.markAllAsReadLabel),
+      ).hitTestable();
+
+      // If the [Text] widget is found, then the [MarkAsReadWidget] is visible.
+      // Otherwise, it is not visible.
+      return markAsReadWidgetButtonDescendant.evaluate().isNotEmpty;
     }
 
     testWidgets('from read to unread', (WidgetTester tester) async {
@@ -909,6 +921,35 @@ void main() {
           expectedTitle: zulipLocalizations.errorMarkAsReadFailedTitle,
           expectedMessage: 'Oops');
       });
+
+      testWidgets('MessageList does not shift down when mark as read button is pressed', (WidgetTester tester) async {
+        final message = eg.streamMessage(flags: []);
+        final unreadMsgs = eg.unreadMsgs(streams:[
+          UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId, unreadMessageIds: [message.id])
+        ]);
+        await setupMessageListPage(tester, messages: [message], unreadMsgs: unreadMsgs);
+        check(isMarkAsReadButtonVisible(tester)).isTrue();
+
+        // Get the top position of the first message item before the mark as read button is pressed
+        // and then after the button is pressed. The top position should remain the same.
+        // This is to ensure that the message list does not shift down when the mark as read button is pressed.
+        final firstMessageItemTopPositionBefore = tester.getTopLeft(find.byType(MessageItem).first).dy;
+
+        store.handleEvent(UpdateMessageFlagsAddEvent(
+          id: 1,
+          flag: MessageFlag.read,
+          messages: [message.id],
+          all: false,
+        ));
+
+        await tester.pumpAndSettle();
+        check(isMarkAsReadButtonVisible(tester)).isFalse();
+
+        final firstMessageItemTopPositionAfter = tester.getTopLeft(find.byType(MessageItem).first).dy;
+
+        check(firstMessageItemTopPositionBefore).equals(firstMessageItemTopPositionAfter);
+
+      });
     });
   });
 }