msglist: Update layout of MessageWithPossibleSender

Adjusted 'fetch older messages on scroll' test as new layout is more compact given consecutive messages with the same sender.
sirpengi · Jan 9, 2024 · 579bbdb · 579bbdb
1 parent 7bd88af
commit 579bbdb
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 54 deletions.
diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -841,6 +841,9 @@ String formatHeaderDate(
 }
 
 /// A Zulip message, showing the sender's name and avatar if specified.
+// Design referenced from:
+//   - https://github.com/zulip/zulip-mobile/issues/5511
+//   - https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=144-12134&mode=design&t=Vkvdz5hmNVd5um1u-0
 class MessageWithPossibleSender extends StatelessWidget {
   const MessageWithPossibleSender({super.key, required this.item});
 
@@ -852,61 +855,67 @@ class MessageWithPossibleSender extends StatelessWidget {
 
     final senderRow = item.showSender
       ? Padding(
-          padding: const EdgeInsets.fromLTRB(3, 6, 11, 0),
-          child: GestureDetector(
-            onTap: () => Navigator.push(context,
-              ProfilePage.buildRoute(context: context,
-                userId: message.senderId)),
-            child: Avatar(size: 35, borderRadius: 4,
-              userId: message.senderId)))
-      : const SizedBox(width: 3 + 35 + 11);
+          padding: const EdgeInsets.only(top: 6, left: 16, right: 16),
+          child: Row(
+            mainAxisAlignment: MainAxisAlignment.spaceBetween,
+            crossAxisAlignment: CrossAxisAlignment.baseline,
+            textBaseline: TextBaseline.alphabetic,
+            children: [
+              Flexible(
+                child: GestureDetector(
+                  onTap: () => Navigator.push(context,
+                    ProfilePage.buildRoute(context: context,
+                      userId: message.senderId)),
+                  child: Row(
+                    children: [
+                      Avatar(size: 32, borderRadius: 3, userId: message.senderId),
+                      const SizedBox(width: 8),
+                      Flexible(
+                        child: Text(message.senderFullName, // TODO get from user data
+                          style: const TextStyle(
+                            fontFamily: 'Source Sans 3',
+                            fontSize: 18,
+                            height: (22 / 18),
+                          ).merge(weightVariableTextStyle(context, wght: 600,
+                              wghtIfPlatformRequestsBold: 900)),
+                          overflow: TextOverflow.ellipsis)),
+                    ]))),
+              Text(
+                _kMessageTimestampFormat.format(
+                  DateTime.fromMillisecondsSinceEpoch(1000 * message.timestamp)),
+                style: TextStyle(
+                  color: _kMessageTimestampColor,
+                  fontFamily: 'Source Sans 3',
+                  fontSize: 16,
+                  height: (18 / 16),
+                  fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')],
+                ).merge(weightVariableTextStyle(context))),
+            ]),
+      ) : null;
 
     return GestureDetector(
       behavior: HitTestBehavior.translucent,
       onLongPress: () => showMessageActionSheet(context: context, message: message),
-      // TODO clean up this layout, by less precisely imitating web
-      child: Padding(
-        padding: const EdgeInsets.only(top: 2, bottom: 3, left: 8, right: 8),
-        child: Row(crossAxisAlignment: CrossAxisAlignment.start, children: [
-          senderRow,
-          Expanded(
-            child: Column(
-              crossAxisAlignment: CrossAxisAlignment.stretch,
-              children: [
-                if (item.showSender) ...[
-                  const SizedBox(height: 3),
-                  GestureDetector(
-                    onTap: () => Navigator.push(context,
-                      ProfilePage.buildRoute(context: context,
-                        userId: message.senderId)),
-                    child: Text(message.senderFullName, // TODO get from user data
-                      style: const TextStyle(
-                        fontFamily: 'Source Sans 3',
-                        fontSize: 18,
-                        height: (22 / 18),
-                      ).merge(weightVariableTextStyle(context, wght: 600,
-                                wghtIfPlatformRequestsBold: 900)))),
-                  const SizedBox(height: 4),
-                ],
-                MessageContent(message: message, content: item.content),
-                if ((message.reactions?.total ?? 0) > 0)
-                  ReactionChipsList(messageId: message.id, reactions: message.reactions!)
-              ])),
-          Container(
-            width: 80,
-            padding: const EdgeInsets.only(top: 4, right: 16 - 8),
-            alignment: Alignment.topRight,
-            child: Text(
-              _kMessageTimestampFormat.format(
-                DateTime.fromMillisecondsSinceEpoch(1000 * message.timestamp)),
-              style: TextStyle(
-                color: _kMessageTimestampColor,
-                fontFamily: 'Source Sans 3',
-                fontSize: 16,
-                height: (18 / 16),
-                fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')],
-              ).merge(weightVariableTextStyle(context)))),
-        ])));
+      child: Column(
+        children: [
+          if (senderRow != null) senderRow,
+          Row(
+            crossAxisAlignment: CrossAxisAlignment.start,
+            children: [
+              const SizedBox(width: 16),
+              Expanded(
+                child: Padding(
+                  padding: const EdgeInsets.symmetric(vertical: 4),
+                  child: Column(
+                    crossAxisAlignment: CrossAxisAlignment.stretch,
+                    children: [
+                      MessageContent(message: message, content: item.content),
+                      if ((message.reactions?.total ?? 0) > 0)
+                        ReactionChipsList(messageId: message.id, reactions: message.reactions!)
+                    ]))),
+              const SizedBox(width: 16),
+            ]),
+        ]));
   }
 }
 

diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart
@@ -86,8 +86,8 @@ void main() {
 
     testWidgets('basic', (tester) async {
       await setupMessageListPage(tester, foundOldest: false,
-        messages: List.generate(200, (i) => eg.streamMessage(id: 950 + i, sender: eg.selfUser)));
-      check(itemCount(tester)).equals(203);
+        messages: List.generate(300, (i) => eg.streamMessage(id: 950 + i, sender: eg.selfUser)));
+      check(itemCount(tester)).equals(303);
 
       // Fling-scroll upward...
       await tester.fling(find.byType(MessageListPage), const Offset(0, 300), 8000);
@@ -100,7 +100,7 @@ void main() {
       await tester.pump(Duration.zero); // Allow a frame for the response to arrive.
 
       // Now we have more messages.
-      check(itemCount(tester)).equals(303);
+      check(itemCount(tester)).equals(403);
     });
 
     testWidgets('observe double-fetch glitch', (tester) async {