Skip to content

Commit cd4ae57

Browse files
PIG208PIG208
committed
narrow: Support MentionsNarrow.
Because of how narrow interacts with the entire app, there are test updates all over the place. The way I checked for the completeness of this change is by looking at places where CombinedFeedNarrow is referenced. Signed-off-by: Zixuan James Li <[email protected]>
1 parent abe5029 commit cd4ae57

15 files changed

+198
-7
lines changed

‎assets/l10n/app_en.arb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,10 @@
344344
"@loginErrorMissingUsername": {
345345
"description": "Error message when an empty username was provided."
346346
},
347+
"mentionsPageTitle": "Mentions",
348+
"@mentionsPageTitle": {
349+
"description": "Title for the page of @-mentions."
350+
},
347351
"topicValidationErrorTooLong": "Topic length shouldn't be greater than 60 characters.",
348352
"@topicValidationErrorTooLong": {
349353
"description": "Topic validation error when topic is too long."

‎lib/model/autocomplete.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ class MentionAutocompleteView extends ChangeNotifier {
188188
required Narrow narrow,
189189
}) {
190190
assert(narrow is! CombinedFeedNarrow);
191+
assert(narrow is! MentionsNarrow);
191192
return store.users.values.toList()
192193
..sort((userA, userB) => compareByDms(userA, userB, store: store));
193194
}

‎lib/model/message_list.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
370370

371371
case TopicNarrow():
372372
case DmNarrow():
373+
case MentionsNarrow():
373374
return true;
374375
}
375376
}
@@ -385,6 +386,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
385386

386387
case TopicNarrow():
387388
case DmNarrow():
389+
case MentionsNarrow():
388390
return true;
389391
}
390392
}

‎lib/model/narrow.dart

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,4 +286,27 @@ class DmNarrow extends Narrow implements SendableNarrow {
286286
int get hashCode => Object.hash('DmNarrow', _key);
287287
}
288288

289-
// TODO other narrow types: starred, mentioned; searches; arbitrary
289+
class MentionsNarrow extends Narrow {
290+
const MentionsNarrow();
291+
292+
@override
293+
bool containsMessage(Message message) {
294+
return message.flags.any((flag) =>
295+
flag == MessageFlag.mentioned || flag == MessageFlag.wildcardMentioned);
296+
}
297+
298+
@override
299+
ApiNarrow apiEncode() => [ApiNarrowIsMentioned()];
300+
301+
@override
302+
bool operator ==(Object other) {
303+
if (other is! MentionsNarrow) return false;
304+
// Conceptually there's only one value of this type.
305+
return true;
306+
}
307+
308+
@override
309+
int get hashCode => 'MentionedNarrow'.hashCode;
310+
}
311+
312+
// TODO other narrow types: starred; searches; arbitrary

‎lib/model/unreads.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ class Unreads extends ChangeNotifier {
193193

194194
int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0;
195195

196+
int countInMentionsNarrow() => mentions.length;
197+
196198
int countInNarrow(Narrow narrow) {
197199
switch (narrow) {
198200
case CombinedFeedNarrow():
@@ -203,6 +205,8 @@ class Unreads extends ChangeNotifier {
203205
return countInTopicNarrow(narrow.streamId, narrow.topic);
204206
case DmNarrow():
205207
return countInDmNarrow(narrow);
208+
case MentionsNarrow():
209+
return countInMentionsNarrow();
206210
}
207211
}
208212

‎lib/widgets/compose_box.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ class ComposeBox extends StatelessWidget {
967967
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
968968
} else if (narrow is DmNarrow) {
969969
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
970-
} else if (narrow is CombinedFeedNarrow) {
970+
} else if (narrow is CombinedFeedNarrow || narrow is MentionsNarrow) {
971971
return const SizedBox.shrink();
972972
} else {
973973
throw Exception("impossible narrow"); // TODO(dart-3): show this statically

‎lib/widgets/message_list.dart

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ class _MessageListPageState extends State<MessageListPage> {
6363
bool removeAppBarBottomBorder = false;
6464
switch(widget.narrow) {
6565
case CombinedFeedNarrow():
66+
case MentionsNarrow():
6667
appBarBackgroundColor = null; // i.e., inherit
6768

6869
case StreamNarrow(:final streamId):
@@ -109,7 +110,8 @@ class _MessageListPageState extends State<MessageListPage> {
109110
// if those details get complicated, refactor to avoid copying.
110111
// TODO(#311) If we have a bottom nav, it will pad the bottom
111112
// inset, and this should always be true.
112-
removeBottom: widget.narrow is! CombinedFeedNarrow,
113+
removeBottom: widget.narrow is! CombinedFeedNarrow
114+
&& widget.narrow is! MentionsNarrow,
113115

114116
child: Expanded(
115117
child: MessageList(narrow: widget.narrow))),
@@ -170,6 +172,9 @@ class MessageListAppBarTitle extends StatelessWidget {
170172
final names = otherRecipientIds.map((id) => store.users[id]?.fullName ?? '(unknown user)');
171173
return Text("DMs with ${names.join(", ")}"); // TODO show avatars
172174
}
175+
176+
case MentionsNarrow():
177+
return Text(zulipLocalizations.mentionsPageTitle);
173178
}
174179
}
175180
}
@@ -340,7 +345,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
340345
return _buildItem(data, i);
341346
}));
342347

343-
if (widget.narrow is CombinedFeedNarrow) {
348+
if (widget.narrow is CombinedFeedNarrow || widget.narrow is MentionsNarrow) {
344349
// TODO(#311) If we have a bottom nav, it will pad the bottom
345350
// inset, and this shouldn't be necessary
346351
sliver = SliverSafeArea(sliver: sliver);
@@ -521,7 +526,7 @@ class RecipientHeader extends StatelessWidget {
521526
final message = this.message;
522527
return switch (message) {
523528
StreamMessage() => StreamMessageRecipientHeader(message: message,
524-
showStream: narrow is CombinedFeedNarrow),
529+
showStream: narrow is CombinedFeedNarrow || narrow is MentionsNarrow),
525530
DmMessage() => DmRecipientHeader(message: message),
526531
};
527532
}
@@ -1111,5 +1116,12 @@ Future<void> _legacyMarkNarrowAsRead(BuildContext context, Narrow narrow) async
11111116
messages: unreadDms,
11121117
op: UpdateMessageFlagsOp.add,
11131118
flag: MessageFlag.read);
1119+
case MentionsNarrow():
1120+
final unreadMentions = store.unreads.mentions.toList();
1121+
if (unreadMentions.isEmpty) return;
1122+
await updateMessageFlags(connection,
1123+
messages: unreadMentions,
1124+
op: UpdateMessageFlagsOp.add,
1125+
flag: MessageFlag.read);
11141126
}
11151127
}

‎test/api/route/messages_test.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,9 @@ void main() {
188188
{'operator': 'stream', 'operand': 12},
189189
{'operator': 'topic', 'operand': 'stuff'},
190190
]));
191+
checkNarrow(const MentionsNarrow().apiEncode(), jsonEncode([
192+
{'operator': 'is', 'operand': 'mentioned'},
193+
]));
191194

192195
checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([
193196
{'operator': 'dm', 'operand': [123, 234]},

‎test/model/autocomplete_test.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,15 @@ void main() {
472472
expected: [0, 1, 2, 3, 4])
473473
).throws();
474474
});
475+
476+
test('MentionsNarrow', () async {
477+
// As we do not expect a compose box in [MentionsNarrow], it should
478+
// not proceed to show any results.
479+
await check(checkResultsIn(
480+
const MentionsNarrow(),
481+
expected: [0, 1, 2, 3, 4])
482+
).throws();
483+
});
475484
});
476485
});
477486
}

‎test/model/compose_test.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,16 @@ hello
299299
'#narrow/pm-with/1,2-pm/near/12345');
300300
});
301301

302+
test('MentionsNarrow', () {
303+
final store = eg.store();
304+
check(narrowLink(store, const MentionsNarrow()))
305+
.equals(store.realmUrl.resolve('#narrow/is/mentioned'));
306+
check(narrowLink(store, const MentionsNarrow(), nearMessageId: 1))
307+
.equals(store.realmUrl.resolve('#narrow/is/mentioned/near/1'));
308+
});
309+
302310
// TODO other Narrow subclasses as we add them:
303-
// starred, mentioned; searches; arbitrary
311+
// starred; searches; arbitrary
304312
});
305313

306314
group('mention', () {

‎test/model/message_list_test.dart

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,53 @@ void main() {
693693
checkNotifiedOnce();
694694
check(model.messages.map((m) => m.id)).deepEquals(expected..add(301));
695695
});
696+
697+
test('in MentionsNarrow', () async {
698+
final stream1 = eg.stream(streamId: 1, name: 'stream 1');
699+
final stream2 = eg.stream(streamId: 2, name: 'stream 2');
700+
await prepare(narrow: const MentionsNarrow());
701+
await store.addStreams([stream1, stream2]);
702+
await store.addSubscription(eg.subscription(stream1));
703+
await store.addUserTopic(stream1, 'B', UserTopicVisibilityPolicy.muted);
704+
await store.addSubscription(eg.subscription(stream2, isMuted: true));
705+
await store.addUserTopic(stream2, 'C', UserTopicVisibilityPolicy.unmuted);
706+
707+
const topics = ["A", "B", "C", "D"];
708+
List<Message> genMessages(int startingId) => [
709+
...List.generate(4, (i) =>
710+
eg.streamMessage(
711+
id: startingId + i,
712+
stream: (i < 2) ? stream1 : stream2,
713+
topic: topics[i],
714+
flags: [MessageFlag.wildcardMentioned])),
715+
eg.dmMessage(
716+
id: startingId + 4,
717+
from: eg.otherUser, to: [eg.selfUser],
718+
flags: [MessageFlag.mentioned]),
719+
];
720+
721+
// Check filtering on fetchInitial…
722+
await prepareMessages(foundOldest: false, messages: genMessages(201));
723+
final expected = <int>[];
724+
check(model.messages.map((m) => m.id))
725+
.deepEquals(expected..addAll([201, 202, 203, 204, 205]));
726+
727+
// … and on fetchOlder…
728+
connection.prepare(json: olderResult(
729+
anchor: 201, foundOldest: true, messages: genMessages(101)).toJson());
730+
await model.fetchOlder();
731+
checkNotified(count: 2);
732+
check(model.messages.map((m) => m.id))
733+
.deepEquals(expected..insertAll(0, [101, 102, 103, 104, 105]));
734+
735+
// … and on MessageEvent.
736+
final messages = genMessages(301);
737+
for (var i = 0; i < 5; i += 1) {
738+
await store.handleEvent(MessageEvent(id: 0, message: messages[i]));
739+
checkNotifiedOnce();
740+
check(model.messages.map((m) => m.id)).deepEquals(expected..add(301 + i));
741+
}
742+
});
696743
});
697744

698745
test('recipient headers are maintained consistently', () async {
@@ -938,6 +985,7 @@ void checkInvariants(MessageListView model) {
938985
.isTrue();
939986
case TopicNarrow():
940987
case DmNarrow():
988+
case MentionsNarrow():
941989
}
942990
}
943991

‎test/model/narrow_test.dart

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,4 +149,17 @@ void main() {
149149
check(narrow123.containsMessage(dm(user3, [user1, user2]))).isTrue();
150150
});
151151
});
152+
153+
group('MentionsNarrow', () {
154+
test('containsMessage', () {
155+
const narrow = MentionsNarrow();
156+
157+
check(narrow.containsMessage(eg.streamMessage(
158+
flags: []))).isFalse();
159+
check(narrow.containsMessage(eg.streamMessage(
160+
flags:[MessageFlag.mentioned]))).isTrue();
161+
check(narrow.containsMessage(eg.streamMessage(
162+
flags: [MessageFlag.wildcardMentioned]))).isTrue();
163+
});
164+
});
152165
}

‎test/model/unreads_test.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,18 @@ void main() {
215215
eg.otherUser.userId, selfUserId: eg.selfUser.userId);
216216
check(model.countInDmNarrow(narrow)).equals(5);
217217
});
218+
219+
test('countInMentionsNarrow', () async {
220+
final stream = eg.stream();
221+
prepare();
222+
await streamStore.addStream(stream);
223+
fillWithMessages([
224+
eg.streamMessage(stream: stream, flags: []),
225+
eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]),
226+
eg.streamMessage(stream: stream, flags: [MessageFlag.wildcardMentioned]),
227+
]);
228+
check(model.countInMentionsNarrow()).equals(2);
229+
});
218230
});
219231

220232
group('handleMessageEvent', () {

‎test/widgets/action_sheet_test.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,12 @@ void main() {
386386
await setupToMessageActionSheet(tester, message: message, narrow: const CombinedFeedNarrow());
387387
check(findQuoteAndReplyButton(tester)).isNull();
388388
});
389+
390+
testWidgets('not offered in MentionsNarrow (composing to reply is not yet supported)', (WidgetTester tester) async {
391+
final message = eg.streamMessage();
392+
await setupToMessageActionSheet(tester, message: message, narrow: const MentionsNarrow());
393+
check(findQuoteAndReplyButton(tester)).isNull();
394+
});
389395
});
390396

391397
group('CopyMessageTextButton', () {

‎test/widgets/message_list_test.dart

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,19 @@ void main() {
101101
final padding = MediaQuery.of(element).padding;
102102
check(padding).equals(EdgeInsets.zero);
103103
});
104+
105+
testWidgets('content in MentionsNarrow not asked to consume insets (including bottom)', (tester) async {
106+
const fakePadding = FakeViewPadding(left: 10, top: 10, right: 10, bottom: 10);
107+
tester.view.viewInsets = fakePadding;
108+
tester.view.padding = fakePadding;
109+
110+
await setupMessageListPage(tester, narrow: const MentionsNarrow(),
111+
messages: [eg.streamMessage(content: ContentExample.codeBlockPlain.html, flags: [MessageFlag.mentioned])]);
112+
113+
final element = tester.element(find.byType(CodeBlock));
114+
final padding = MediaQuery.of(element).padding;
115+
check(padding).equals(EdgeInsets.zero);
116+
});
104117
});
105118

106119
group('fetch older messages on scroll', () {
@@ -256,7 +269,8 @@ void main() {
256269
group('recipient headers', () {
257270
group('StreamMessageRecipientHeader', () {
258271
final stream = eg.stream(name: 'stream name');
259-
final message = eg.streamMessage(stream: stream, topic: 'topic name');
272+
final message = eg.streamMessage(
273+
stream: stream, topic: 'topic name');
260274

261275
FinderResult<Element> findInMessageList(String text) {
262276
// Stream name shows up in [AppBar] so need to avoid matching that
@@ -274,6 +288,15 @@ void main() {
274288
check(findInMessageList('topic name')).length.equals(1);
275289
});
276290

291+
testWidgets('show stream name in MentionsNarrow', (tester) async {
292+
await setupMessageListPage(tester,
293+
narrow: const MentionsNarrow(),
294+
messages: [message], subscriptions: [eg.subscription(stream)]);
295+
await tester.pump();
296+
check(findInMessageList('stream name')).length.equals(1);
297+
check(findInMessageList('topic name')).length.equals(1);
298+
});
299+
277300
testWidgets('do not show stream name in StreamNarrow', (tester) async {
278301
await setupMessageListPage(tester,
279302
narrow: StreamNarrow(stream.streamId),
@@ -927,6 +950,29 @@ void main() {
927950
check(store.unreads).oldUnreadsMissing.isTrue();
928951
});
929952

953+
testWidgets('MentionsNarrow on legacy server', (WidgetTester tester) async {
954+
await setupMessageListPage(tester,
955+
narrow: const MentionsNarrow(),
956+
messages: [eg.streamMessage(flags: [MessageFlag.mentioned])],
957+
unreadMsgs: eg.unreadMsgs(mentions: [message.id]),
958+
);
959+
check(isMarkAsReadButtonVisible(tester)).isTrue();
960+
961+
connection.zulipFeatureLevel = 154;
962+
connection.prepare(json:
963+
UpdateMessageFlagsResult(messages: [message.id]).toJson());
964+
await tester.tap(find.byType(MarkAsReadWidget));
965+
check(connection.lastRequest).isA<http.Request>()
966+
..method.equals('POST')
967+
..url.path.equals('/api/v1/messages/flags')
968+
..bodyFields.deepEquals({
969+
'messages': jsonEncode([message.id]),
970+
'op': 'add',
971+
'flag': 'read',
972+
});
973+
await tester.pumpAndSettle();
974+
});
975+
930976
testWidgets('StreamNarrow on legacy server', (WidgetTester tester) async {
931977
final narrow = StreamNarrow(message.streamId);
932978
await setupMessageListPage(tester,

0 commit comments

Comments
 (0)