Skip to content

Commit f35dc2a

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

File tree

3 files changed

+112
-4
lines changed

3 files changed

+112
-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: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1653,6 +1653,78 @@ void main() {
16531653

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

16581730
group('OutboxMessageWithPossibleSender', () {

0 commit comments

Comments
 (0)