Skip to content

Commit b713924

Browse files
committed
msglist: In single-conversation view, make recipient headers not tappable.
Fixes: zulip#1171
1 parent 3ff7096 commit b713924

File tree

2 files changed

+145
-9
lines changed

2 files changed

+145
-9
lines changed

lib/widgets/message_list.dart

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -899,8 +899,9 @@ class RecipientHeader extends StatelessWidget {
899899
final message = this.message;
900900
return switch (message) {
901901
StreamMessage() => StreamMessageRecipientHeader(message: message,
902-
showStream: _containsDifferentChannels(narrow)),
903-
DmMessage() => DmRecipientHeader(message: message),
902+
showStream: _containsDifferentChannels(narrow),
903+
inTopicNarrow: narrow is TopicNarrow),
904+
DmMessage() => DmRecipientHeader(message: message, inDmNarrow: narrow is DmNarrow),
904905
};
905906
}
906907
}
@@ -1015,10 +1016,12 @@ class StreamMessageRecipientHeader extends StatelessWidget {
10151016
super.key,
10161017
required this.message,
10171018
required this.showStream,
1019+
required this.inTopicNarrow,
10181020
});
10191021

10201022
final StreamMessage message;
10211023
final bool showStream;
1024+
final bool inTopicNarrow;
10221025

10231026
@override
10241027
Widget build(BuildContext context) {
@@ -1104,10 +1107,15 @@ class StreamMessageRecipientHeader extends StatelessWidget {
11041107
store.topicVisibilityPolicy(message.streamId, topic))),
11051108
]));
11061109

1110+
// When already in a topic narrow, disable tap interaction that would just
1111+
// push a MessageListPage for the same topic narrow.
1112+
// TODO(#1039) simplify by removing topic-narrow condition if we remove
1113+
// recipient headers in topic narrows
11071114
return GestureDetector(
1108-
onTap: () => Navigator.push(context,
1109-
MessageListPage.buildRoute(context: context,
1110-
narrow: TopicNarrow.ofMessage(message))),
1115+
onTap: !inTopicNarrow
1116+
? () => Navigator.push(context, MessageListPage.buildRoute(context: context,
1117+
narrow: TopicNarrow.ofMessage(message)))
1118+
: null,
11111119
onLongPress: () => showTopicActionSheet(context,
11121120
channelId: message.streamId, topic: topic),
11131121
child: ColoredBox(
@@ -1126,9 +1134,10 @@ class StreamMessageRecipientHeader extends StatelessWidget {
11261134
}
11271135

11281136
class DmRecipientHeader extends StatelessWidget {
1129-
const DmRecipientHeader({super.key, required this.message});
1137+
const DmRecipientHeader({super.key, required this.message, required this.inDmNarrow});
11301138

11311139
final DmMessage message;
1140+
final bool inDmNarrow;
11321141

11331142
@override
11341143
Widget build(BuildContext context) {
@@ -1148,10 +1157,15 @@ class DmRecipientHeader extends StatelessWidget {
11481157

11491158
final messageListTheme = MessageListTheme.of(context);
11501159

1160+
// When already in a DM narrow, disable tap interaction that would just
1161+
// push a MessageListPage for the same DM narrow.
1162+
// TODO(#1244) simplify by removing DM-narrow condition if we remove
1163+
// recipient headers in DM narrows
11511164
return GestureDetector(
1152-
onTap: () => Navigator.push(context,
1153-
MessageListPage.buildRoute(context: context,
1154-
narrow: DmNarrow.ofMessage(message, selfUserId: store.selfUserId))),
1165+
onTap: !inDmNarrow
1166+
? () => Navigator.push(context, MessageListPage.buildRoute(context: context,
1167+
narrow: DmNarrow.ofMessage(message, selfUserId: store.selfUserId)))
1168+
: null,
11551169
child: ColoredBox(
11561170
color: messageListTheme.dmRecipientHeaderBg,
11571171
child: Padding(

test/widgets/message_list_test.dart

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,72 @@ void main() {
922922
await tester.pump();
923923
tester.widget(find.text('new stream name'));
924924
});
925+
926+
testWidgets('navigates to TopicNarrow on tapping topic in CombinedFeedNarrow', (tester) async {
927+
final pushedRoutes = <Route<void>>[];
928+
final navObserver = TestNavigatorObserver()
929+
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
930+
931+
final channel = eg.stream();
932+
final message = eg.streamMessage(stream: channel, topic: 'topic name');
933+
934+
await setupMessageListPage(
935+
tester,
936+
narrow: const CombinedFeedNarrow(),
937+
messages: [message],
938+
subscriptions: [eg.subscription(channel)],
939+
navObservers: [navObserver],
940+
);
941+
942+
assert(pushedRoutes.length == 1);
943+
pushedRoutes.clear();
944+
945+
connection.prepare(json: eg.newestGetMessagesResult(
946+
messages: [message],
947+
foundOldest: true,
948+
).toJson());
949+
950+
final topicFinder = find.descendant(
951+
of: find.byType(StreamMessageRecipientHeader),
952+
matching: find.text('topic name'),
953+
);
954+
await tester.tap(topicFinder);
955+
await tester.pumpAndSettle();
956+
957+
check(pushedRoutes).single
958+
.isA<WidgetRoute>()
959+
.page.isA<MessageListPage>()
960+
.initNarrow.equals(TopicNarrow(channel.streamId, 'topic name'));
961+
});
962+
963+
testWidgets('does not navigate on tapping topic in TopicNarrow', (tester) async {
964+
final pushedRoutes = <Route<void>>[];
965+
final navObserver = TestNavigatorObserver()
966+
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
967+
968+
final channel = eg.stream();
969+
final message = eg.streamMessage(stream: channel, topic: 'topic name');
970+
971+
await setupMessageListPage(
972+
tester,
973+
narrow: TopicNarrow(channel.streamId, 'topic name'),
974+
navObservers: [navObserver],
975+
streams: [channel],
976+
messages: [message],
977+
);
978+
979+
assert(pushedRoutes.length == 1);
980+
pushedRoutes.clear();
981+
982+
final topicFinder = find.descendant(
983+
of: find.byType(StreamMessageRecipientHeader),
984+
matching: find.text('topic name'),
985+
);
986+
await tester.tap(topicFinder);
987+
await tester.pumpAndSettle();
988+
989+
check(pushedRoutes).isEmpty();
990+
});
925991
});
926992

927993
group('DmRecipientHeader', () {
@@ -987,6 +1053,62 @@ void main() {
9871053
tester.widget(find.textContaining(RegExp("Dec 1[89], 2022")));
9881054
tester.widget(find.textContaining(RegExp("Aug 2[23], 2022")));
9891055
});
1056+
1057+
testWidgets('navigates to DmNarrow on tapping recipient header in CombinedFeedNarrow', (tester) async {
1058+
final pushedRoutes = <Route<void>>[];
1059+
final navObserver = TestNavigatorObserver()
1060+
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
1061+
1062+
final dmMessage = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]);
1063+
1064+
await setupMessageListPage(
1065+
tester,
1066+
narrow: const CombinedFeedNarrow(),
1067+
messages: [dmMessage],
1068+
navObservers: [navObserver],
1069+
);
1070+
1071+
assert(pushedRoutes.length == 1);
1072+
pushedRoutes.clear();
1073+
1074+
connection.prepare(json: eg.newestGetMessagesResult(
1075+
messages: [dmMessage],
1076+
foundOldest: true,
1077+
).toJson());
1078+
1079+
final recipientHeaderFinder = find.byType(DmRecipientHeader);
1080+
await tester.tap(recipientHeaderFinder);
1081+
await tester.pumpAndSettle();
1082+
1083+
check(pushedRoutes).single
1084+
.isA<WidgetRoute>()
1085+
.page.isA<MessageListPage>()
1086+
.initNarrow.equals(DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId));
1087+
});
1088+
1089+
testWidgets('does not navigate on tapping recipient header in DmNarrow', (tester) async {
1090+
final pushedRoutes = <Route<void>>[];
1091+
final navObserver = TestNavigatorObserver()
1092+
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
1093+
1094+
final dmMessage = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]);
1095+
1096+
await setupMessageListPage(
1097+
tester,
1098+
narrow: DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId),
1099+
navObservers: [navObserver],
1100+
messages: [dmMessage],
1101+
);
1102+
1103+
assert(pushedRoutes.length == 1);
1104+
pushedRoutes.clear();
1105+
1106+
final recipientHeaderFinder = find.byType(DmRecipientHeader);
1107+
await tester.tap(recipientHeaderFinder);
1108+
await tester.pumpAndSettle();
1109+
1110+
check(pushedRoutes).isEmpty();
1111+
});
9901112
});
9911113

9921114
group('formatHeaderDate', () {

0 commit comments

Comments
 (0)