Skip to content

Commit 75caabe

Browse files
committed
msglist: Tapping a message in Starred or Mentions opens anchored msglist
Fixes zulip#1621.
1 parent 0cf88f3 commit 75caabe

File tree

3 files changed

+113
-4
lines changed

3 files changed

+113
-4
lines changed

lib/widgets/message_list.dart

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:async';
2+
13
import 'package:collection/collection.dart';
24
import 'package:flutter/material.dart';
35
import 'package:flutter_color_models/flutter_color_models.dart';
@@ -974,6 +976,16 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
974976
}
975977
}
976978

979+
/// Whether tapping a message should open its conversation.
980+
bool get _tapMessageOpensConversation => switch (widget.narrow) {
981+
CombinedFeedNarrow()
982+
|| ChannelNarrow()
983+
|| TopicNarrow()
984+
|| DmNarrow() => false,
985+
MentionsNarrow()
986+
|| StarredMessagesNarrow() => true,
987+
};
988+
977989
Widget _buildItem(MessageListItem data) {
978990
switch (data) {
979991
case MessageListRecipientHeaderItem():
@@ -990,10 +1002,16 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
9901002
return MessageItem(
9911003
key: ValueKey(data.message.id),
9921004
header: header,
993-
item: data);
1005+
item: data,
1006+
tapOpensConversation: _tapMessageOpensConversation,
1007+
);
9941008
case MessageListOutboxMessageItem():
9951009
final header = RecipientHeader(message: data.message, narrow: widget.narrow);
996-
return MessageItem(header: header, item: data);
1010+
return MessageItem(
1011+
header: header,
1012+
item: data,
1013+
tapOpensConversation: _tapMessageOpensConversation,
1014+
);
9971015
}
9981016
}
9991017
}
@@ -1317,10 +1335,12 @@ class MessageItem extends StatelessWidget {
13171335
super.key,
13181336
required this.item,
13191337
required this.header,
1338+
required this.tapOpensConversation,
13201339
});
13211340

13221341
final MessageListMessageBaseItem item;
13231342
final Widget header;
1343+
final bool tapOpensConversation;
13241344

13251345
@override
13261346
Widget build(BuildContext context) {
@@ -1331,7 +1351,10 @@ class MessageItem extends StatelessWidget {
13311351
color: designVariables.bgMessageRegular,
13321352
child: Column(children: [
13331353
switch (item) {
1334-
MessageListMessageItem() => MessageWithPossibleSender(item: item),
1354+
MessageListMessageItem() => MessageWithPossibleSender(
1355+
item: item,
1356+
tapOpensConversation: tapOpensConversation,
1357+
),
13351358
MessageListOutboxMessageItem() => OutboxMessageWithPossibleSender(item: item),
13361359
},
13371360
// TODO refine this padding; discussion:
@@ -1748,9 +1771,14 @@ class _SenderRow extends StatelessWidget {
17481771
// - https://github.com/zulip/zulip-mobile/issues/5511
17491772
// - https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A20849&mode=dev
17501773
class MessageWithPossibleSender extends StatelessWidget {
1751-
const MessageWithPossibleSender({super.key, required this.item});
1774+
const MessageWithPossibleSender({
1775+
super.key,
1776+
required this.item,
1777+
required this.tapOpensConversation,
1778+
});
17521779

17531780
final MessageListMessageItem item;
1781+
final bool tapOpensConversation;
17541782

17551783
@override
17561784
Widget build(BuildContext context) {
@@ -1800,6 +1828,13 @@ class MessageWithPossibleSender extends StatelessWidget {
18001828

18011829
return GestureDetector(
18021830
behavior: HitTestBehavior.translucent,
1831+
onTap: tapOpensConversation
1832+
? () => unawaited(Navigator.push(context,
1833+
MessageListPage.buildRoute(context: context,
1834+
narrow: SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
1835+
// TODO(#1655) "this view does not mark messages as read on scroll"
1836+
initAnchorMessageId: message.id)))
1837+
: null,
18031838
onLongPress: () => showMessageActionSheet(context: context, message: message),
18041839
child: Padding(
18051840
padding: const EdgeInsets.only(top: 4),

test/widgets/message_list_checks.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ import 'package:zulip/widgets/message_list.dart';
44

55
extension MessageListPageChecks on Subject<MessageListPage> {
66
Subject<Narrow> get initNarrow => has((x) => x.initNarrow, 'initNarrow');
7+
Subject<int?> get initAnchorMessageId => has((x) => x.initAnchorMessageId, 'initAnchorMessageId');
78
}

test/widgets/message_list_test.dart

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import '../api/fake_api.dart';
3838
import '../example_data.dart' as eg;
3939
import '../model/binding.dart';
4040
import '../model/content_test.dart';
41+
import '../model/message_list_test.dart';
4142
import '../model/test_store.dart';
4243
import '../flutter_checks.dart';
4344
import '../stdlib_checks.dart';
@@ -1653,6 +1654,78 @@ void main() {
16531654

16541655
debugNetworkImageHttpClientProvider = null;
16551656
});
1657+
1658+
group('Opens conversation on tap?', () {
1659+
// (copied from test/widgets/content_test.dart)
1660+
Future<void> tapText(WidgetTester tester, Finder textFinder) async {
1661+
final height = tester.getSize(textFinder).height;
1662+
final target = tester.getTopLeft(textFinder)
1663+
.translate(height/4, height/2); // aim for middle of first letter
1664+
await tester.tapAt(target);
1665+
}
1666+
1667+
final subscription = eg.subscription(eg.stream(streamId: eg.defaultStreamMessageStreamId));
1668+
final topic = 'some topic';
1669+
1670+
void doTest(Narrow narrow, bool expected, {required Message Function() mkMessage}) {
1671+
testWidgets('${expected ? 'yes' : 'no'}, if in $narrow', (tester) async {
1672+
final message = mkMessage();
1673+
1674+
Route<dynamic>? lastPushedRoute;
1675+
final navObserver = TestNavigatorObserver()
1676+
..onPushed = ((route, prevRoute) => lastPushedRoute = route);
1677+
1678+
await setupMessageListPage(
1679+
tester,
1680+
narrow: narrow,
1681+
messages: [message],
1682+
subscriptions: [subscription],
1683+
navObservers: [navObserver]
1684+
);
1685+
lastPushedRoute = null;
1686+
1687+
// Tapping interactive content still works.
1688+
await store.handleEvent(eg.updateMessageEditEvent(message,
1689+
renderedContent: '<p><a href="https://example/">link</a></p>'));
1690+
await tester.pump();
1691+
await tapText(tester, find.text('link'));
1692+
await tester.pump(Duration.zero);
1693+
check(lastPushedRoute).isNull();
1694+
final launchUrlCalls = testBinding.takeLaunchUrlCalls();
1695+
check(launchUrlCalls.single.url).equals(Uri.parse('https://example/'));
1696+
1697+
// Tapping non-interactive content opens the conversation (if expected).
1698+
await store.handleEvent(eg.updateMessageEditEvent(message,
1699+
renderedContent: '<p>plain content</p>'));
1700+
await tester.pump();
1701+
await tapText(tester, find.text('plain content'));
1702+
if (expected) {
1703+
final expectedNarrow = SendableNarrow.ofMessage(message, selfUserId: store.selfUserId);
1704+
1705+
check(lastPushedRoute).isNotNull().isA<MaterialAccountWidgetRoute>()
1706+
.page.isA<MessageListPage>()
1707+
..initNarrow.equals(expectedNarrow)
1708+
..initAnchorMessageId.equals(message.id);
1709+
} else {
1710+
check(lastPushedRoute).isNull();
1711+
}
1712+
1713+
// TODO test tapping whitespace in message
1714+
});
1715+
}
1716+
1717+
doTest(CombinedFeedNarrow(), false, mkMessage: () => eg.streamMessage());
1718+
doTest(ChannelNarrow(subscription.streamId), false,
1719+
mkMessage: () => eg.streamMessage(stream: subscription));
1720+
doTest(TopicNarrow(subscription.streamId, eg.t(topic)), false,
1721+
mkMessage: () => eg.streamMessage(stream: subscription));
1722+
doTest(DmNarrow.withUsers([], selfUserId: eg.selfUser.userId), false,
1723+
mkMessage: () => eg.streamMessage(stream: subscription, topic: topic));
1724+
doTest(StarredMessagesNarrow(), true,
1725+
mkMessage: () => eg.streamMessage(flags: [MessageFlag.starred]));
1726+
doTest(MentionsNarrow(), true,
1727+
mkMessage: () => eg.streamMessage(flags: [MessageFlag.mentioned]));
1728+
});
16561729
});
16571730

16581731
group('OutboxMessageWithPossibleSender', () {

0 commit comments

Comments
 (0)