diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 1fd2db9d1d..e4ea5b0fc4 100644 Binary files a/assets/icons/ZulipIcons.ttf and b/assets/icons/ZulipIcons.ttf differ diff --git a/assets/icons/edited.svg b/assets/icons/edited.svg new file mode 100644 index 0000000000..9146bba9e2 --- /dev/null +++ b/assets/icons/edited.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/message_moved.svg b/assets/icons/message_moved.svg new file mode 100644 index 0000000000..591a988c34 --- /dev/null +++ b/assets/icons/message_moved.svg @@ -0,0 +1,3 @@ + + + diff --git a/lib/widgets/edit_state_marker.dart b/lib/widgets/edit_state_marker.dart new file mode 100644 index 0000000000..0c6abfe442 --- /dev/null +++ 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 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/icons.dart b/lib/widgets/icons.dart index 23080a2257..9b3666a27c 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -42,38 +42,44 @@ abstract final class ZulipIcons { /// The Zulip custom icon "clock". static const IconData clock = IconData(0xf106, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "edited". + static const IconData edited = IconData(0xf107, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "globe". - static const IconData globe = IconData(0xf107, fontFamily: "Zulip Icons"); + static const IconData globe = IconData(0xf108, fontFamily: "Zulip Icons"); /// The Zulip custom icon "group_dm". - static const IconData group_dm = IconData(0xf108, fontFamily: "Zulip Icons"); + static const IconData group_dm = IconData(0xf109, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_sign". - static const IconData hash_sign = IconData(0xf109, fontFamily: "Zulip Icons"); + static const IconData hash_sign = IconData(0xf10a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "language". - static const IconData language = IconData(0xf10a, fontFamily: "Zulip Icons"); + static const IconData language = IconData(0xf10b, fontFamily: "Zulip Icons"); /// The Zulip custom icon "lock". - static const IconData lock = IconData(0xf10b, fontFamily: "Zulip Icons"); + static const IconData lock = IconData(0xf10c, fontFamily: "Zulip Icons"); + + /// The Zulip custom icon "message_moved". + static const IconData message_moved = IconData(0xf10d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "mute". - static const IconData mute = IconData(0xf10c, fontFamily: "Zulip Icons"); + static const IconData mute = IconData(0xf10e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "read_receipts". - static const IconData read_receipts = IconData(0xf10d, fontFamily: "Zulip Icons"); + static const IconData read_receipts = IconData(0xf10f, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star_filled". - static const IconData star_filled = IconData(0xf10e, fontFamily: "Zulip Icons"); + static const IconData star_filled = IconData(0xf110, fontFamily: "Zulip Icons"); /// The Zulip custom icon "topic". - static const IconData topic = IconData(0xf10f, fontFamily: "Zulip Icons"); + static const IconData topic = IconData(0xf111, fontFamily: "Zulip Icons"); /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf110, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf112, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf111, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf113, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index b804fcab92..329dabf58d 100644 --- 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'; @@ -896,12 +897,10 @@ class MessageWithPossibleSender extends StatelessWidget { final MessageListMessageItem item; - // TODO(#95) unchanged in dark theme? - static final _starColor = const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(); - @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); + final designVariables = DesignVariables.of(context); final message = item.message; final sender = store.users[message.senderId]; @@ -964,25 +963,21 @@ class MessageWithPossibleSender extends StatelessWidget { if (senderRow != null) Padding(padding: const EdgeInsets.fromLTRB(16, 2, 16, 0), child: senderRow), - Row(crossAxisAlignment: CrossAxisAlignment.start, children: [ - const SizedBox(width: 16), - Expanded( - child: Column( + EditStateMarker( + editState: message.editState, + children: [ + 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) - // TODO(#157): fix how star marker aligns with message content - // Design from Figma at: - // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=813%3A28817&mode=dev . - ? Padding(padding: const EdgeInsets.only(top: 4), - child: Icon(ZulipIcons.star_filled, size: 16, color: _starColor)) - : null), - ]), + SizedBox(width: 16, + child: message.flags.contains(MessageFlag.starred) + ? Icon(ZulipIcons.star_filled, size: 16, color: designVariables.star) + : null), + ]), ]))); } } diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index c8742cae72..c9415e7be2 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -123,6 +123,8 @@ class DesignVariables extends ThemeExtension { mainBackground: const Color(0xfff0f0f0), 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() : @@ -133,6 +135,10 @@ class DesignVariables extends ThemeExtension { mainBackground: const Color(0xff1d1d1d), title: const Color(0xffffffff), 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._({ @@ -142,6 +148,8 @@ class DesignVariables extends ThemeExtension { required this.mainBackground, required this.title, required this.streamColorSwatches, + required this.star, + required this.editedMovedMarkerCollapsed, }); /// The [DesignVariables] from the context's active theme. @@ -163,6 +171,10 @@ class DesignVariables extends ThemeExtension { // Not exactly from the Figma design, but from Vlad anyway. final StreamColorSwatches streamColorSwatches; + // Not named variables in Figma; taken from older Figma drafts, or elsewhere. + final Color star; + final Color editedMovedMarkerCollapsed; + @override DesignVariables copyWith({ Color? bgTopBar, @@ -171,6 +183,8 @@ class DesignVariables extends ThemeExtension { Color? mainBackground, Color? title, StreamColorSwatches? streamColorSwatches, + Color? star, + Color? editedMovedMarkerCollapsed, }) { return DesignVariables._( bgTopBar: bgTopBar ?? this.bgTopBar, @@ -179,6 +193,8 @@ class DesignVariables extends ThemeExtension { mainBackground: mainBackground ?? this.mainBackground, title: title ?? this.title, streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches, + star: star ?? this.star, + editedMovedMarkerCollapsed: editedMovedMarkerCollapsed ?? this.editedMovedMarkerCollapsed, ); } @@ -194,6 +210,8 @@ class DesignVariables extends ThemeExtension { mainBackground: Color.lerp(mainBackground, other.mainBackground, t)!, 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 index b583446d78..41e19b8f3c 100644 --- 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> captureMessageRects( + WidgetTester tester, + List messages, + Message targetMessage, + ) { + assert(messages.contains(targetMessage)); + final List> 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: