ui: Add edited/moved marker.

PIG208 · gnprice · commit 42d53c19e5ed · 2024-07-16T20:05:00.000-07:00
This adds basic support to displaying a edited/moved marker next to the message content. As of now this is implemented without the swipe gesture control that would expand the marker. Partially addresses #171. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
diff --git a/lib/widgets/edit_state_marker.dart b/lib/widgets/edit_state_marker.dart
@@ -0,0 +1,74 @@
+import 'package:flutter/material.dart';
+
+import '../api/model/model.dart';
+import 'icons.dart';
+import 'text.dart';
+import 'theme.dart';
+
+class EditStateMarker extends StatelessWidget {
+  const EditStateMarker({
+    super.key,
+    required this.editState,
+    required this.children,
+  });
+
+  final MessageEditState editState;
+  final List<Widget> children;
+
+  @override
+  Widget build(BuildContext context) {
+    final hasMarker = editState != MessageEditState.none;
+
+    return Row(
+      crossAxisAlignment: CrossAxisAlignment.baseline,
+      textBaseline: localizedTextBaseline(context),
+      children: [
+        hasMarker
+          ? _EditStateMarkerPill(editState: editState)
+          : const SizedBox(width: _EditStateMarkerPill.widthCollapsed),
+        ...children,
+      ],
+    );
+  }
+}
+
+class _EditStateMarkerPill extends StatelessWidget {
+  const _EditStateMarkerPill({required this.editState});
+
+  final MessageEditState editState;
+
+  /// The minimum width of the marker.
+  // Currently, only the collapsed state of the marker has been implemented,
+  // where only the marker icon, not the marker text, is visible.
+  static const double widthCollapsed = 16;
+
+  @override
+  Widget build(BuildContext context) {
+    final designVariables = DesignVariables.of(context);
+
+    final IconData icon;
+    final Offset offset;
+    switch (editState) {
+      case MessageEditState.none:
+        assert(false);
+        return const SizedBox(width: widthCollapsed);
+      case MessageEditState.edited:
+        icon = ZulipIcons.edited;
+        // These offsets are chosen ad hoc, but give a good vertical alignment
+        // of the icons with the first line of the message, when the message
+        // begins with a paragraph, at default text scaling.  See:
+        //   https://github.com/zulip/zulip-flutter/pull/762#issuecomment-2232041922
+        offset = const Offset(0, 2);
+      case MessageEditState.moved:
+        icon = ZulipIcons.message_moved;
+        offset = const Offset(0, 3);
+    }
+
+    return ConstrainedBox(
+      constraints: const BoxConstraints(maxWidth: widthCollapsed),
+      child: Transform.translate(
+        offset: offset,
+        child: Icon(
+          icon, size: 16, color: designVariables.editedMovedMarkerCollapsed)));
+  }
+}
diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -21,6 +21,7 @@ import 'page.dart';
 import 'profile.dart';
 import 'sticky_header.dart';
 import 'store.dart';
+import 'edit_state_marker.dart';
 import 'text.dart';
 import 'theme.dart';
 
@@ -962,18 +963,16 @@ class MessageWithPossibleSender extends StatelessWidget {
           if (senderRow != null)
             Padding(padding: const EdgeInsets.fromLTRB(16, 2, 16, 0),
               child: senderRow),
-          Row(crossAxisAlignment: CrossAxisAlignment.baseline,
-            textBaseline: localizedTextBaseline(context),
+          EditStateMarker(
+            editState: message.editState,
             children: [
-              const SizedBox(width: 16),
-              Expanded(
-                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!)
-                  ])),
+              Expanded(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!)
+                ])),
               SizedBox(width: 16,
                 child: message.flags.contains(MessageFlag.starred)
                   ? Icon(ZulipIcons.star_filled, size: 16, color: designVariables.star)
diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart
@@ -124,6 +124,7 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       title: const Color(0xff1a1a1a),
       streamColorSwatches: StreamColorSwatches.light,
       star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
+      editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
     );
 
   DesignVariables.dark() :
@@ -136,6 +137,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       streamColorSwatches: StreamColorSwatches.dark,
       // TODO(#95) unchanged in dark theme?
       star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
+      // TODO(#95) need dark-theme color
+      editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
     );
 
   DesignVariables._({
@@ -146,6 +149,7 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
     required this.title,
     required this.streamColorSwatches,
     required this.star,
+    required this.editedMovedMarkerCollapsed,
   });
 
   /// The [DesignVariables] from the context's active theme.
@@ -169,6 +173,7 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
 
   // Not named variables in Figma; taken from older Figma drafts, or elsewhere.
   final Color star;
+  final Color editedMovedMarkerCollapsed;
 
   @override
   DesignVariables copyWith({
@@ -179,6 +184,7 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
     Color? title,
     StreamColorSwatches? streamColorSwatches,
     Color? star,
+    Color? editedMovedMarkerCollapsed,
   }) {
     return DesignVariables._(
       bgTopBar: bgTopBar ?? this.bgTopBar,
@@ -188,6 +194,7 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       title: title ?? this.title,
       streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches,
       star: star ?? this.star,
+      editedMovedMarkerCollapsed: editedMovedMarkerCollapsed ?? this.editedMovedMarkerCollapsed,
     );
   }
 
@@ -204,6 +211,7 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       title: Color.lerp(title, other.title, t)!,
       streamColorSwatches: StreamColorSwatches.lerp(streamColorSwatches, other.streamColorSwatches, t),
       star: Color.lerp(star, other.star, t)!,
+      editedMovedMarkerCollapsed: Color.lerp(editedMovedMarkerCollapsed, other.editedMovedMarkerCollapsed, t)!,
     );
   }
 }
diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart
@@ -16,6 +16,7 @@ import 'package:zulip/model/localizations.dart';
 import 'package:zulip/model/narrow.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/widgets/content.dart';
+import 'package:zulip/widgets/emoji_reaction.dart';
 import 'package:zulip/widgets/icons.dart';
 import 'package:zulip/widgets/message_list.dart';
 import 'package:zulip/widgets/store.dart';
@@ -576,6 +577,99 @@ void main() {
     });
   });
 
+  group('EditStateMarker', () {
+    void checkMarkersCount({required int edited, required int moved}) {
+      check(find.byIcon(ZulipIcons.edited).evaluate()).length.equals(edited);
+      check(find.byIcon(ZulipIcons.message_moved).evaluate()).length.equals(moved);
+    }
+
+    testWidgets('no edited or moved messages', (WidgetTester tester) async {
+      final message = eg.streamMessage();
+      await setupMessageListPage(tester, messages: [message]);
+      checkMarkersCount(edited: 0, moved: 0);
+    });
+
+    testWidgets('edited and moved messages from events', (WidgetTester tester) async {
+      final message = eg.streamMessage();
+      final message2 = eg.streamMessage();
+      await setupMessageListPage(tester, messages: [message, message2]);
+      checkMarkersCount(edited: 0, moved: 0);
+
+      await store.handleEvent(eg.updateMessageEditEvent(message, renderedContent: 'edited'));
+      await tester.pump();
+      checkMarkersCount(edited: 1, moved: 0);
+
+      await store.handleEvent(eg.updateMessageMoveEvent(
+        [message, message2], origTopic: 'old', newTopic: 'new'));
+      await tester.pump();
+      checkMarkersCount(edited: 1, moved: 1);
+
+      await store.handleEvent(eg.updateMessageEditEvent(message2, renderedContent: 'edited'));
+      await tester.pump();
+      checkMarkersCount(edited: 2, moved: 0);
+    });
+
+    List<List<(String, Rect)>> captureMessageRects(
+      WidgetTester tester,
+      List<Message> messages,
+      Message targetMessage,
+    ) {
+      assert(messages.contains(targetMessage));
+      final List<List<(String, Rect)>> result = [];
+      for (final message in messages) {
+        final baseFinder = find.byWidgetPredicate(
+          (widget) => widget is MessageWithPossibleSender
+            && widget.item.message.id == message.id);
+
+        Rect getRectMatching(Finder matching) {
+          final finder = find.descendant(
+            of: baseFinder, matching: matching)..tryEvaluate();
+          check(finder.found, because: '${message.content}: $matching')
+            .length.equals(1);
+          return tester.getRect(finder.first);
+        }
+
+        result.add([
+          ('MessageWithPossibleSender', tester.getRect(baseFinder)),
+          ('MessageContent', getRectMatching(find.byType(MessageContent))),
+          ('Paragraph', getRectMatching(find.byType(Paragraph))),
+          if (message.id == targetMessage.id) ...[
+            ('Star', getRectMatching(find.byIcon(ZulipIcons.star_filled))),
+            ('ReactionChipsList', getRectMatching(find.byType(ReactionChipsList))),
+          ],
+        ]);
+      }
+      return result;
+    }
+
+    testWidgets('edit state updates do not affect layout', (WidgetTester tester) async {
+      final messages = [
+        eg.streamMessage(),
+        eg.streamMessage(
+          reactions: [eg.unicodeEmojiReaction, eg.realmEmojiReaction],
+          flags: [MessageFlag.starred]),
+        eg.streamMessage(),
+      ];
+      final StreamMessage messageWithMarker = messages[1];
+      await setupMessageListPage(tester, messages: messages);
+      final rectsBefore = captureMessageRects(tester, messages, messageWithMarker);
+      checkMarkersCount(edited: 0, moved: 0);
+
+      await store.handleEvent(eg.updateMessageMoveEvent(
+        [messageWithMarker], origTopic: 'old', newTopic: messageWithMarker.topic));
+      await tester.pump();
+      check(captureMessageRects(tester, messages, messageWithMarker))
+        .deepEquals(rectsBefore);
+      checkMarkersCount(edited: 0, moved: 1);
+
+      await store.handleEvent(eg.updateMessageEditEvent(messageWithMarker));
+      await tester.pump();
+      check(captureMessageRects(tester, messages, messageWithMarker))
+        .deepEquals(rectsBefore);
+      checkMarkersCount(edited: 1, moved: 0);
+    });
+  });
+
   group('_UnreadMarker animations', () {
     // TODO: Improve animation state testing so it is less tied to
     //   implementation details and more focused on output, see:
