From b1d902dfd2c9bd0816c30f368fb86130593ae36c Mon Sep 17 00:00:00 2001 From: Shu Chen Date: Tue, 12 Dec 2023 15:57:20 +0000 Subject: [PATCH 1/3] icons: Add star_filled custom icon Taken today from Figma: https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A21700&mode=dev --- assets/icons/ZulipIcons.ttf | Bin 5716 -> 5924 bytes assets/icons/star_filled.svg | 5 +++++ lib/widgets/icons.dart | 9 ++++++--- 3 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 assets/icons/star_filled.svg diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 6ca113431c0183b0540458f464e6d16fdb9299dc..04e2b86e9c729a2017e7552250ba8395e8395ad3 100644 GIT binary patch delta 1376 zcmb7EJ!}(66n-O(LyhdagwMlPcsr`BdzMH^-8Vnt(TETb~sMH#Z*N*U=zp?iW=1=N# z<43(qiSq~$Zo+V?;azL85!Q|14(7#KuQ6%;eteiH{T4|aY|bq#)|=mt64@$z@{d-_ zvEHBpRPp)S$ET0J>|?!to=vyTqNr{2FA)9>wONGqvjfCQqdcBbN}{JIO(r-) z4rM6^vBEGkh5y#SFeI17<5 z>5wQRl!>PZx+@-oUkYLw$O=CNJr$&*VitqHi=;)m_J7h9#W)Dt0aY8LBG`d%OUJep z#415=4PkB~Rgt2F)T=?t8LHUG zxP(&>2c1OTIpm+GwEv^(hT;gT`7GpRv9+t)a;r`jTM8vR1?O_Em``JM8D;1#Lj+`^ zPSL8sn1l)TBog4CCAz@l5_r35Lc#)1O1R)D386VHA-;r~L>4^L5)U8)T$T{uSY4t3 zZb+2Cvl2FVP9g;spPWDsFfWk=FGz@sU6jayuSjHz7%WNT!B-{H{yput3tKo!0lZ|Q zb>bH8&jNn(3hNsF^kC$|es z(enHl%h)5lPdPY(rM@b!XjazJZfv&=GpymK-LKXF delta 1128 zcma)5OHUI~7(LUO(o*_vXXyJy!$JulA<6<1<3@A=3B<<^E3Fiv(3A?eY8P%u3>GJG zsV-cRxDghH5W|8H7se1{q6-(sg_`JwA7Jp@>0Ri~xjA#@`{vy9efQ4X=lvi08c+aq zp$Qd(m!_^>Jmv160F)8p@x;`?(6yt;ZF zO`5dIr9yG#$H7Z#zYu53OgJ?y$LBpFU#`^GMr4gm)Nb%kUKSuyTgvbWE5kZtzuD|kO>%GpbM!g3K z_z<@A3B(y0$Z$^1@{G{eX*s5u4K*mt!p@PRNsMA);y@=Yax4|%T@-DbyQvFsnn7Ek zQ#k@wI~pSo^$8>`bN!!o#bP2X>!hnrw=}t%+D;TS8pP3K^%kZ@2L)LQqI~V8X^!l- zin$oI^J4WrX4`ua4DHY!@4MhSDMV)fX2a>D#H3Yf(_YR8HdGbJyDbG+P^81m?6 zF2w#sUtxAHx-_`L6g$+#_PN=5jtfk4uYzV%OPWu$Vw6*2@i65Pic?uVkbyrJO(%n9 z)_&i7rrH~BQB2v9C40!z5+QOy!biR(5hKq?#K}d8Fu5ebpO}kkqnlio5I^>ugh8Hf zh#MgaFG!@w6$!EbixN6{Ng_b5N@U1237!=2gjf(quPzZK-<3#{S0p@1KJbiN2%38x oTV~A>9e8gaw7>cPvHjZN>)gH^_^j~&_8Mbm%_y2*jZqu$2aI!>`~Uy| diff --git a/assets/icons/star_filled.svg b/assets/icons/star_filled.svg new file mode 100644 index 0000000000..de3a73cf10 --- /dev/null +++ b/assets/icons/star_filled.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index fe6ddf30cf..461be1f107 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -57,14 +57,17 @@ abstract final class ZulipIcons { /// The Zulip custom icon "read_receipts". static const IconData read_receipts = IconData(0xf10b, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "star_filled". + static const IconData star_filled = IconData(0xf10c, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "topic". - static const IconData topic = IconData(0xf10c, fontFamily: "Zulip Icons"); + static const IconData topic = IconData(0xf10d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf10d, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf10e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf10e, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf10f, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } From d95b602cbb73777a0d6870bc4b06b58f804e1a7e Mon Sep 17 00:00:00 2001 From: Shu Chen Date: Mon, 4 Dec 2023 19:37:51 +0000 Subject: [PATCH 2/3] msglist: Add star marker in messages --- lib/widgets/message_list.dart | 11 ++++++++++- test/widgets/message_list_test.dart | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index d9e3dbf0e9..2e01d1ff45 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -849,6 +849,8 @@ class MessageWithPossibleSender extends StatelessWidget { final MessageListMessageItem item; + static final _starColor = const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(); + @override Widget build(BuildContext context) { final message = item.message; @@ -913,7 +915,14 @@ class MessageWithPossibleSender extends StatelessWidget { if ((message.reactions?.total ?? 0) > 0) ReactionChipsList(messageId: message.id, reactions: message.reactions!) ])), - const SizedBox(width: 16), + 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), ]), ]))); } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index f116ac5fcd..6d2ecda5e6 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -455,6 +455,20 @@ void main() { }); }); + group('Starred messages', () { + testWidgets('unstarred message', (WidgetTester tester) async { + final message = eg.streamMessage(flags: []); + await setupMessageListPage(tester, messages: [message]); + check(find.byIcon(ZulipIcons.star_filled).evaluate()).isEmpty(); + }); + + testWidgets('starred message', (WidgetTester tester) async { + final message = eg.streamMessage(flags: [MessageFlag.starred]); + await setupMessageListPage(tester, messages: [message]); + check(find.byIcon(ZulipIcons.star_filled).evaluate()).length.equals(1); + }); + }); + group('_UnreadMarker animations', () { // TODO: Improve animation state testing so it is less tied to // implementation details and more focused on output, see: From fd9e858f1549ddfc6fafc8659f00d1cea6cd52fa Mon Sep 17 00:00:00 2001 From: Shu Chen Date: Mon, 4 Dec 2023 19:33:41 +0000 Subject: [PATCH 3/3] action_sheet: Add button to "star" and "unstar" message Fixes: #170 --- assets/l10n/app_en.arb | 16 +++++ lib/widgets/action_sheet.dart | 50 ++++++++++++++ test/widgets/action_sheet_test.dart | 100 ++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 574aded4f9..63e8249b72 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -55,6 +55,14 @@ "@actionSheetOptionQuoteAndReply": { "description": "Label for Quote and reply button on action sheet." }, + "actionSheetOptionStarMessage": "Star message", + "@actionSheetOptionStarMessage": { + "description": "Label for star button on action sheet." + }, + "actionSheetOptionUnstarMessage": "Unstar message", + "@actionSheetOptionUnstarMessage": { + "description": "Label for unstar button on action sheet." + }, "errorCouldNotFetchMessageSource": "Could not fetch message source", "@errorCouldNotFetchMessageSource": { "description": "Error message when the source of a message could not be fetched." @@ -128,6 +136,14 @@ "@errorSharingFailed": { "description": "Error message when sharing a message failed." }, + "errorStarMessageFailedTitle": "Failed to star message", + "@errorStarMessageFailedTitle": { + "description": "Error title when starring a message failed." + }, + "errorUnstarMessageFailedTitle": "Failed to unstar message", + "@errorUnstarMessageFailedTitle": { + "description": "Error title when unstarring a message failed." + }, "successLinkCopied": "Link copied", "@successLinkCopied": { "description": "Success message after copy link action completed." diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index a35cb08f3b..7ca86d3779 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -10,6 +10,7 @@ import 'clipboard.dart'; import 'compose_box.dart'; import 'dialog.dart'; import 'draggable_scrollable_modal_bottom_sheet.dart'; +import 'icons.dart'; import 'message_list.dart'; import 'store.dart'; @@ -38,6 +39,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes builder: (BuildContext _) { return Column(children: [ if (!hasThumbsUpReactionVote) AddThumbsUpButton(message: message, messageListContext: context), + StarButton(message: message, messageListContext: context), ShareButton(message: message, messageListContext: context), if (isComposeBoxOffered) QuoteAndReplyButton( message: message, @@ -115,6 +117,54 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton { }; } +class StarButton extends MessageActionSheetMenuItemButton { + StarButton({ + super.key, + required super.message, + required super.messageListContext, + }); + + @override get icon => ZulipIcons.star_filled; + + @override + String label(ZulipLocalizations zulipLocalizations) { + return message.flags.contains(MessageFlag.starred) + ? zulipLocalizations.actionSheetOptionUnstarMessage + : zulipLocalizations.actionSheetOptionStarMessage; + } + + @override get onPressed => (BuildContext context) async { + Navigator.of(context).pop(); + final zulipLocalizations = ZulipLocalizations.of(messageListContext); + final op = message.flags.contains(MessageFlag.starred) + ? UpdateMessageFlagsOp.remove + : UpdateMessageFlagsOp.add; + + try { + final connection = PerAccountStoreWidget.of(messageListContext).connection; + await updateMessageFlags(connection, messages: [message.id], + op: op, flag: MessageFlag.starred); + } catch (e) { + if (!messageListContext.mounted) return; + + String? errorMessage; + switch (e) { + case ZulipApiException(): + errorMessage = e.message; + // TODO specific messages for common errors, like network errors + // (support with reusable code) + default: + } + + await showErrorDialog(context: messageListContext, + title: switch(op) { + UpdateMessageFlagsOp.remove => zulipLocalizations.errorUnstarMessageFailedTitle, + UpdateMessageFlagsOp.add => zulipLocalizations.errorStarMessageFailedTitle, + }, message: errorMessage); + } + }; +} + class ShareButton extends MessageActionSheetMenuItemButton { ShareButton({ super.key, diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index e4b1df53e9..91bca5b091 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -1,3 +1,5 @@ +import 'dart:convert'; + import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -7,10 +9,12 @@ import 'package:http/http.dart' as http; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/compose.dart'; +import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/content.dart'; +import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/store.dart'; import 'package:share_plus_platform_interface/method_channel/method_channel_share.dart'; @@ -142,6 +146,102 @@ void main() { }); }); + group('StarButton', () { + Future tapButton(WidgetTester tester) async { + // Starred messages include the same icon so we need to + // match only by descendants of [BottomSheet]. + await tester.ensureVisible(find.descendant( + of: find.byType(BottomSheet), + matching: find.byIcon(ZulipIcons.star_filled, skipOffstage: false))); + await tester.tap(find.descendant( + of: find.byType(BottomSheet), + matching: find.byIcon(ZulipIcons.star_filled))); + await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e + } + + testWidgets('star success', (WidgetTester tester) async { + final message = eg.streamMessage(flags: []); + await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + + final connection = store.connection as FakeApiConnection; + connection.prepare(json: {}); + await tapButton(tester); + await tester.pump(Duration.zero); + + check(connection.lastRequest).isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages/flags') + ..bodyFields.deepEquals({ + 'messages': jsonEncode([message.id]), + 'op': 'add', + 'flag': 'starred', + }); + }); + + testWidgets('unstar success', (WidgetTester tester) async { + final message = eg.streamMessage(flags: [MessageFlag.starred]); + await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + + final connection = store.connection as FakeApiConnection; + connection.prepare(json: {}); + await tapButton(tester); + await tester.pump(Duration.zero); + + check(connection.lastRequest).isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages/flags') + ..bodyFields.deepEquals({ + 'messages': jsonEncode([message.id]), + 'op': 'remove', + 'flag': 'starred', + }); + }); + + testWidgets('star request has an error', (WidgetTester tester) async { + final message = eg.streamMessage(flags: []); + await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + + final connection = store.connection as FakeApiConnection; + + connection.prepare(httpStatus: 400, json: { + 'code': 'BAD_REQUEST', + 'msg': 'Invalid message(s)', + 'result': 'error', + }); + await tapButton(tester); + await tester.pump(Duration.zero); // error arrives; error dialog shows + + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: zulipLocalizations.errorStarMessageFailedTitle, + expectedMessage: 'Invalid message(s)'))); + }); + + testWidgets('unstar request has an error', (WidgetTester tester) async { + final message = eg.streamMessage(flags: [MessageFlag.starred]); + await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + + final connection = store.connection as FakeApiConnection; + + connection.prepare(httpStatus: 400, json: { + 'code': 'BAD_REQUEST', + 'msg': 'Invalid message(s)', + 'result': 'error', + }); + await tapButton(tester); + await tester.pump(Duration.zero); // error arrives; error dialog shows + + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: zulipLocalizations.errorUnstarMessageFailedTitle, + expectedMessage: 'Invalid message(s)'))); + }); + }); + group('ShareButton', () { // Tests should call this. MockSharePlus setupMockSharePlus() {