From 3e7683d6ea2f8f65c7554d4ffd790682da7df399 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 10 Dec 2024 22:17:10 -0500 Subject: [PATCH 01/11] msglist [nfc]: Remove extra parens Signed-off-by: Zixuan James Li --- lib/widgets/message_list.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index f9e62f01ce..03085ad653 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -317,7 +317,7 @@ class MessageListAppBarTitle extends StatelessWidget { required String text, }) { // A null [Icon.icon] makes a blank space. - final icon = (stream != null) ? iconDataForStream(stream) : null; + final icon = stream != null ? iconDataForStream(stream) : null; return Row( mainAxisSize: MainAxisSize.min, // TODO(design): The vertical alignment of the stream privacy icon is a bit ad hoc. @@ -1008,7 +1008,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { padding: const EdgeInsets.only(left: 6, right: 6, bottom: 3), child: Icon(size: 16, color: iconColor, // A null [Icon.icon] makes a blank space. - (stream != null) ? iconDataForStream(stream) : null)), + stream != null ? iconDataForStream(stream) : null)), Padding( padding: const EdgeInsets.symmetric(vertical: 11), child: Text(streamName, From 1e04f7bb348d3983e3c869f2dd674dd5622f0a1e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 22 Nov 2024 10:50:02 -0500 Subject: [PATCH 02/11] api: Add ApiConnection.patch The tests, ApiConnection.{post,patch,delete}, are mostly similar to each other, because the majority of them are testing that the params are parsed into the body with the same content type. If we find the need to update these test cases with new examples, it will be ripe to refactor them with a helper. Until then, just duplicate them for simplicity. Signed-off-by: Zixuan James Li --- lib/api/core.dart | 10 ++++++++++ test/api/core_test.dart | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/api/core.dart b/lib/api/core.dart index 320454b12b..96f9e2db11 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -224,6 +224,16 @@ class ApiConnection { return send(routeName, fromJson, request); } + Future patch(String routeName, T Function(Map) fromJson, + String path, Map? params) async { + final url = realmUrl.replace(path: "/api/v1/$path"); + final request = http.Request('PATCH', url); + if (params != null) { + request.bodyFields = encodeParameters(params)!; + } + return send(routeName, fromJson, request); + } + Future delete(String routeName, T Function(Map) fromJson, String path, Map? params) async { final url = realmUrl.replace(path: "/api/v1/$path"); diff --git a/test/api/core_test.dart b/test/api/core_test.dart index 865b142b5c..b45cdeaebf 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -201,6 +201,37 @@ void main() { checkRequest(['asdf'.codeUnits], 100, filename: null); }); + test('ApiConnection.patch', () async { + void checkRequest(Map? params, String expectedBody, {bool expectContentType = true}) { + finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async { + connection.prepare(json: {}); + await connection.patch(kExampleRouteName, (json) => json, 'example/route', params); + check(connection.lastRequest!).isA() + ..method.equals('PATCH') + ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route') + ..headers.deepEquals({ + ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), + ...kFallbackUserAgentHeader, + if (expectContentType) + 'content-type': 'application/x-www-form-urlencoded; charset=utf-8', + }) + ..body.equals(expectedBody); + })); + } + + checkRequest(null, '', expectContentType: false); + checkRequest({}, ''); + checkRequest({'x': 3}, 'x=3'); + checkRequest({'x': 3, 'y': 4}, 'x=3&y=4'); + checkRequest({'x': null}, 'x=null'); + checkRequest({'x': true}, 'x=true'); + checkRequest({'x': 'foo'}, 'x=%22foo%22'); + checkRequest({'x': [1, 2]}, 'x=%5B1%2C2%5D'); + checkRequest({'x': {'y': 1}}, 'x=%7B%22y%22%3A1%7D'); + checkRequest({'x': RawParameter('foo')}, 'x=foo'); + checkRequest({'x': RawParameter('foo'), 'y': 'bar'}, 'x=foo&y=%22bar%22'); + }); + test('ApiConnection.delete', () async { void checkRequest(Map? params, String expectedBody, {bool expectContentType = true}) { finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async { From 868f8b70498d67066e4a98a7218511bfb45785a6 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 24 Oct 2024 16:09:25 -0400 Subject: [PATCH 03/11] api: Add route updateUserTopic and its compat helper For the legacy case, there can be an error when muting a topic that is already muted or unmuting one that is already unmuted. The callers are not expected to handle such errors because they aren't really actionable. Similar to getMessageCompat, updateUserTopicCompat is expected to be dropped, eventually. Signed-off-by: Zixuan James Li --- lib/api/route/channels.dart | 49 ++++++++++++++ test/api/route/channels_test.dart | 107 ++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 test/api/route/channels_test.dart diff --git a/lib/api/route/channels.dart b/lib/api/route/channels.dart index d03ce501cf..00832f7fd0 100644 --- a/lib/api/route/channels.dart +++ b/lib/api/route/channels.dart @@ -1,6 +1,7 @@ import 'package:json_annotation/json_annotation.dart'; import '../core.dart'; +import '../model/model.dart'; part 'channels.g.dart'; /// https://zulip.com/api/get-stream-topics @@ -38,3 +39,51 @@ class GetStreamTopicsEntry { Map toJson() => _$GetStreamTopicsEntryToJson(this); } + +/// Update a topic's visibility policy. +/// +/// This encapsulates a server-feature check. +// TODO(server-7): remove this and just use updateUserTopic +Future updateUserTopicCompat(ApiConnection connection, { + required int streamId, + required String topic, + required UserTopicVisibilityPolicy visibilityPolicy, +}) { + final useLegacyApi = connection.zulipFeatureLevel! < 170; + if (useLegacyApi) { + final op = switch (visibilityPolicy) { + UserTopicVisibilityPolicy.none => 'remove', + UserTopicVisibilityPolicy.muted => 'add', + _ => throw UnsupportedError('$visibilityPolicy on old server'), + }; + // https://zulip.com/api/mute-topic + return connection.patch('muteTopic', (_) {}, 'users/me/subscriptions/muted_topics', { + 'stream_id': streamId, + 'topic': RawParameter(topic), + 'op': RawParameter(op), + }); + } else { + return updateUserTopic(connection, + streamId: streamId, + topic: topic, + visibilityPolicy: visibilityPolicy); + } +} + +/// https://zulip.com/api/update-user-topic +/// +/// This binding only supports feature levels 170+. +// TODO(server-7) remove FL 170+ mention in doc, and the related `assert` +Future updateUserTopic(ApiConnection connection, { + required int streamId, + required String topic, + required UserTopicVisibilityPolicy visibilityPolicy, +}) { + assert(visibilityPolicy != UserTopicVisibilityPolicy.unknown); + assert(connection.zulipFeatureLevel! >= 170); + return connection.post('updateUserTopic', (_) {}, 'user_topics', { + 'stream_id': streamId, + 'topic': RawParameter(topic), + 'visibility_policy': visibilityPolicy, + }); +} diff --git a/test/api/route/channels_test.dart b/test/api/route/channels_test.dart new file mode 100644 index 0000000000..d86c9447fe --- /dev/null +++ b/test/api/route/channels_test.dart @@ -0,0 +1,107 @@ +import 'package:checks/checks.dart'; +import 'package:http/http.dart' as http; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; + +import '../../stdlib_checks.dart'; +import '../fake_api.dart'; + +void main() { + test('smoke updateUserTopic', () { + return FakeApiConnection.with_((connection) async { + connection.prepare(json: {}); + await updateUserTopic(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.followed); + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/user_topics') + ..bodyFields.deepEquals({ + 'stream_id': '1', + 'topic': 'topic', + 'visibility_policy': '3', + }); + }); + }); + + test('updateUserTopic only accepts valid visibility policy', () { + return FakeApiConnection.with_((connection) async { + check(() => updateUserTopic(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.unknown), + ).throws(); + }); + }); + + test('updateUserTopicCompat when FL >= 170', () { + return FakeApiConnection.with_((connection) async { + connection.prepare(json: {}); + await updateUserTopicCompat(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.followed); + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/user_topics') + ..bodyFields.deepEquals({ + 'stream_id': '1', + 'topic': 'topic', + 'visibility_policy': '3', + }); + }); + }); + + group('legacy: use muteTopic when FL < 170', () { + test('updateUserTopic throws AssertionError when FL < 170', () { + return FakeApiConnection.with_(zulipFeatureLevel: 169, (connection) async { + check(() => updateUserTopic(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.muted), + ).throws(); + }); + }); + + test('updateUserTopicCompat throws UnsupportedError on unsupported policy', () { + return FakeApiConnection.with_(zulipFeatureLevel: 169, (connection) async { + check(() => updateUserTopicCompat(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.followed), + ).throws(); + }); + }); + + test('policy: none -> op: remove', () { + return FakeApiConnection.with_(zulipFeatureLevel: 169, (connection) async { + connection.prepare(json: {}); + await updateUserTopicCompat(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.none); + check(connection.takeRequests()).single.isA() + ..method.equals('PATCH') + ..url.path.equals('/api/v1/users/me/subscriptions/muted_topics') + ..bodyFields.deepEquals({ + 'stream_id': '1', + 'topic': 'topic', + 'op': 'remove', + }); + }); + }); + + test('policy: muted -> op: add', () { + return FakeApiConnection.with_(zulipFeatureLevel: 169, (connection) async { + connection.prepare(json: {}); + await updateUserTopicCompat(connection, + streamId: 1, topic: 'topic', + visibilityPolicy: UserTopicVisibilityPolicy.muted); + check(connection.takeRequests()).single.isA() + ..method.equals('PATCH') + ..url.path.equals('/api/v1/users/me/subscriptions/muted_topics') + ..bodyFields.deepEquals({ + 'stream_id': '1', + 'topic': 'topic', + 'op': 'add', + }); + }); + }); + }); +} From df10d698546eeadecbc3413f086b6eb342bbe3a2 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 24 Oct 2024 13:54:20 -0400 Subject: [PATCH 04/11] icons: Take mute/unmute/following icons from the web app Renamed "mute-new.svg", "unmute-new.svg" to "mute.svg" and "unmute.svg", respectively. These are taken from the web app because we checked https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=544-22131&node-type=canvas&m=dev and the Figma doesn't have the corresponding icons at the time this is implemented. See: https://github.com/zulip/zulip/tree/da4443f392cc8aa9e6879d905cb1ccd50b66127b/web/shared/icons Signed-off-by: Zixuan James Li --- assets/icons/ZulipIcons.ttf | Bin 10604 -> 11424 bytes assets/icons/follow.svg | 1 + assets/icons/inherit.svg | 4 +++ assets/icons/mute.svg | 6 +---- assets/icons/unmute.svg | 8 +++--- lib/widgets/icons.dart | 50 ++++++++++++++++++++---------------- 6 files changed, 38 insertions(+), 31 deletions(-) create mode 100644 assets/icons/follow.svg create mode 100644 assets/icons/inherit.svg diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 4f54b9a7305fe9734217d2b3b37ed2f80b3d1bbc..9a7d0f9a5a2810277aad91e38785bbe01849b605 100644 GIT binary patch delta 2844 zcmah}S!`Ta8UD}R?<{wG$Fq6HnSJ$g?HSuM-ePBKu!8G^##x+2!A>08!5gicI0ZUU z`cTwHm0()bN)d|GsudJ?sEbs}B9-2yh|kCgzojZ#j6*s+DF|)u3r)DdVPL&`oiT8-+B$UcYx|V6dIF;@%|$qJHNEH z{^VQRoACV(zW=(oa=t!2@W<{u2yg_7$CjqoSJ@34xlHlhSy+eOzc~)gN*~darI3yewZXmCV(lkg% z;OV9;Mvn4SfNZ07>Yz?oxG{H9hPv=wplVj6hkOefxQ7QUg{vC z6cD!;8Dx}|gH^@0iIWe7XQ*!3Rkzt4l##ZJnn=fw9sT_}QDzb$atO$66r{)_{6Tt% z&QQYUAcK_m`sWN3sTGkr5G#wX0{n7_8bg$Ox?LpKQQHn-QKYD#k0QnK#tF^B)PwIq z8ly?{@3s5yvpXxJR0Ulrcq`k)`7PdA^1rRYX^J+>{5iLk%X zA&s(hV^KQQcwe#_x1@+H{$hJeYWzmIG%91;i$Di}W5Vv(2Z884X!()ofOOe4EdJ)du5;U4fQGYFWS8J zwjS3{>bG6=N8)$I-{Ib~_{y^U z2n&TV0;-}K%o3_KjNU?Cg$^nuZ85IN7xKL$j9XXY%n+aze(V-w8TVv|Wpzxd!LVPG zv_nNa|9UVMjmE}}LM!tQ1_Px5qqVSgjU{V)`u8PRvN~E#iZ2+Mu;jh4c6EDw?SigJ zzF=6Nnp?QDFqau$n4P;cVZ;i??$1qpbHFIZ*w$~8{i#&{p1wq)Z{xZd*6#Q5u6+?6 zMjyN|AsFDTghL62B3UMdGOMG^ZmCx7>#d5Z0vhS-wS=?*WAqoUhQe`Hsn$l|0s_+b zUuB(b%a@lYt!5)?1{7KF=vvs=BqOZp9zj+D{(6hgZxw_wJz<8D!-ppi4=02E_z!() zT?vE&ik?pBx;cDc$gjJ^$J;tPCzpq2CPu7eP>}>J8Z*sUR1+j6n6ySFW`>rxm^m98q*XdvWs)F_t9hd<^#2-`PK-9^-rGg3gI+U+4rs8W-=F_ z;2aeP%wxyQ<5y0fI9O{>gj89JMa|RT&EYNYC+uN~Os-Y=>xdW^xNBG(VYUnzww1-? zJfcAr$Br!Iad6m;>dAheuO1czkLHy`K@+?!-WgXQp+|eMyIS0mqO%*`CT4iCFyTnL zH5c|Bm|kJ4n}3#kf*5tVihCYB{6N_iEv4Gdy3?Km9pz@RIoT@4>mIgwqiL5EVM0oY zCk(m2Q#Qr3+`W2oe7HLmQDsq&ghu&QZVib>zBk}G$b)m@_*=#y3u}#E7GGiXZ{TdF AT>t<8 delta 1977 zcmb7ETWl0n82-=f?9A@$w%y%sFWZIf?(AjS-R<7k8%4?`Mj<4%Y1IUx(6vAd(rqaP zDUxhMG!i6MGd4kqkr$&eJ`noCgCQyUYaN@Smg?$Y?Nxx*{x~1YOGP~$a~vC;DF-PWbO1c8)h4jKquzmdTsJh+x6`QqWCBhs!UIvnEh<* z@>fL0CD?q;lb2!0ziVr2tFp~1$I^&TzomiQ+=W-)J<#&lALQa5Y@hST8}meOJx~Mi zlx?yEt%LZNKfEjbW{8b9&QlYG(Vr`HgMMIs7GoFKilj(AQc*gJxEkryPebr@VWd-( zVibq$p1HAOsL?{sM+=X#LQ)}6L_x@n6u2kQg(qm3o}-Oc#Sw&Dua?)Fpe`^a!KGuy zgG3Q70G4%K5aR;$+`1DHawsFwpbZ%E_UTx5W4WJp(bH&C6C882$*Rl%;Xf^RqTVQ+ zA2ywLNx>Tdc@9<)tUj#1kBZm?*(#%q4dGkm#!R<*$+w|GjVcr=IPx}iqSX!%7-)1A zlDE6nBH&bXc{_BN~KX=(#u4hf`%HSb6cZ-V@=<`q_{9i!@_7=G$TBm*f}x-<@- z40oP6L(RMo>qje%)@RVJyk#s)C38sL#=_>C@;>udc>|C%Wp&(q-|orYhBtq{L27)I zhSYG*Z-f0~%rzt|;XOb{XzsVL`Mo`wk0V+hF%pQLv5s>O%FH5~VfCznT+&Fqid~4| zw3W?*vIF#r$<$g9-ub>nF4LQYSGHh8OBFqxs&0yHE@S*-M zL)HW$kS_@2A#veY=z}~ckbpcSkb^udkbxW*=!KjRNJ1WI@DIRmw-*J9kjDhFkdp$q zgs3iHKu!rnA#od8C_x?XE6yVq7DS=IpFA2mTPYdu4%?Xs3 zt~<^#b6D$m{uBFvecpaYDXKl{MfI*@r{k>SzH`v|yB5|y(*AN?blr1j+-Kaknx_B% zyYz*os!NMLzhmjW;4P^kt*zm{wb@?5(JvEu+x+cYSV#K+D|w3<*21E?5-+!Rv&sMq zNlFXr2(e}FpNXU9UU40 \ No newline at end of file diff --git a/assets/icons/inherit.svg b/assets/icons/inherit.svg new file mode 100644 index 0000000000..408b67c56b --- /dev/null +++ b/assets/icons/inherit.svg @@ -0,0 +1,4 @@ + + + + diff --git a/assets/icons/mute.svg b/assets/icons/mute.svg index 52f7cc5d77..96b816c878 100644 --- a/assets/icons/mute.svg +++ b/assets/icons/mute.svg @@ -1,5 +1 @@ - - - - - \ No newline at end of file + \ No newline at end of file diff --git a/assets/icons/unmute.svg b/assets/icons/unmute.svg index f82c654d29..17c368f92a 100644 --- a/assets/icons/unmute.svg +++ b/assets/icons/unmute.svg @@ -1,5 +1,5 @@ - - - - + + + + diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index ea83562085..fed77cdf90 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -54,71 +54,77 @@ abstract final class ZulipIcons { /// The Zulip custom icon "copy". static const IconData copy = IconData(0xf10a, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "follow". + static const IconData follow = IconData(0xf10b, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "format_quote". - static const IconData format_quote = IconData(0xf10b, fontFamily: "Zulip Icons"); + static const IconData format_quote = IconData(0xf10c, fontFamily: "Zulip Icons"); /// The Zulip custom icon "globe". - static const IconData globe = IconData(0xf10c, fontFamily: "Zulip Icons"); + static const IconData globe = IconData(0xf10d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "group_dm". - static const IconData group_dm = IconData(0xf10d, fontFamily: "Zulip Icons"); + static const IconData group_dm = IconData(0xf10e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_italic". - static const IconData hash_italic = IconData(0xf10e, fontFamily: "Zulip Icons"); + static const IconData hash_italic = IconData(0xf10f, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_sign". - static const IconData hash_sign = IconData(0xf10f, fontFamily: "Zulip Icons"); + static const IconData hash_sign = IconData(0xf110, fontFamily: "Zulip Icons"); /// The Zulip custom icon "image". - static const IconData image = IconData(0xf110, fontFamily: "Zulip Icons"); + static const IconData image = IconData(0xf111, fontFamily: "Zulip Icons"); /// The Zulip custom icon "inbox". - static const IconData inbox = IconData(0xf111, fontFamily: "Zulip Icons"); + static const IconData inbox = IconData(0xf112, fontFamily: "Zulip Icons"); + + /// The Zulip custom icon "inherit". + static const IconData inherit = IconData(0xf113, fontFamily: "Zulip Icons"); /// The Zulip custom icon "language". - static const IconData language = IconData(0xf112, fontFamily: "Zulip Icons"); + static const IconData language = IconData(0xf114, fontFamily: "Zulip Icons"); /// The Zulip custom icon "lock". - static const IconData lock = IconData(0xf113, fontFamily: "Zulip Icons"); + static const IconData lock = IconData(0xf115, fontFamily: "Zulip Icons"); /// The Zulip custom icon "menu". - static const IconData menu = IconData(0xf114, fontFamily: "Zulip Icons"); + static const IconData menu = IconData(0xf116, fontFamily: "Zulip Icons"); /// The Zulip custom icon "message_feed". - static const IconData message_feed = IconData(0xf115, fontFamily: "Zulip Icons"); + static const IconData message_feed = IconData(0xf117, fontFamily: "Zulip Icons"); /// The Zulip custom icon "mute". - static const IconData mute = IconData(0xf116, fontFamily: "Zulip Icons"); + static const IconData mute = IconData(0xf118, fontFamily: "Zulip Icons"); /// The Zulip custom icon "read_receipts". - static const IconData read_receipts = IconData(0xf117, fontFamily: "Zulip Icons"); + static const IconData read_receipts = IconData(0xf119, fontFamily: "Zulip Icons"); /// The Zulip custom icon "send". - static const IconData send = IconData(0xf118, fontFamily: "Zulip Icons"); + static const IconData send = IconData(0xf11a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "share". - static const IconData share = IconData(0xf119, fontFamily: "Zulip Icons"); + static const IconData share = IconData(0xf11b, fontFamily: "Zulip Icons"); /// The Zulip custom icon "share_ios". - static const IconData share_ios = IconData(0xf11a, fontFamily: "Zulip Icons"); + static const IconData share_ios = IconData(0xf11c, fontFamily: "Zulip Icons"); /// The Zulip custom icon "smile". - static const IconData smile = IconData(0xf11b, fontFamily: "Zulip Icons"); + static const IconData smile = IconData(0xf11d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star". - static const IconData star = IconData(0xf11c, fontFamily: "Zulip Icons"); + static const IconData star = IconData(0xf11e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star_filled". - static const IconData star_filled = IconData(0xf11d, fontFamily: "Zulip Icons"); + static const IconData star_filled = IconData(0xf11f, fontFamily: "Zulip Icons"); /// The Zulip custom icon "topic". - static const IconData topic = IconData(0xf11e, fontFamily: "Zulip Icons"); + static const IconData topic = IconData(0xf120, fontFamily: "Zulip Icons"); /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf11f, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf121, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf120, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf122, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } From b1767b231f359323631e4e51674107d7c5ab631b Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 25 Nov 2024 14:00:09 -0500 Subject: [PATCH 05/11] msglist: Show channel name and topic name on two rows This will eventually be superseded by #1039, so we should keep the implementation as simple as possible for now. The two-line app bar idea comes from the legacy mobile app. This gives us a place to show the topic visibility policy on the app bar. References: https://github.com/zulip/zulip-mobile/blob/a115df1f71c9dc31e9b41060a8d57b51c017d786/src/title/TitleStream.js#L113-L141 https://github.com/zulip/zulip-mobile/blob/a115df1f71c9dc31e9b41060a8d57b51c017d786/src/styles/navStyles.js#L5-L18 Signed-off-by: Zixuan James Li --- lib/widgets/message_list.dart | 51 +++++++++++++++++++++++++---- test/widgets/message_list_test.dart | 2 +- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 03085ad653..1e3066b561 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -262,6 +262,8 @@ class _MessageListPageState extends State implements MessageLis List? actions; if (narrow case TopicNarrow(:final streamId)) { + // The helper [_getEffectiveCenterTitle] relies on the fact that we + // have at most one action here. (actions ??= []).add(IconButton( icon: const Icon(ZulipIcons.message_feed), tooltip: zulipLocalizations.channelFeedButtonTooltip, @@ -314,7 +316,6 @@ class MessageListAppBarTitle extends StatelessWidget { Widget _buildStreamRow(BuildContext context, { ZulipStream? stream, - required String text, }) { // A null [Icon.icon] makes a blank space. final icon = stream != null ? iconDataForStream(stream) : null; @@ -327,10 +328,42 @@ class MessageListAppBarTitle extends StatelessWidget { children: [ Icon(size: 16, icon), const SizedBox(width: 4), - Flexible(child: Text(text)), + Flexible(child: Text(stream?.name ?? '(unknown channel)')), ]); } + Widget _buildTopicRow(BuildContext context, { + required ZulipStream? stream, + required String topic, + }) { + return Text(topic, style: const TextStyle( + fontSize: 13, + ).merge(weightVariableTextStyle(context))); + } + + // TODO(upstream): provide an API for this + // Adapted from [AppBar._getEffectiveCenterTitle]. + bool _getEffectiveCenterTitle(ThemeData theme) { + bool platformCenter() { + switch (theme.platform) { + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + return false; + case TargetPlatform.iOS: + case TargetPlatform.macOS: + // We rely on the fact that there is at most one action + // on the message list app bar, so that the expression returned + // in the original helper, `actions == null || actions!.length < 2`, + // always evaluates to `true`: + return true; + } + } + + return theme.appBarTheme.centerTitle ?? platformCenter(); + } + @override Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); @@ -348,14 +381,20 @@ class MessageListAppBarTitle extends StatelessWidget { case ChannelNarrow(:var streamId): final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; - final streamName = stream?.name ?? '(unknown channel)'; - return _buildStreamRow(context, stream: stream, text: streamName); + return _buildStreamRow(context, stream: stream); case TopicNarrow(:var streamId, :var topic): + final theme = Theme.of(context); final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; - final streamName = stream?.name ?? '(unknown channel)'; - return _buildStreamRow(context, stream: stream, text: "$streamName > $topic"); + final centerTitle = _getEffectiveCenterTitle(theme); + return Column( + crossAxisAlignment: centerTitle ? CrossAxisAlignment.center + : CrossAxisAlignment.start, + children: [ + _buildStreamRow(context, stream: stream), + _buildTopicRow(context, stream: stream, topic: topic), + ]); case DmNarrow(:var otherRecipientIds): final store = PerAccountStoreWidget.of(context); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 1bd7f798cf..3c8ba12213 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -725,7 +725,7 @@ void main() { ).length.equals(1); check(find.descendant( of: find.byType(MessageListAppBarTitle), - matching: find.text('${channel.name} > new topic')).evaluate() + matching: find.text('new topic')).evaluate() ).length.equals(1); }); }); From 8540c29b06badb4f2ab251838ae5583e3d080995 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 3 Dec 2024 19:12:08 -0500 Subject: [PATCH 06/11] msglist: Show topic visibility on app bar The design took some inspiration from the legacy mobile app. This displays a privacy level related icon (e.g.: web public, invite only). We will have a different place to show channel mute/unmute status in #347. The color for the icon is taken from the web app: https://github.com/zulip/zulip/blob/dc58c8450f8524f226115a7b449b05e01ae15d8b/web/styles/message_header.css#L296-L297 https://github.com/zulip/zulip/blob/dc58c8450f8524f226115a7b449b05e01ae15d8b/web/styles/app_variables.css#L590 https://github.com/zulip/zulip/blob/dc58c8450f8524f226115a7b449b05e01ae15d8b/web/styles/app_variables.css#L1330 In the web app, these colors are used for the topic visibility icons on message recipient headers. To maintain the different conventional title alignment on iOS and Android, we borrow some checks from AppBar in title centering. Signed-off-by: Zixuan James Li --- lib/widgets/icons.dart | 18 ++++++++++++++++++ lib/widgets/message_list.dart | 21 ++++++++++++++++++--- lib/widgets/theme.dart | 7 +++++++ test/widgets/message_list_test.dart | 16 ++++++++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index fed77cdf90..2bc1621b56 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -139,3 +139,21 @@ IconData iconDataForStream(ZulipStream stream) { ZulipStream() => ZulipIcons.hash_sign, }; } + +IconData? iconDataForTopicVisibilityPolicy(UserTopicVisibilityPolicy policy) { + switch (policy) { + case UserTopicVisibilityPolicy.muted: + return ZulipIcons.mute; + case UserTopicVisibilityPolicy.unmuted: + return ZulipIcons.unmute; + case UserTopicVisibilityPolicy.followed: + return ZulipIcons.follow; + case UserTopicVisibilityPolicy.none: + return null; + case UserTopicVisibilityPolicy.unknown: + // This case is unreachable (or should be) because we keep `unknown` out + // of our data structures. We plan to remove the `unknown` case in #1074. + assert(false); + return null; + } +} diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 1e3066b561..c892a77df4 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -336,9 +336,24 @@ class MessageListAppBarTitle extends StatelessWidget { required ZulipStream? stream, required String topic, }) { - return Text(topic, style: const TextStyle( - fontSize: 13, - ).merge(weightVariableTextStyle(context))); + final store = PerAccountStoreWidget.of(context); + final designVariables = DesignVariables.of(context); + final icon = stream == null ? null + : iconDataForTopicVisibilityPolicy( + store.topicVisibilityPolicy(stream.streamId, topic)); + return Row( + mainAxisSize: MainAxisSize.min, + children: [ + Flexible(child: Text(topic, style: const TextStyle( + fontSize: 13, + ).merge(weightVariableTextStyle(context)))), + if (icon != null) + Padding( + padding: const EdgeInsetsDirectional.only(start: 4), + child: Icon(icon, + // TODO(design) copies the recipient header in web; is there a better color? + color: designVariables.colorMessageHeaderIconInteractive, size: 14)), + ]); } // TODO(upstream): provide an API for this diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index 13bb12b391..9dbb340d56 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -136,6 +136,7 @@ class DesignVariables extends ThemeExtension { title: const Color(0xff1a1a1a), channelColorSwatches: ChannelColorSwatches.light, atMentionMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(), + colorMessageHeaderIconInteractive: Colors.black.withValues(alpha: 0.2), contextMenuCancelBg: const Color(0xff797986).withValues(alpha: 0.15), contextMenuCancelPressedBg: const Color(0xff797986).withValues(alpha: 0.20), dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.35, 0.93).toColor(), @@ -180,6 +181,7 @@ class DesignVariables extends ThemeExtension { contextMenuCancelPressedBg: const Color(0xff797986).withValues(alpha: 0.20), // the same as the light mode in Figma // TODO(design-dark) need proper dark-theme color (this is ad hoc) atMentionMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(), + colorMessageHeaderIconInteractive: Colors.white.withValues(alpha: 0.2), dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.15, 0.2).toColor(), // TODO(design-dark) need proper dark-theme color (this is ad hoc) groupDmConversationIcon: Colors.white.withValues(alpha: 0.5), @@ -225,6 +227,7 @@ class DesignVariables extends ThemeExtension { required this.title, required this.channelColorSwatches, required this.atMentionMarker, + required this.colorMessageHeaderIconInteractive, required this.contextMenuCancelBg, required this.contextMenuCancelPressedBg, required this.dmHeaderBg, @@ -278,6 +281,7 @@ class DesignVariables extends ThemeExtension { // Not named variables in Figma; taken from older Figma drafts, or elsewhere. final Color atMentionMarker; + final Color colorMessageHeaderIconInteractive; final Color contextMenuCancelBg; // In Figma, but unnamed. final Color contextMenuCancelPressedBg; // In Figma, but unnamed. final Color dmHeaderBg; @@ -318,6 +322,7 @@ class DesignVariables extends ThemeExtension { Color? title, ChannelColorSwatches? channelColorSwatches, Color? atMentionMarker, + Color? colorMessageHeaderIconInteractive, Color? contextMenuCancelBg, Color? contextMenuCancelPressedBg, Color? dmHeaderBg, @@ -357,6 +362,7 @@ class DesignVariables extends ThemeExtension { title: title ?? this.title, channelColorSwatches: channelColorSwatches ?? this.channelColorSwatches, atMentionMarker: atMentionMarker ?? this.atMentionMarker, + colorMessageHeaderIconInteractive: colorMessageHeaderIconInteractive ?? this.colorMessageHeaderIconInteractive, contextMenuCancelBg: contextMenuCancelBg ?? this.contextMenuCancelBg, contextMenuCancelPressedBg: contextMenuCancelPressedBg ?? this.contextMenuCancelPressedBg, dmHeaderBg: dmHeaderBg ?? this.dmHeaderBg, @@ -403,6 +409,7 @@ class DesignVariables extends ThemeExtension { title: Color.lerp(title, other.title, t)!, channelColorSwatches: ChannelColorSwatches.lerp(channelColorSwatches, other.channelColorSwatches, t), atMentionMarker: Color.lerp(atMentionMarker, other.atMentionMarker, t)!, + colorMessageHeaderIconInteractive: Color.lerp(colorMessageHeaderIconInteractive, other.colorMessageHeaderIconInteractive, t)!, contextMenuCancelBg: Color.lerp(contextMenuCancelBg, other.contextMenuCancelBg, t)!, contextMenuCancelPressedBg: Color.lerp(contextMenuCancelPressedBg, other.contextMenuCancelPressedBg, t)!, dmHeaderBg: Color.lerp(dmHeaderBg, other.dmHeaderBg, t)!, diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 3c8ba12213..bda13c941a 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -152,6 +152,22 @@ void main() { .page.isA().initNarrow .equals(ChannelNarrow(channel.streamId)); }); + + testWidgets('show topic visibility policy for topic narrows', (tester) async { + final channel = eg.stream(); + const topic = 'topic'; + await setupMessageListPage(tester, + narrow: TopicNarrow(channel.streamId, topic), + streams: [channel], subscriptions: [eg.subscription(channel)], + messageCount: 1); + await store.handleEvent(eg.userTopicEvent( + channel.streamId, topic, UserTopicVisibilityPolicy.muted)); + await tester.pump(); + + check(find.descendant( + of: find.byType(MessageListAppBarTitle), + matching: find.byIcon(ZulipIcons.mute))).findsOne(); + }); }); group('presents message content appropriately', () { From 7b74c065efbc82ddb44854e19dc2ee03556400df Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 11 Dec 2024 14:19:35 -0500 Subject: [PATCH 07/11] msglist [nfc]: Extract topicWidget Signed-off-by: Zixuan James Li --- lib/widgets/message_list.dart | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index c892a77df4..5612438b84 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1079,6 +1079,14 @@ class StreamMessageRecipientHeader extends StatelessWidget { ])); } + final topicWidget = Padding( + padding: const EdgeInsets.symmetric(vertical: 11), + child: Text(topic, + // TODO: Give a way to see the whole topic (maybe a + // long-press interaction?) + overflow: TextOverflow.ellipsis, + style: recipientHeaderTextStyle(context))); + return GestureDetector( onTap: () => Navigator.push(context, MessageListPage.buildRoute(context: context, @@ -1090,14 +1098,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { children: [ // TODO(#282): Long stream name will break layout; find a fix. streamWidget, - Expanded( - child: Padding( - padding: const EdgeInsets.symmetric(vertical: 11), - child: Text(topic, - // TODO: Give a way to see the whole topic (maybe a - // long-press interaction?) - overflow: TextOverflow.ellipsis, - style: recipientHeaderTextStyle(context)))), + Expanded(child: topicWidget), // TODO topic links? // Then web also has edit/resolve/mute buttons. Skip those for mobile. RecipientHeaderDate(message: message), From 585ef8b2cc39db84aee79f9c5e5150755a4508a8 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 3 Dec 2024 19:42:52 -0500 Subject: [PATCH 08/11] msglist: Show visibility policy on recipient headers This design was inspired by the legacy mobile app. Reference: https://github.com/zulip/zulip-mobile/blob/a115df1f71c9dc31e9b41060a8d57b51c017d786/src/streams/TopicItem.js#L47-L51 Signed-off-by: Zixuan James Li --- lib/widgets/message_list.dart | 21 ++++++++++++++++----- test/widgets/message_list_test.dart | 24 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 5612438b84..c1b3fd0691 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1020,6 +1020,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A20849&mode=dev // https://github.com/zulip/zulip-mobile/issues/5511 final store = PerAccountStoreWidget.of(context); + final designVariables = DesignVariables.of(context); final topic = message.topic; @@ -1081,11 +1082,21 @@ class StreamMessageRecipientHeader extends StatelessWidget { final topicWidget = Padding( padding: const EdgeInsets.symmetric(vertical: 11), - child: Text(topic, - // TODO: Give a way to see the whole topic (maybe a - // long-press interaction?) - overflow: TextOverflow.ellipsis, - style: recipientHeaderTextStyle(context))); + child: Row( + children: [ + Flexible( + child: Text(topic, + // TODO: Give a way to see the whole topic (maybe a + // long-press interaction?) + overflow: TextOverflow.ellipsis, + style: recipientHeaderTextStyle(context))), + const SizedBox(width: 4), + // TODO(design) copies the recipient header in web; is there a better color? + Icon(size: 14, color: designVariables.colorMessageHeaderIconInteractive, + // A null [Icon.icon] makes a blank space. + iconDataForTopicVisibilityPolicy( + store.topicVisibilityPolicy(message.streamId, topic))), + ])); return GestureDetector( onTap: () => Navigator.push(context, diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index bda13c941a..b1f443c8b5 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -803,6 +803,30 @@ void main() { check(findInMessageList('topic name')).length.equals(1); }); + testWidgets('show topic visibility icon when followed', (tester) async { + await setupMessageListPage(tester, + narrow: const CombinedFeedNarrow(), + messages: [message], subscriptions: [eg.subscription(stream)]); + await store.handleEvent(eg.userTopicEvent( + stream.streamId, message.topic, UserTopicVisibilityPolicy.followed)); + await tester.pump(); + check(find.descendant( + of: find.byType(MessageList), + matching: find.byIcon(ZulipIcons.follow))).findsOne(); + }); + + testWidgets('show topic visibility icon when unmuted', (tester) async { + await setupMessageListPage(tester, + narrow: TopicNarrow.ofMessage(message), + messages: [message], subscriptions: [eg.subscription(stream, isMuted: true)]); + await store.handleEvent(eg.userTopicEvent( + stream.streamId, message.topic, UserTopicVisibilityPolicy.unmuted)); + await tester.pump(); + check(find.descendant( + of: find.byType(MessageList), + matching: find.byIcon(ZulipIcons.unmute))).findsOne(); + }); + testWidgets('color of recipient header background', (tester) async { final subscription = eg.subscription(stream, color: Colors.red.argbInt); final swatch = ChannelColorSwatch.light(subscription.color); From 44045371d69f702d2587a9f4013f8df2d640b7d5 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 3 Dec 2024 20:09:11 -0500 Subject: [PATCH 09/11] inbox [nfc]: Generalize _AtMentionMarker to take different icons This is helpful for adding marker of topic visibility. Signed-off-by: Zixuan James Li --- lib/widgets/inbox.dart | 18 +++++++++++------- lib/widgets/theme.dart | 14 +++++++------- test/widgets/inbox_test.dart | 20 +++++++++++++------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index dcbd1acdb2..0f84a4373d 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -291,7 +291,7 @@ abstract class _HeaderItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - if (hasMention) const _AtMentionMarker(), + if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: unreadCountBadgeBackgroundColor(context), @@ -414,7 +414,7 @@ class _DmItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - if (hasMention) const _AtMentionMarker(), + if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge(backgroundColor: null, count: count)), @@ -539,7 +539,7 @@ class _TopicItem extends StatelessWidget { overflow: TextOverflow.ellipsis, topic))), const SizedBox(width: 12), - if (hasMention) const _AtMentionMarker(), + if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: colorSwatchFor(context, subscription), @@ -548,16 +548,20 @@ class _TopicItem extends StatelessWidget { } } -class _AtMentionMarker extends StatelessWidget { - const _AtMentionMarker(); +class _IconMarker extends StatelessWidget { + const _IconMarker({required this.icon}); + + final IconData icon; @override Widget build(BuildContext context) { final designVariables = DesignVariables.of(context); - // Design for at-mention marker based on Figma screen: + // Design for icon markers based on Figma screen: // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=224-16386&mode=design&t=JsNndFQ8fKFH0SjS-0 return Padding( padding: const EdgeInsetsDirectional.only(end: 4), - child: Icon(ZulipIcons.at_sign, size: 14, color: designVariables.atMentionMarker)); + // This color comes from the Figma screen for the "@" marker, but not + // the topic visibility markers. + child: Icon(icon, size: 14, color: designVariables.inboxItemIconMarker)); } } diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index 9dbb340d56..282decd473 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -135,13 +135,13 @@ class DesignVariables extends ThemeExtension { textInput: const Color(0xff000000), title: const Color(0xff1a1a1a), channelColorSwatches: ChannelColorSwatches.light, - atMentionMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(), colorMessageHeaderIconInteractive: Colors.black.withValues(alpha: 0.2), contextMenuCancelBg: const Color(0xff797986).withValues(alpha: 0.15), contextMenuCancelPressedBg: const Color(0xff797986).withValues(alpha: 0.20), dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.35, 0.93).toColor(), groupDmConversationIcon: Colors.black.withValues(alpha: 0.5), groupDmConversationIconBg: const Color(0x33808080), + inboxItemIconMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(), loginOrDivider: const Color(0xffdedede), loginOrDividerText: const Color(0xff575757), modalBarrierColor: const Color(0xff000000).withValues(alpha: 0.3), @@ -180,13 +180,13 @@ class DesignVariables extends ThemeExtension { contextMenuCancelBg: const Color(0xff797986).withValues(alpha: 0.15), // the same as the light mode in Figma contextMenuCancelPressedBg: const Color(0xff797986).withValues(alpha: 0.20), // the same as the light mode in Figma // TODO(design-dark) need proper dark-theme color (this is ad hoc) - atMentionMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(), colorMessageHeaderIconInteractive: Colors.white.withValues(alpha: 0.2), dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.15, 0.2).toColor(), // TODO(design-dark) need proper dark-theme color (this is ad hoc) groupDmConversationIcon: Colors.white.withValues(alpha: 0.5), // TODO(design-dark) need proper dark-theme color (this is ad hoc) groupDmConversationIconBg: const Color(0x33cccccc), + inboxItemIconMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(), loginOrDivider: const Color(0xff424242), loginOrDividerText: const Color(0xffa8a8a8), modalBarrierColor: const Color(0xff000000).withValues(alpha: 0.5), @@ -226,13 +226,13 @@ class DesignVariables extends ThemeExtension { required this.textInput, required this.title, required this.channelColorSwatches, - required this.atMentionMarker, required this.colorMessageHeaderIconInteractive, required this.contextMenuCancelBg, required this.contextMenuCancelPressedBg, required this.dmHeaderBg, required this.groupDmConversationIcon, required this.groupDmConversationIconBg, + required this.inboxItemIconMarker, required this.loginOrDivider, required this.loginOrDividerText, required this.modalBarrierColor, @@ -280,13 +280,13 @@ class DesignVariables extends ThemeExtension { final ChannelColorSwatches channelColorSwatches; // Not named variables in Figma; taken from older Figma drafts, or elsewhere. - final Color atMentionMarker; final Color colorMessageHeaderIconInteractive; final Color contextMenuCancelBg; // In Figma, but unnamed. final Color contextMenuCancelPressedBg; // In Figma, but unnamed. final Color dmHeaderBg; final Color groupDmConversationIcon; final Color groupDmConversationIconBg; + final Color inboxItemIconMarker; final Color loginOrDivider; // TODO(design-dark) need proper dark-theme color (this is ad hoc) final Color loginOrDividerText; // TODO(design-dark) need proper dark-theme color (this is ad hoc) final Color modalBarrierColor; @@ -321,13 +321,13 @@ class DesignVariables extends ThemeExtension { Color? textInput, Color? title, ChannelColorSwatches? channelColorSwatches, - Color? atMentionMarker, Color? colorMessageHeaderIconInteractive, Color? contextMenuCancelBg, Color? contextMenuCancelPressedBg, Color? dmHeaderBg, Color? groupDmConversationIcon, Color? groupDmConversationIconBg, + Color? inboxItemIconMarker, Color? loginOrDivider, Color? loginOrDividerText, Color? modalBarrierColor, @@ -361,13 +361,13 @@ class DesignVariables extends ThemeExtension { textInput: textInput ?? this.textInput, title: title ?? this.title, channelColorSwatches: channelColorSwatches ?? this.channelColorSwatches, - atMentionMarker: atMentionMarker ?? this.atMentionMarker, colorMessageHeaderIconInteractive: colorMessageHeaderIconInteractive ?? this.colorMessageHeaderIconInteractive, contextMenuCancelBg: contextMenuCancelBg ?? this.contextMenuCancelBg, contextMenuCancelPressedBg: contextMenuCancelPressedBg ?? this.contextMenuCancelPressedBg, dmHeaderBg: dmHeaderBg ?? this.dmHeaderBg, groupDmConversationIcon: groupDmConversationIcon ?? this.groupDmConversationIcon, groupDmConversationIconBg: groupDmConversationIconBg ?? this.groupDmConversationIconBg, + inboxItemIconMarker: inboxItemIconMarker ?? this.inboxItemIconMarker, loginOrDivider: loginOrDivider ?? this.loginOrDivider, loginOrDividerText: loginOrDividerText ?? this.loginOrDividerText, modalBarrierColor: modalBarrierColor ?? this.modalBarrierColor, @@ -408,13 +408,13 @@ class DesignVariables extends ThemeExtension { textInput: Color.lerp(textInput, other.textInput, t)!, title: Color.lerp(title, other.title, t)!, channelColorSwatches: ChannelColorSwatches.lerp(channelColorSwatches, other.channelColorSwatches, t), - atMentionMarker: Color.lerp(atMentionMarker, other.atMentionMarker, t)!, colorMessageHeaderIconInteractive: Color.lerp(colorMessageHeaderIconInteractive, other.colorMessageHeaderIconInteractive, t)!, contextMenuCancelBg: Color.lerp(contextMenuCancelBg, other.contextMenuCancelBg, t)!, contextMenuCancelPressedBg: Color.lerp(contextMenuCancelPressedBg, other.contextMenuCancelPressedBg, t)!, dmHeaderBg: Color.lerp(dmHeaderBg, other.dmHeaderBg, t)!, groupDmConversationIcon: Color.lerp(groupDmConversationIcon, other.groupDmConversationIcon, t)!, groupDmConversationIconBg: Color.lerp(groupDmConversationIconBg, other.groupDmConversationIconBg, t)!, + inboxItemIconMarker: Color.lerp(inboxItemIconMarker, other.inboxItemIconMarker, t)!, loginOrDivider: Color.lerp(loginOrDivider, other.loginOrDivider, t)!, loginOrDividerText: Color.lerp(loginOrDividerText, other.loginOrDividerText, t)!, modalBarrierColor: Color.lerp(modalBarrierColor, other.modalBarrierColor, t)!, diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index 12e2c78800..82cc216189 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -182,6 +182,17 @@ void main() { )); } + bool hasIcon(WidgetTester tester, { + required Widget? parent, + required IconData icon, + }) { + check(parent).isNotNull(); + return tester.widgetList(find.descendant( + of: find.byWidget(parent!), + matching: find.byIcon(icon), + )).isNotEmpty; + } + group('InboxPage', () { testWidgets('page builds; empty', (tester) async { await setupPage(tester, unreadMessages: []); @@ -246,13 +257,8 @@ void main() { final subscription = eg.subscription(stream); const topic = 'lunch'; - bool hasAtSign(WidgetTester tester, Widget? parent) { - check(parent).isNotNull(); - return tester.widgetList(find.descendant( - of: find.byWidget(parent!), - matching: find.byIcon(ZulipIcons.at_sign), - )).isNotEmpty; - } + bool hasAtSign(WidgetTester tester, Widget? parent) => + hasIcon(tester, parent: parent, icon: ZulipIcons.at_sign); testWidgets('topic with a mention', (tester) async { await setupPage(tester, From 9f18c53a6457e09548805ff04d4d12a78aeed333 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 3 Dec 2024 20:40:10 -0500 Subject: [PATCH 10/11] inbox: Show topic visibility marker for topic items Signed-off-by: Zixuan James Li --- lib/widgets/inbox.dart | 4 +++ test/widgets/inbox_test.dart | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 0f84a4373d..a8c70bee53 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -514,6 +514,8 @@ class _TopicItem extends StatelessWidget { final subscription = store.subscriptions[streamId]!; final designVariables = DesignVariables.of(context); + final visibilityIcon = iconDataForTopicVisibilityPolicy( + store.topicVisibilityPolicy(streamId, topic)); return Material( color: designVariables.background, // TODO(design) check if this is the right variable @@ -540,6 +542,8 @@ class _TopicItem extends StatelessWidget { topic))), const SizedBox(width: 12), if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), + // TODO(design) copies the "@" marker color; is there a better color? + if (visibilityIcon != null) _IconMarker(icon: visibilityIcon), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: colorSwatchFor(context, subscription), diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index 82cc216189..b7ddbd7e23 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -305,6 +305,53 @@ void main() { }); }); + group('topic visibility', () { + final channel = eg.stream(); + const topic = 'topic'; + final message = eg.streamMessage(stream: channel, topic: topic); + + testWidgets('followed', (tester) async { + await setupPage(tester, + streams: [channel], + subscriptions: [eg.subscription(channel)], + unreadMessages: [message]); + await store.addUserTopic(channel, topic, UserTopicVisibilityPolicy.followed); + await tester.pump(); + check(hasIcon(tester, + parent: findRowByLabel(tester, topic), + icon: ZulipIcons.follow)).isTrue(); + }); + + testWidgets('followed and mentioned', (tester) async { + await setupPage(tester, + streams: [channel], + subscriptions: [eg.subscription(channel)], + unreadMessages: [eg.streamMessage(stream: channel, topic: topic, + flags: [MessageFlag.mentioned])]); + await store.addUserTopic(channel, topic, UserTopicVisibilityPolicy.followed); + await tester.pump(); + check(hasIcon(tester, + parent: findRowByLabel(tester, topic), + icon: ZulipIcons.follow)).isTrue(); + check(hasIcon(tester, + parent: findRowByLabel(tester, topic), + icon: ZulipIcons.at_sign)).isTrue(); + }); + + testWidgets('unmuted', (tester) async { + await setupPage(tester, + users: [eg.selfUser, eg.otherUser], + streams: [channel], + subscriptions: [eg.subscription(channel, isMuted: true)], + unreadMessages: [message]); + await store.addUserTopic(channel, topic, UserTopicVisibilityPolicy.unmuted); + await tester.pump(); + check(hasIcon(tester, + parent: findRowByLabel(tester, topic), + icon: ZulipIcons.unmute)).isTrue(); + }); + }); + group('collapsing', () { Icon findHeaderCollapseIcon(WidgetTester tester, Widget headerRow) { return tester.widget( From c5f1cd465f9bbb98b1eab6ba87b6b4423e3a6843 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 24 Oct 2024 18:55:08 -0400 Subject: [PATCH 11/11] action_sheet: Support muting/unmuting/following topics Currently, we don't have buttons, like "resolve topic", other than the ones added here. The switch statements follow the layout of the legacy app implementation. See also: https://github.com/zulip/zulip-mobile/blob/715d60a5e87fe37032bce58bd72edb99208e15be/src/action-sheets/index.js#L656-L753 Fixes: #348 Signed-off-by: Zixuan James Li --- assets/l10n/app_en.arb | 32 ++ lib/api/model/model.dart | 2 +- lib/generated/l10n/zulip_localizations.dart | 48 +++ .../l10n/zulip_localizations_ar.dart | 24 ++ .../l10n/zulip_localizations_en.dart | 24 ++ .../l10n/zulip_localizations_fr.dart | 24 ++ .../l10n/zulip_localizations_ja.dart | 24 ++ .../l10n/zulip_localizations_pl.dart | 24 ++ .../l10n/zulip_localizations_ru.dart | 24 ++ lib/widgets/action_sheet.dart | 222 ++++++++++++++ lib/widgets/inbox.dart | 3 + lib/widgets/message_list.dart | 22 +- test/widgets/action_sheet_test.dart | 283 ++++++++++++++++++ 13 files changed, 748 insertions(+), 8 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 1b8e6ff8b8..489ec8e972 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -59,6 +59,22 @@ "@permissionsDeniedReadExternalStorage": { "description": "Message for dialog asking the user to grant permissions for external storage read access." }, + "actionSheetOptionMuteTopic": "Mute topic", + "@actionSheetOptionMuteTopic": { + "description": "Label for muting a topic on action sheet." + }, + "actionSheetOptionUnmuteTopic": "Unmute topic", + "@actionSheetOptionUnmuteTopic": { + "description": "Label for unmuting a topic on action sheet." + }, + "actionSheetOptionFollowTopic": "Follow topic", + "@actionSheetOptionFollowTopic": { + "description": "Label for following a topic on action sheet." + }, + "actionSheetOptionUnfollowTopic": "Unfollow topic", + "@actionSheetOptionUnfollowTopic": { + "description": "Label for unfollowing a topic on action sheet." + }, "actionSheetOptionCopyMessageText": "Copy message text", "@actionSheetOptionCopyMessageText": { "description": "Label for copy message text button on action sheet." @@ -201,6 +217,22 @@ "event": {"type": "String", "example": "UpdateMessageEvent(id: 123, messageIds: [2345, 3456], newTopic: 'dinner')"} } }, + "errorMuteTopicFailed": "Failed to mute topic", + "@errorMuteTopicFailed": { + "description": "Error message when muting a topic failed." + }, + "errorUnmuteTopicFailed": "Failed to unmute topic", + "@errorUnmuteTopicFailed": { + "description": "Error message when unmuting a topic failed." + }, + "errorFollowTopicFailed": "Failed to follow topic", + "@errorFollowTopicFailed": { + "description": "Error message when following a topic failed." + }, + "errorUnfollowTopicFailed": "Failed to unfollow topic", + "@errorUnfollowTopicFailed": { + "description": "Error message when unfollowing a topic failed." + }, "errorSharingFailed": "Sharing failed", "@errorSharingFailed": { "description": "Error message when sharing a message failed." diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index d009bb9f29..b36e1f2490 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -493,7 +493,7 @@ enum UserTopicVisibilityPolicy { muted(apiValue: 1), unmuted(apiValue: 2), // TODO(server-7) newly added followed(apiValue: 3), // TODO(server-8) newly added - unknown(apiValue: null); + unknown(apiValue: null); // TODO(#1074) remove this const UserTopicVisibilityPolicy({required this.apiValue}); diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 53936a926d..6fa55bbce6 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -193,6 +193,30 @@ abstract class ZulipLocalizations { /// **'To upload files, please grant Zulip additional permissions in Settings.'** String get permissionsDeniedReadExternalStorage; + /// Label for muting a topic on action sheet. + /// + /// In en, this message translates to: + /// **'Mute topic'** + String get actionSheetOptionMuteTopic; + + /// Label for unmuting a topic on action sheet. + /// + /// In en, this message translates to: + /// **'Unmute topic'** + String get actionSheetOptionUnmuteTopic; + + /// Label for following a topic on action sheet. + /// + /// In en, this message translates to: + /// **'Follow topic'** + String get actionSheetOptionFollowTopic; + + /// Label for unfollowing a topic on action sheet. + /// + /// In en, this message translates to: + /// **'Unfollow topic'** + String get actionSheetOptionUnfollowTopic; + /// Label for copy message text button on action sheet. /// /// In en, this message translates to: @@ -361,6 +385,30 @@ abstract class ZulipLocalizations { /// **'Error handling a Zulip event from {serverUrl}; will retry.\n\nError: {error}\n\nEvent: {event}'** String errorHandlingEventDetails(String serverUrl, String error, String event); + /// Error message when muting a topic failed. + /// + /// In en, this message translates to: + /// **'Failed to mute topic'** + String get errorMuteTopicFailed; + + /// Error message when unmuting a topic failed. + /// + /// In en, this message translates to: + /// **'Failed to unmute topic'** + String get errorUnmuteTopicFailed; + + /// Error message when following a topic failed. + /// + /// In en, this message translates to: + /// **'Failed to follow topic'** + String get errorFollowTopicFailed; + + /// Error message when unfollowing a topic failed. + /// + /// In en, this message translates to: + /// **'Failed to unfollow topic'** + String get errorUnfollowTopicFailed; + /// Error message when sharing a message failed. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 95ff1d0aea..411e904316 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -53,6 +53,18 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMuteTopic => 'Mute topic'; + + @override + String get actionSheetOptionUnmuteTopic => 'Unmute topic'; + + @override + String get actionSheetOptionFollowTopic => 'Follow topic'; + + @override + String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; @@ -165,6 +177,18 @@ class ZulipLocalizationsAr extends ZulipLocalizations { return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event'; } + @override + String get errorMuteTopicFailed => 'Failed to mute topic'; + + @override + String get errorUnmuteTopicFailed => 'Failed to unmute topic'; + + @override + String get errorFollowTopicFailed => 'Failed to follow topic'; + + @override + String get errorUnfollowTopicFailed => 'Failed to unfollow topic'; + @override String get errorSharingFailed => 'Sharing failed'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index d440ed2b10..d2708842a2 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -53,6 +53,18 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMuteTopic => 'Mute topic'; + + @override + String get actionSheetOptionUnmuteTopic => 'Unmute topic'; + + @override + String get actionSheetOptionFollowTopic => 'Follow topic'; + + @override + String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; @@ -165,6 +177,18 @@ class ZulipLocalizationsEn extends ZulipLocalizations { return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event'; } + @override + String get errorMuteTopicFailed => 'Failed to mute topic'; + + @override + String get errorUnmuteTopicFailed => 'Failed to unmute topic'; + + @override + String get errorFollowTopicFailed => 'Failed to follow topic'; + + @override + String get errorUnfollowTopicFailed => 'Failed to unfollow topic'; + @override String get errorSharingFailed => 'Sharing failed'; diff --git a/lib/generated/l10n/zulip_localizations_fr.dart b/lib/generated/l10n/zulip_localizations_fr.dart index 0caaba81f9..4b5a4734b4 100644 --- a/lib/generated/l10n/zulip_localizations_fr.dart +++ b/lib/generated/l10n/zulip_localizations_fr.dart @@ -53,6 +53,18 @@ class ZulipLocalizationsFr extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMuteTopic => 'Mute topic'; + + @override + String get actionSheetOptionUnmuteTopic => 'Unmute topic'; + + @override + String get actionSheetOptionFollowTopic => 'Follow topic'; + + @override + String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; @@ -165,6 +177,18 @@ class ZulipLocalizationsFr extends ZulipLocalizations { return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event'; } + @override + String get errorMuteTopicFailed => 'Failed to mute topic'; + + @override + String get errorUnmuteTopicFailed => 'Failed to unmute topic'; + + @override + String get errorFollowTopicFailed => 'Failed to follow topic'; + + @override + String get errorUnfollowTopicFailed => 'Failed to unfollow topic'; + @override String get errorSharingFailed => 'Sharing failed'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 42128ba024..18a4a5a3ee 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -53,6 +53,18 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMuteTopic => 'Mute topic'; + + @override + String get actionSheetOptionUnmuteTopic => 'Unmute topic'; + + @override + String get actionSheetOptionFollowTopic => 'Follow topic'; + + @override + String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; @@ -165,6 +177,18 @@ class ZulipLocalizationsJa extends ZulipLocalizations { return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event'; } + @override + String get errorMuteTopicFailed => 'Failed to mute topic'; + + @override + String get errorUnmuteTopicFailed => 'Failed to unmute topic'; + + @override + String get errorFollowTopicFailed => 'Failed to follow topic'; + + @override + String get errorUnfollowTopicFailed => 'Failed to unfollow topic'; + @override String get errorSharingFailed => 'Sharing failed'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 03d4c91302..8936470ad5 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -53,6 +53,18 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'Aby odebrać pliki Zulip musi uzyskać dodatkowe uprawnienia w Ustawieniach.'; + @override + String get actionSheetOptionMuteTopic => 'Mute topic'; + + @override + String get actionSheetOptionUnmuteTopic => 'Unmute topic'; + + @override + String get actionSheetOptionFollowTopic => 'Follow topic'; + + @override + String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override String get actionSheetOptionCopyMessageText => 'Skopiuj tekst wiadomości'; @@ -165,6 +177,18 @@ class ZulipLocalizationsPl extends ZulipLocalizations { return 'Błąd zdarzenia Zulip z $serverUrl; ponawiam.\n\nBłąd: $error\n\nZdarzenie: $event'; } + @override + String get errorMuteTopicFailed => 'Failed to mute topic'; + + @override + String get errorUnmuteTopicFailed => 'Failed to unmute topic'; + + @override + String get errorFollowTopicFailed => 'Failed to follow topic'; + + @override + String get errorUnfollowTopicFailed => 'Failed to unfollow topic'; + @override String get errorSharingFailed => 'Udostępnianie bez powodzenia'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 025f940e46..195451c5a6 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -53,6 +53,18 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'Для загрузки файлов, пожалуйста, предоставьте Zulip дополнительные разрешения в настройках.'; + @override + String get actionSheetOptionMuteTopic => 'Mute topic'; + + @override + String get actionSheetOptionUnmuteTopic => 'Unmute topic'; + + @override + String get actionSheetOptionFollowTopic => 'Follow topic'; + + @override + String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override String get actionSheetOptionCopyMessageText => 'Скопировать текст сообщения'; @@ -165,6 +177,18 @@ class ZulipLocalizationsRu extends ZulipLocalizations { return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event'; } + @override + String get errorMuteTopicFailed => 'Failed to mute topic'; + + @override + String get errorUnmuteTopicFailed => 'Failed to unmute topic'; + + @override + String get errorFollowTopicFailed => 'Failed to follow topic'; + + @override + String get errorUnfollowTopicFailed => 'Failed to unfollow topic'; + @override String get errorSharingFailed => 'Sharing failed'; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 1b6c8b2e58..4a6d63b032 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -7,6 +7,7 @@ import 'package:share_plus/share_plus.dart'; import '../api/exception.dart'; import '../api/model/model.dart'; +import '../api/route/channels.dart'; import '../api/route/messages.dart'; import '../generated/l10n/zulip_localizations.dart'; import '../model/internal_link.dart'; @@ -143,6 +144,227 @@ class ActionSheetCancelButton extends StatelessWidget { } } +/// Show a sheet of actions you can take on a topic. +void showTopicActionSheet(BuildContext context, { + required int channelId, + required String topic, +}) { + final narrow = TopicNarrow(channelId, topic); + UserTopicUpdateButton button({ + UserTopicVisibilityPolicy? from, + required UserTopicVisibilityPolicy to, + }) { + return UserTopicUpdateButton( + currentVisibilityPolicy: from, + newVisibilityPolicy: to, + narrow: narrow, + pageContext: context); + } + + final mute = button(to: UserTopicVisibilityPolicy.muted); + final unmute = button(from: UserTopicVisibilityPolicy.muted, + to: UserTopicVisibilityPolicy.none); + final unmuteInMutedChannel = button(to: UserTopicVisibilityPolicy.unmuted); + final follow = button(to: UserTopicVisibilityPolicy.followed); + final unfollow = button(from: UserTopicVisibilityPolicy.followed, + to: UserTopicVisibilityPolicy.none); + + final store = PerAccountStoreWidget.of(context); + final channelMuted = store.subscriptions[channelId]?.isMuted; + final visibilityPolicy = store.topicVisibilityPolicy(channelId, topic); + + // TODO(server-7): simplify this condition away + final supportsUnmutingTopics = store.connection.zulipFeatureLevel! >= 170; + // TODO(server-8): simplify this condition away + final supportsFollowingTopics = store.connection.zulipFeatureLevel! >= 219; + + final optionButtons = []; + if (channelMuted != null && !channelMuted) { + // Channel is subscribed and not muted. + switch (visibilityPolicy) { + case UserTopicVisibilityPolicy.muted: + optionButtons.add(unmute); + if (supportsFollowingTopics) { + optionButtons.add(follow); + } + case UserTopicVisibilityPolicy.none: + case UserTopicVisibilityPolicy.unmuted: + optionButtons.add(mute); + if (supportsFollowingTopics) { + optionButtons.add(follow); + } + case UserTopicVisibilityPolicy.followed: + optionButtons.add(mute); + if (supportsFollowingTopics) { + optionButtons.add(unfollow); + } + case UserTopicVisibilityPolicy.unknown: + // TODO(#1074): This should be unreachable as we keep `unknown` out of + // our data structures. + assert(false); + } + } else if (channelMuted != null && channelMuted) { + // Channel is muted. + if (supportsUnmutingTopics) { + switch (visibilityPolicy) { + case UserTopicVisibilityPolicy.none: + case UserTopicVisibilityPolicy.muted: + optionButtons.add(unmuteInMutedChannel); + if (supportsFollowingTopics) { + optionButtons.add(follow); + } + case UserTopicVisibilityPolicy.unmuted: + optionButtons.add(mute); + if (supportsFollowingTopics) { + optionButtons.add(follow); + } + case UserTopicVisibilityPolicy.followed: + optionButtons.add(mute); + if (supportsFollowingTopics) { + optionButtons.add(unfollow); + } + case UserTopicVisibilityPolicy.unknown: + // TODO(#1074): This should be unreachable as we keep `unknown` out of + // our data structures. + assert(false); + } + } + } else { + // Not subscribed to the channel; there is no user topic change to be made. + } + + if (optionButtons.isEmpty) { + // TODO(a11y): This case makes a no-op gesture handler; as a consequence, + // we're presenting some UI (to people who use screen-reader software) as + // though it offers a gesture interaction that it doesn't meaningfully + // offer, which is confusing. The solution here is probably to remove this + // is-empty case by having at least one button that's always present, + // such as "copy link to topic". + return; + } + + _showActionSheet(context, optionButtons: optionButtons); +} + +class UserTopicUpdateButton extends ActionSheetMenuItemButton { + const UserTopicUpdateButton({ + super.key, + this.currentVisibilityPolicy, + required this.newVisibilityPolicy, + required this.narrow, + required super.pageContext, + }); + + final UserTopicVisibilityPolicy? currentVisibilityPolicy; + final UserTopicVisibilityPolicy newVisibilityPolicy; + final TopicNarrow narrow; + + @override IconData get icon { + switch (newVisibilityPolicy) { + case UserTopicVisibilityPolicy.none: + return ZulipIcons.inherit; + case UserTopicVisibilityPolicy.muted: + return ZulipIcons.mute; + case UserTopicVisibilityPolicy.unmuted: + return ZulipIcons.unmute; + case UserTopicVisibilityPolicy.followed: + return ZulipIcons.follow; + case UserTopicVisibilityPolicy.unknown: + // TODO(#1074): This should be unreachable as we keep `unknown` out of + // our data structures. + assert(false); + return ZulipIcons.inherit; + } + } + + @override + String label(ZulipLocalizations zulipLocalizations) { + switch ((currentVisibilityPolicy, newVisibilityPolicy)) { + case (UserTopicVisibilityPolicy.muted, UserTopicVisibilityPolicy.none): + return zulipLocalizations.actionSheetOptionUnmuteTopic; + case (UserTopicVisibilityPolicy.followed, UserTopicVisibilityPolicy.none): + return zulipLocalizations.actionSheetOptionUnfollowTopic; + + case (_, UserTopicVisibilityPolicy.muted): + return zulipLocalizations.actionSheetOptionMuteTopic; + case (_, UserTopicVisibilityPolicy.unmuted): + return zulipLocalizations.actionSheetOptionUnmuteTopic; + case (_, UserTopicVisibilityPolicy.followed): + return zulipLocalizations.actionSheetOptionFollowTopic; + + case (_, UserTopicVisibilityPolicy.none): + // This is unexpected because `UserTopicVisibilityPolicy.muted` and + // `UserTopicVisibilityPolicy.followed` (handled in separate `case`'s) + // are the only expected `currentVisibilityPolicy` + // when `newVisibilityPolicy` is `UserTopicVisibilityPolicy.none`. + assert(false); + return ''; + + case (_, UserTopicVisibilityPolicy.unknown): + // This case is unreachable (or should be) because we keep `unknown` out + // of our data structures. We plan to remove the `unknown` case in #1074. + assert(false); + return ''; + } + } + + String _errorTitle(ZulipLocalizations zulipLocalizations) { + switch ((currentVisibilityPolicy, newVisibilityPolicy)) { + case (UserTopicVisibilityPolicy.muted, UserTopicVisibilityPolicy.none): + return zulipLocalizations.errorUnmuteTopicFailed; + case (UserTopicVisibilityPolicy.followed, UserTopicVisibilityPolicy.none): + return zulipLocalizations.errorUnfollowTopicFailed; + + case (_, UserTopicVisibilityPolicy.muted): + return zulipLocalizations.errorMuteTopicFailed; + case (_, UserTopicVisibilityPolicy.unmuted): + return zulipLocalizations.errorUnmuteTopicFailed; + case (_, UserTopicVisibilityPolicy.followed): + return zulipLocalizations.errorFollowTopicFailed; + + case (_, UserTopicVisibilityPolicy.none): + // This is unexpected because `UserTopicVisibilityPolicy.muted` and + // `UserTopicVisibilityPolicy.followed` (handled in separate `case`'s) + // are the only expected `currentVisibilityPolicy` + // when `newVisibilityPolicy` is `UserTopicVisibilityPolicy.none`. + assert(false); + return ''; + + case (_, UserTopicVisibilityPolicy.unknown): + // This case is unreachable (or should be) because we keep `unknown` out + // of our data structures. We plan to remove the `unknown` case in #1074. + assert(false); + return ''; + } + } + + @override void onPressed() async { + try { + await updateUserTopicCompat( + PerAccountStoreWidget.of(pageContext).connection, + streamId: narrow.streamId, + topic: narrow.topic, + visibilityPolicy: newVisibilityPolicy); + } catch (e) { + if (!pageContext.mounted) return; + + String? errorMessage; + + switch (e) { + case ZulipApiException(): + errorMessage = e.message; + // TODO(#741) specific messages for common errors, like network errors + // (support with reusable code) + default: + } + + final zulipLocalizations = ZulipLocalizations.of(pageContext); + showErrorDialog(context: pageContext, + title: _errorTitle(zulipLocalizations), message: errorMessage); + } + } +} + /// Show a sheet of actions you can take on a message in the message list. /// /// Must have a [MessageListPage] ancestor. diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index a8c70bee53..f276a1b5cd 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -4,6 +4,7 @@ import '../api/model/model.dart'; import '../model/narrow.dart'; import '../model/recent_dm_conversations.dart'; import '../model/unreads.dart'; +import 'action_sheet.dart'; import 'app_bar.dart'; import 'icons.dart'; import 'message_list.dart'; @@ -525,6 +526,8 @@ class _TopicItem extends StatelessWidget { Navigator.push(context, MessageListPage.buildRoute(context: context, narrow: narrow)); }, + onLongPress: () => showTopicActionSheet(context, + channelId: streamId, topic: topic), child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 34), child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ const SizedBox(width: 63), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index c1b3fd0691..8c32a12115 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -403,13 +403,19 @@ class MessageListAppBarTitle extends StatelessWidget { final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; final centerTitle = _getEffectiveCenterTitle(theme); - return Column( - crossAxisAlignment: centerTitle ? CrossAxisAlignment.center - : CrossAxisAlignment.start, - children: [ - _buildStreamRow(context, stream: stream), - _buildTopicRow(context, stream: stream, topic: topic), - ]); + return SizedBox( + width: double.infinity, + child: GestureDetector( + behavior: HitTestBehavior.translucent, + onLongPress: () => showTopicActionSheet(context, + channelId: streamId, topic: topic), + child: Column( + crossAxisAlignment: centerTitle ? CrossAxisAlignment.center + : CrossAxisAlignment.start, + children: [ + _buildStreamRow(context, stream: stream), + _buildTopicRow(context, stream: stream, topic: topic), + ]))); case DmNarrow(:var otherRecipientIds): final store = PerAccountStoreWidget.of(context); @@ -1102,6 +1108,8 @@ class StreamMessageRecipientHeader extends StatelessWidget { onTap: () => Navigator.push(context, MessageListPage.buildRoute(context: context, narrow: TopicNarrow.ofMessage(message))), + onLongPress: () => showTopicActionSheet(context, + channelId: message.streamId, topic: topic), child: ColoredBox( color: backgroundColor, child: Row( diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index b47a49dba9..6ca5fa1fbd 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/model/events.dart'; @@ -16,9 +17,12 @@ import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/model/typing_status.dart'; +import 'package:zulip/widgets/action_sheet.dart'; +import 'package:zulip/widgets/app_bar.dart'; import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/icons.dart'; +import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:share_plus_platform_interface/method_channel/method_channel_share.dart'; import '../api/fake_api.dart'; @@ -99,6 +103,285 @@ void main() { connection.prepare(httpStatus: 400, json: fakeResponseJson); } + group('showTopicActionSheet', () { + final channel = eg.stream(); + const topic = 'my topic'; + final message = eg.streamMessage( + stream: channel, topic: topic, sender: eg.otherUser); + + Future prepare() async { + addTearDown(testBinding.reset); + + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot( + realmUsers: [eg.selfUser, eg.otherUser], + streams: [channel], + subscriptions: [eg.subscription(channel)])); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + connection = store.connection as FakeApiConnection; + + await store.addMessage(message); + } + + testWidgets('show from inbox', (tester) async { + await prepare(); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const InboxPage())); + await tester.pump(); + + await tester.longPress(find.text(topic)); + // sheet appears onscreen; default duration of bottom-sheet enter animation + await tester.pump(const Duration(milliseconds: 250)); + check(find.byType(BottomSheet)).findsOne(); + check(find.text('Follow topic')).findsOne(); + }); + + testWidgets('show from app bar', (tester) async { + await prepare(); + connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: [message]).toJson()); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: MessageListPage( + initNarrow: TopicNarrow(channel.streamId, topic)))); + // global store, per-account store, and message list get loaded + await tester.pumpAndSettle(); + + await tester.longPress(find.byType(ZulipAppBar)); + // sheet appears onscreen; default duration of bottom-sheet enter animation + await tester.pump(const Duration(milliseconds: 250)); + check(find.byType(BottomSheet)).findsOne(); + check(find.text('Follow topic')).findsOne(); + }); + + testWidgets('show from recipient header', (tester) async { + await prepare(); + connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: [message]).toJson()); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const MessageListPage(initNarrow: CombinedFeedNarrow()))); + // global store, per-account store, and message list get loaded + await tester.pumpAndSettle(); + + await tester.longPress(find.descendant( + of: find.byType(RecipientHeader), matching: find.text(topic))); + // sheet appears onscreen; default duration of bottom-sheet enter animation + await tester.pump(const Duration(milliseconds: 250)); + check(find.byType(BottomSheet)).findsOne(); + check(find.text('Follow topic')).findsOne(); + }); + }); + + group('UserTopicUpdateButton', () { + late ZulipStream channel; + late String topic; + + final mute = find.text('Mute topic'); + final unmute = find.text('Unmute topic'); + final follow = find.text('Follow topic'); + final unfollow = find.text('Unfollow topic'); + + /// Prepare store and bring up a topic action sheet. + /// + /// If `isChannelMuted` is `null`, the user is not subscribed to the + /// channel. + Future setupToTopicActionSheet(WidgetTester tester, { + required bool? isChannelMuted, + required UserTopicVisibilityPolicy visibilityPolicy, + int? zulipFeatureLevel, + }) async { + addTearDown(testBinding.reset); + + channel = eg.stream(); + topic = 'isChannelMuted: $isChannelMuted, policy: $visibilityPolicy'; + + final account = eg.selfAccount.copyWith(zulipFeatureLevel: zulipFeatureLevel); + final subscriptions = isChannelMuted == null ? [] + : [eg.subscription(channel, isMuted: isChannelMuted)]; + await testBinding.globalStore.add(account, eg.initialSnapshot( + realmUsers: [eg.selfUser], + streams: [channel], + subscriptions: subscriptions, + userTopics: [eg.userTopicItem(channel, topic, visibilityPolicy)], + zulipFeatureLevel: zulipFeatureLevel)); + store = await testBinding.globalStore.perAccount(account.id); + connection = store.connection as FakeApiConnection; + + connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: [ + eg.streamMessage(stream: channel, topic: topic)]).toJson()); + await tester.pumpWidget(TestZulipApp(accountId: account.id, + child: MessageListPage( + initNarrow: TopicNarrow(channel.streamId, topic)))); + await tester.pumpAndSettle(); + + await tester.longPress(find.descendant( + of: find.byType(RecipientHeader), matching: find.text(topic))); + // sheet appears onscreen; default duration of bottom-sheet enter animation + await tester.pump(const Duration(milliseconds: 250)); + } + + void checkButtons(List expectedButtonFinders) { + if (expectedButtonFinders.isEmpty) { + check(find.byType(BottomSheet)).findsNothing(); + return; + } + check(find.byType(BottomSheet)).findsOne(); + + for (final buttonFinder in expectedButtonFinders) { + check(buttonFinder).findsOne(); + } + check(find.bySubtype()) + .findsExactly(expectedButtonFinders.length); + } + + void checkUpdateUserTopicRequest(UserTopicVisibilityPolicy expectedPolicy) async { + check(connection.lastRequest).isA() + ..url.path.equals('/api/v1/user_topics') + ..bodyFields.deepEquals({ + 'stream_id': '${channel.streamId}', + 'topic': topic, + 'visibility_policy': jsonEncode(expectedPolicy), + }); + } + + testWidgets('unmuteInMutedChannel', (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: true, + visibilityPolicy: UserTopicVisibilityPolicy.none); + await tester.tap(unmute); + await tester.pump(); + checkUpdateUserTopicRequest(UserTopicVisibilityPolicy.unmuted); + }); + + testWidgets('unmute', (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: false, + visibilityPolicy: UserTopicVisibilityPolicy.muted); + await tester.tap(unmute); + await tester.pump(); + checkUpdateUserTopicRequest(UserTopicVisibilityPolicy.none); + }); + + testWidgets('mute', (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: false, + visibilityPolicy: UserTopicVisibilityPolicy.none); + await tester.tap(mute); + await tester.pump(); + checkUpdateUserTopicRequest(UserTopicVisibilityPolicy.muted); + }); + + testWidgets('follow', (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: false, + visibilityPolicy: UserTopicVisibilityPolicy.none); + await tester.tap(follow); + await tester.pump(); + checkUpdateUserTopicRequest(UserTopicVisibilityPolicy.followed); + }); + + testWidgets('unfollow', (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: false, + visibilityPolicy: UserTopicVisibilityPolicy.followed); + await tester.tap(unfollow); + await tester.pump(); + checkUpdateUserTopicRequest(UserTopicVisibilityPolicy.none); + }); + + testWidgets('request fails with an error dialog', (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: false, + visibilityPolicy: UserTopicVisibilityPolicy.followed); + + connection.prepare(httpStatus: 400, json: { + 'result': 'error', 'code': 'BAD_REQUEST', 'msg': ''}); + await tester.tap(unfollow); + await tester.pumpAndSettle(); + + checkErrorDialog(tester, expectedTitle: 'Failed to unfollow topic'); + }); + + group('check expected buttons', () { + final testCases = [ + (false, UserTopicVisibilityPolicy.muted, [unmute, follow]), + (false, UserTopicVisibilityPolicy.none, [mute, follow]), + (false, UserTopicVisibilityPolicy.unmuted, [mute, follow]), + (false, UserTopicVisibilityPolicy.followed, [mute, unfollow]), + + (true, UserTopicVisibilityPolicy.muted, [unmute, follow]), + (true, UserTopicVisibilityPolicy.none, [unmute, follow]), + (true, UserTopicVisibilityPolicy.unmuted, [mute, follow]), + (true, UserTopicVisibilityPolicy.followed, [mute, unfollow]), + + (null, UserTopicVisibilityPolicy.none, []), + ]; + + for (final (isChannelMuted, visibilityPolicy, buttons) in testCases) { + final description = 'isChannelMuted: ${isChannelMuted ?? "(not subscribed)"}, $visibilityPolicy'; + testWidgets(description, (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: isChannelMuted, + visibilityPolicy: visibilityPolicy); + checkButtons(buttons); + }); + } + }); + + group('legacy: follow is unsupported when FL < 219', () { + final testCases = [ + (false, UserTopicVisibilityPolicy.muted, [unmute]), + (false, UserTopicVisibilityPolicy.none, [mute]), + (false, UserTopicVisibilityPolicy.unmuted, [mute]), + (false, UserTopicVisibilityPolicy.followed, [mute]), + + (true, UserTopicVisibilityPolicy.muted, [unmute]), + (true, UserTopicVisibilityPolicy.none, [unmute]), + (true, UserTopicVisibilityPolicy.unmuted, [mute]), + (true, UserTopicVisibilityPolicy.followed, [mute]), + + (null, UserTopicVisibilityPolicy.none, []), + ]; + + for (final (isChannelMuted, visibilityPolicy, buttons) in testCases) { + final description = 'isChannelMuted: ${isChannelMuted ?? "(not subscribed)"}, $visibilityPolicy'; + testWidgets(description, (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: isChannelMuted, + visibilityPolicy: visibilityPolicy, + zulipFeatureLevel: 218); + checkButtons(buttons); + }); + } + }); + + group('legacy: unmute is unsupported when FL < 170', () { + final testCases = [ + (false, UserTopicVisibilityPolicy.muted, [unmute]), + (false, UserTopicVisibilityPolicy.none, [mute]), + (false, UserTopicVisibilityPolicy.unmuted, [mute]), + (false, UserTopicVisibilityPolicy.followed, [mute]), + + (true, UserTopicVisibilityPolicy.muted, []), + (true, UserTopicVisibilityPolicy.none, []), + (true, UserTopicVisibilityPolicy.unmuted, []), + (true, UserTopicVisibilityPolicy.followed, []), + + (null, UserTopicVisibilityPolicy.none, []), + ]; + + for (final (isChannelMuted, visibilityPolicy, buttons) in testCases) { + final description = 'isChannelMuted: ${isChannelMuted ?? "(not subscribed)"}, $visibilityPolicy'; + testWidgets(description, (tester) async { + await setupToTopicActionSheet(tester, + isChannelMuted: isChannelMuted, + visibilityPolicy: visibilityPolicy, + zulipFeatureLevel: 169); + checkButtons(buttons); + }); + } + }); + }); + group('AddThumbsUpButton', () { Future tapButton(WidgetTester tester) async { await tester.ensureVisible(find.byIcon(ZulipIcons.smile, skipOffstage: false));