diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 15132a2b10..e1beadac31 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -472,6 +472,10 @@ "@combinedFeedPageTitle": { "description": "Title for the page of combined feed." }, + "mentionsPageTitle": "Mentions", + "@mentionsPageTitle": { + "description": "Title for the page of @-mentions." + }, "notifGroupDmConversationLabel": "{senderFullName} to you and {numOthers, plural, =1{1 other} other{{numOthers} others}}", "@notifGroupDmConversationLabel": { "description": "Label for a group DM conversation notification.", diff --git a/lib/api/model/narrow.dart b/lib/api/model/narrow.dart index 7383d382ba..9851389bc1 100644 --- a/lib/api/model/narrow.dart +++ b/lib/api/model/narrow.dart @@ -115,6 +115,13 @@ class ApiNarrowPmWith extends ApiNarrowDm { } // TODO: generalize into ApiNarrowIs +class ApiNarrowIsMentioned extends ApiNarrowElement { + @override String get operator => 'is'; + @override String get operand => 'mentioned'; + + ApiNarrowIsMentioned({super.negated}); +} + class ApiNarrowIsUnread extends ApiNarrowElement { @override String get operator => 'is'; @override String get operand => 'unread'; diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 867d426b81..3054fd5348 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -222,6 +222,7 @@ class MentionAutocompleteView extends ChangeNotifier { case DmNarrow(): break; case CombinedFeedNarrow(): + case MentionsNarrow(): assert(false, 'No compose box, thus no autocomplete is available in ${narrow.runtimeType}.'); } return (userA, userB) => _compareByRelevance(userA, userB, diff --git a/lib/model/internal_link.dart b/lib/model/internal_link.dart index 422a0d8182..e933a0e6f8 100644 --- a/lib/model/internal_link.dart +++ b/lib/model/internal_link.dart @@ -85,6 +85,8 @@ Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { fragment.write('${element.operand.join(',')}-$suffix'); case ApiNarrowDm(): assert(false, 'ApiNarrowDm should have been resolved'); + case ApiNarrowIsMentioned(): + fragment.write(element.operand.toString()); case ApiNarrowIsUnread(): fragment.write(element.operand.toString()); case ApiNarrowMessageId(): @@ -152,6 +154,7 @@ Narrow? _interpretNarrowSegments(List segments, PerAccountStore store) { ApiNarrowStream? streamElement; ApiNarrowTopic? topicElement; ApiNarrowDm? dmElement; + ApiNarrowIsMentioned? isMentionedElement; for (var i = 0; i < segments.length; i += 2) { final (operator, negated) = _parseOperator(segments[i]); @@ -179,6 +182,10 @@ Narrow? _interpretNarrowSegments(List segments, PerAccountStore store) { if (dmIds == null) return null; dmElement = ApiNarrowDm(dmIds, negated: negated); + case _NarrowOperator.is_: + if (isMentionedElement != null) return null; + if (operand == 'mentioned') isMentionedElement = ApiNarrowIsMentioned(); + case _NarrowOperator.near: // TODO(#82): support for near case _NarrowOperator.with_: // TODO(#683): support for with continue; @@ -188,7 +195,10 @@ Narrow? _interpretNarrowSegments(List segments, PerAccountStore store) { } } - if (dmElement != null) { + if (isMentionedElement != null) { + if (streamElement != null || topicElement != null || dmElement != null) return null; + return const MentionsNarrow(); + } else if (dmElement != null) { if (streamElement != null || topicElement != null) return null; return DmNarrow.withUsers(dmElement.operand, selfUserId: store.selfUserId); } else if (streamElement != null) { @@ -210,6 +220,9 @@ enum _NarrowOperator { // cannot use `with` as it is a reserved keyword in Dart @JsonValue('with') with_, + // cannot use `is` as it is a reserved keyword in Dart + @JsonValue('is') + is_, pmWith, stream, channel, diff --git a/lib/model/internal_link.g.dart b/lib/model/internal_link.g.dart index 7978a3d939..5271c4a2ec 100644 --- a/lib/model/internal_link.g.dart +++ b/lib/model/internal_link.g.dart @@ -12,6 +12,7 @@ const _$_NarrowOperatorEnumMap = { _NarrowOperator.dm: 'dm', _NarrowOperator.near: 'near', _NarrowOperator.with_: 'with', + _NarrowOperator.is_: 'is', _NarrowOperator.pmWith: 'pm-with', _NarrowOperator.stream: 'stream', _NarrowOperator.channel: 'channel', diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 83fd512391..fa9dc026c1 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -370,6 +370,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { case TopicNarrow(): case DmNarrow(): + case MentionsNarrow(): return true; } } @@ -385,6 +386,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { case TopicNarrow(): case DmNarrow(): + case MentionsNarrow(): return true; } } diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index a23caa40b9..c92034cb62 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -286,4 +286,38 @@ class DmNarrow extends Narrow implements SendableNarrow { int get hashCode => Object.hash('DmNarrow', _key); } -// TODO other narrow types: starred, mentioned; searches; arbitrary +class MentionsNarrow extends Narrow { + const MentionsNarrow(); + + @override + bool containsMessage(Message message) { + return message.flags.any((flag) { + switch (flag) { + case MessageFlag.mentioned: + case MessageFlag.wildcardMentioned: + return true; + + case MessageFlag.read: + case MessageFlag.starred: + case MessageFlag.collapsed: + case MessageFlag.hasAlertWord: + case MessageFlag.historical: + case MessageFlag.unknown: + return false; + } + }); + } + + @override + ApiNarrow apiEncode() => [ApiNarrowIsMentioned()]; + + @override + bool operator ==(Object other) { + if (other is! MentionsNarrow) return false; + // Conceptually there's only one value of this type. + return true; + } + + @override + int get hashCode => 'MentionsNarrow'.hashCode; +} diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 00a0418af4..1749373f74 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -193,6 +193,8 @@ class Unreads extends ChangeNotifier { int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0; + int countInMentionsNarrow() => mentions.length; + int countInNarrow(Narrow narrow) { switch (narrow) { case CombinedFeedNarrow(): @@ -203,6 +205,8 @@ class Unreads extends ChangeNotifier { return countInTopicNarrow(narrow.streamId, narrow.topic); case DmNarrow(): return countInDmNarrow(narrow); + case MentionsNarrow(): + return countInMentionsNarrow(); } } diff --git a/lib/widgets/actions.dart b/lib/widgets/actions.dart index 02b3e5f566..553e977598 100644 --- a/lib/widgets/actions.dart +++ b/lib/widgets/actions.dart @@ -134,5 +134,12 @@ Future _legacyMarkNarrowAsRead(BuildContext context, Narrow narrow) async messages: unreadDms, op: UpdateMessageFlagsOp.add, flag: MessageFlag.read); + case MentionsNarrow(): + final unreadMentions = store.unreads.mentions.toList(); + if (unreadMentions.isEmpty) return; + await updateMessageFlags(connection, + messages: unreadMentions, + op: UpdateMessageFlagsOp.add, + flag: MessageFlag.read); } } diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 63cf045628..b201281ba3 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -276,6 +276,12 @@ class HomePage extends StatelessWidget { narrow: const CombinedFeedNarrow())), child: Text(zulipLocalizations.combinedFeedPageTitle)), const SizedBox(height: 16), + ElevatedButton( + onPressed: () => Navigator.push(context, + MessageListPage.buildRoute(context: context, + narrow: const MentionsNarrow())), + child: Text(zulipLocalizations.mentionsPageTitle)), + const SizedBox(height: 16), ElevatedButton( onPressed: () => Navigator.push(context, InboxPage.buildRoute(context: context)), diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index df5dc71d08..557edba588 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1023,6 +1023,19 @@ class ComposeBox extends StatelessWidget { final GlobalKey? controllerKey; final Narrow narrow; + static bool hasComposeBox(Narrow narrow) { + switch (narrow) { + case ChannelNarrow(): + case TopicNarrow(): + case DmNarrow(): + return true; + + case CombinedFeedNarrow(): + case MentionsNarrow(): + return false; + } + } + @override Widget build(BuildContext context) { final narrow = this.narrow; @@ -1034,6 +1047,7 @@ class ComposeBox extends StatelessWidget { case DmNarrow(): return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow); case CombinedFeedNarrow(): + case MentionsNarrow(): return const SizedBox.shrink(); } } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index cb9ff0a038..44e2b84650 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -235,6 +235,7 @@ class _MessageListPageState extends State implements MessageLis bool removeAppBarBottomBorder = false; switch(widget.narrow) { case CombinedFeedNarrow(): + case MentionsNarrow(): appBarBackgroundColor = null; // i.e., inherit case ChannelNarrow(:final streamId): @@ -277,11 +278,9 @@ class _MessageListPageState extends State implements MessageLis context: context, // The compose box, when present, pads the bottom inset. - // TODO this copies the details of when the compose box is shown; - // if those details get complicated, refactor to avoid copying. // TODO(#311) If we have a bottom nav, it will pad the bottom // inset, and this should always be true. - removeBottom: widget.narrow is! CombinedFeedNarrow, + removeBottom: ComposeBox.hasComposeBox(widget.narrow), child: Expanded( child: MessageList(narrow: widget.narrow))), @@ -322,6 +321,9 @@ class MessageListAppBarTitle extends StatelessWidget { case CombinedFeedNarrow(): return Text(zulipLocalizations.combinedFeedPageTitle); + case MentionsNarrow(): + return Text(zulipLocalizations.mentionsPageTitle); + case ChannelNarrow(:var streamId): final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; @@ -514,7 +516,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat return _buildItem(data, i); })); - if (widget.narrow is CombinedFeedNarrow) { + if (!ComposeBox.hasComposeBox(widget.narrow)) { // TODO(#311) If we have a bottom nav, it will pad the bottom // inset, and this shouldn't be necessary sliver = SliverSafeArea(sliver: sliver); @@ -807,12 +809,25 @@ class RecipientHeader extends StatelessWidget { final Message message; final Narrow narrow; + static bool _containsDifferentChannels(Narrow narrow) { + switch (narrow) { + case CombinedFeedNarrow(): + case MentionsNarrow(): + return true; + + case ChannelNarrow(): + case TopicNarrow(): + case DmNarrow(): + return false; + } + } + @override Widget build(BuildContext context) { final message = this.message; return switch (message) { StreamMessage() => StreamMessageRecipientHeader(message: message, - showStream: narrow is CombinedFeedNarrow), + showStream: _containsDifferentChannels(narrow)), DmMessage() => DmRecipientHeader(message: message), }; } diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart index 32efc676cd..0af7bb9b30 100644 --- a/test/api/route/messages_test.dart +++ b/test/api/route/messages_test.dart @@ -188,6 +188,9 @@ void main() { {'operator': 'stream', 'operand': 12}, {'operator': 'topic', 'operand': 'stuff'}, ])); + checkNarrow(const MentionsNarrow().apiEncode(), jsonEncode([ + {'operator': 'is', 'operand': 'mentioned'}, + ])); checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([ {'operator': 'dm', 'operand': [123, 234]}, diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index f74006fed2..54ed729a64 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -571,6 +571,13 @@ void main() { check(() => MentionAutocompleteView.init(store: store, narrow: narrow)) .throws(); }); + + test('MentionsNarrow gives error', () async { + await prepare(users: [eg.user(), eg.user()], messages: []); + const narrow = MentionsNarrow(); + check(() => MentionAutocompleteView.init(store: store, narrow: narrow)) + .throws(); + }); }); test('final results end-to-end', () async { diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index 96b2491102..b5be162171 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -231,6 +231,14 @@ hello .equals(store.realmUrl.resolve('#narrow/near/1')); }); + test('MentionsNarrow', () { + final store = eg.store(); + check(narrowLink(store, const MentionsNarrow())) + .equals(store.realmUrl.resolve('#narrow/is/mentioned')); + check(narrowLink(store, const MentionsNarrow(), nearMessageId: 1)) + .equals(store.realmUrl.resolve('#narrow/is/mentioned/near/1')); + }); + test('ChannelNarrow / TopicNarrow', () { void checkNarrow(String expectedFragment, { required int streamId, @@ -298,9 +306,6 @@ hello '#narrow/dm/1,2-dm/near/12345', '#narrow/pm-with/1,2-pm/near/12345'); }); - - // TODO other Narrow subclasses as we add them: - // starred, mentioned; searches; arbitrary }); group('mention', () { diff --git a/test/model/internal_link_test.dart b/test/model/internal_link_test.dart index 117e824169..9636e6055a 100644 --- a/test/model/internal_link_test.dart +++ b/test/model/internal_link_test.dart @@ -221,6 +221,20 @@ void main() { testExpectedNarrows(testCases, streams: streams); }); + group('"/#narrow/is/mentioned returns expected MentionsNarrow', () { + final testCases = [ + ('/#narrow/is/mentioned', const MentionsNarrow()), + ('/#narrow/is/mentioned/near/1', const MentionsNarrow()), + ('/#narrow/is/mentioned/with/2', const MentionsNarrow()), + ('/#narrow/channel/7-test-here/is/mentioned', null), + ('/#narrow/channel/check/topic/test/is/mentioned', null), + ('/#narrow/topic/test/is/mentioned', null), + ('/#narrow/dm/17327-Chris-Bobbe-(Test-Account)/is/mentioned', null), + ('/#narrow/-is/mentioned', null), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + group('unexpected link shapes are rejected', () { final testCases = [ ('/#narrow/stream/name/topic/', null), // missing operand diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 441e8b7e46..545074a97c 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -734,6 +734,46 @@ void main() { checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); }); + + test('in MentionsNarrow', () async { + final stream = eg.stream(streamId: 1, name: 'muted stream'); + const mutedTopic = 'muted'; + await prepare(narrow: const MentionsNarrow()); + await store.addStream(stream); + await store.addUserTopic(stream, mutedTopic, UserTopicVisibilityPolicy.muted); + await store.addSubscription(eg.subscription(stream, isMuted: true)); + + List getMessages(int startingId) => [ + eg.streamMessage(id: startingId, + stream: stream, topic: mutedTopic, flags: [MessageFlag.wildcardMentioned]), + eg.streamMessage(id: startingId + 1, + stream: stream, topic: mutedTopic, flags: [MessageFlag.mentioned]), + eg.dmMessage(id: startingId + 2, + from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.mentioned]), + ]; + + // Check filtering on fetchInitial… + await prepareMessages(foundOldest: false, messages: getMessages(201)); + final expected = []; + check(model.messages.map((m) => m.id)) + .deepEquals(expected..addAll([201, 202, 203])); + + // … and on fetchOlder… + connection.prepare(json: olderResult( + anchor: 201, foundOldest: true, messages: getMessages(101)).toJson()); + await model.fetchOlder(); + checkNotified(count: 2); + check(model.messages.map((m) => m.id)) + .deepEquals(expected..insertAll(0, [101, 102, 103])); + + // … and on MessageEvent. + final messages = getMessages(301); + for (var i = 0; i < 3; i += 1) { + await store.handleEvent(MessageEvent(id: 0, message: messages[i])); + checkNotifiedOnce(); + check(model.messages.map((m) => m.id)).deepEquals(expected..add(301 + i)); + } + }); }); test('recipient headers are maintained consistently', () async { @@ -979,6 +1019,7 @@ void checkInvariants(MessageListView model) { .isTrue(); case TopicNarrow(): case DmNarrow(): + case MentionsNarrow(): } } diff --git a/test/model/narrow_test.dart b/test/model/narrow_test.dart index af75b3e6c2..2f2ff229c3 100644 --- a/test/model/narrow_test.dart +++ b/test/model/narrow_test.dart @@ -149,4 +149,17 @@ void main() { check(narrow123.containsMessage(dm(user3, [user1, user2]))).isTrue(); }); }); + + group('MentionsNarrow', () { + test('containsMessage', () { + const narrow = MentionsNarrow(); + + check(narrow.containsMessage( + eg.streamMessage(flags: []))).isFalse(); + check(narrow.containsMessage( + eg.streamMessage(flags:[MessageFlag.mentioned]))).isTrue(); + check(narrow.containsMessage( + eg.streamMessage(flags: [MessageFlag.wildcardMentioned]))).isTrue(); + }); + }); } diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index b5f50405b5..b35f5c770e 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -215,6 +215,18 @@ void main() { eg.otherUser.userId, selfUserId: eg.selfUser.userId); check(model.countInDmNarrow(narrow)).equals(5); }); + + test('countInMentionsNarrow', () async { + final stream = eg.stream(); + prepare(); + await channelStore.addStream(stream); + fillWithMessages([ + eg.streamMessage(stream: stream, flags: []), + eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: stream, flags: [MessageFlag.wildcardMentioned]), + ]); + check(model.countInMentionsNarrow()).equals(2); + }); }); group('handleMessageEvent', () { diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 423ae8dbe0..3b61192f0a 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -377,6 +377,12 @@ void main() { await setupToMessageActionSheet(tester, message: message, narrow: const CombinedFeedNarrow()); check(findQuoteAndReplyButton(tester)).isNull(); }); + + testWidgets('not offered in MentionsNarrow (composing to reply is not yet supported)', (WidgetTester tester) async { + final message = eg.streamMessage(); + await setupToMessageActionSheet(tester, message: message, narrow: const MentionsNarrow()); + check(findQuoteAndReplyButton(tester)).isNull(); + }); }); group('CopyMessageTextButton', () { diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index b315b80037..f1af74dde6 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/model/initial_snapshot.dart'; +import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/narrow.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/localizations.dart'; @@ -258,6 +259,26 @@ void main() { }); }); + testWidgets('MentionsNarrow on legacy server', (WidgetTester tester) async { + const narrow = MentionsNarrow(); + final message = eg.streamMessage(flags: [MessageFlag.mentioned]); + final unreadMsgs = eg.unreadMsgs(mentions: [message.id]); + await prepare(tester, unreadMsgs: unreadMsgs); + connection.zulipFeatureLevel = 154; + connection.prepare(json: + UpdateMessageFlagsResult(messages: [message.id]).toJson()); + markNarrowAsRead(context, narrow, true); // TODO move legacy-server check inside markNarrowAsRead + 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': 'read', + }); + }); + testWidgets('catch-all api errors', (WidgetTester tester) async { final zulipLocalizations = GlobalLocalizations.zulipLocalizations; const narrow = CombinedFeedNarrow(); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 4bc3c296da..3b13c158e4 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -135,6 +135,19 @@ void main() { final padding = MediaQuery.of(element).padding; check(padding).equals(EdgeInsets.zero); }); + + testWidgets('content in MentionsNarrow not asked to consume insets (including bottom)', (tester) async { + const fakePadding = FakeViewPadding(left: 10, top: 10, right: 10, bottom: 10); + tester.view.viewInsets = fakePadding; + tester.view.padding = fakePadding; + + await setupMessageListPage(tester, narrow: const MentionsNarrow(), + messages: [eg.streamMessage(content: ContentExample.codeBlockPlain.html, flags: [MessageFlag.mentioned])]); + + final element = tester.element(find.byType(CodeBlock)); + final padding = MediaQuery.of(element).padding; + check(padding).equals(EdgeInsets.zero); + }); }); testWidgets('smoke test for light/dark/lerped', (tester) async { @@ -628,7 +641,16 @@ void main() { check(findInMessageList('topic name')).length.equals(1); }); - testWidgets('do not show stream name in ChannelNarrow', (tester) async { + testWidgets('show channel name in MentionsNarrow', (tester) async { + await setupMessageListPage(tester, + narrow: const MentionsNarrow(), + messages: [message], subscriptions: [eg.subscription(stream)]); + await tester.pump(); + check(findInMessageList('stream name')).length.equals(1); + check(findInMessageList('topic name')).length.equals(1); + }); + + testWidgets('do not show channel name in ChannelNarrow', (tester) async { await setupMessageListPage(tester, narrow: ChannelNarrow(stream.streamId), messages: [message], streams: [stream]);