From 0194098738406e572e947c62e7b4534edbbefb82 Mon Sep 17 00:00:00 2001 From: loveucifer Date: Tue, 7 Oct 2025 06:20:52 +0530 Subject: [PATCH 1/2] feat: make @-mentions tappable to navigate to user profiles --- lib/model/content.dart | 13 +++- lib/widgets/content.dart | 59 ++++++++++++------ test/widgets/user_mention_test.dart | 94 +++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 20 deletions(-) create mode 100644 test/widgets/user_mention_test.dart diff --git a/lib/model/content.dart b/lib/model/content.dart index 4413857173..dfa958aa70 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -928,8 +928,11 @@ class UserMentionNode extends InlineContainerNode { const UserMentionNode({ super.debugHtmlNode, required super.nodes, + this.userId, }); + final int? userId; + // For the legacy design, we don't need this information in code; instead, // the inner text already shows how to communicate it to the user // (e.g., silent mentions' text lacks a leading "@"), @@ -1083,7 +1086,15 @@ class _ZulipInlineContentParser { // either a debug-mode check, or perhaps we can make expectations much // tighter on a UserMentionNode's contents overall. final nodes = parseInlineContentList(element.nodes); - return UserMentionNode(nodes: nodes, debugHtmlNode: debugHtmlNode); + + // Extract user ID from data-user-id attribute if present + int? userId; + final userIdStr = element.attributes['data-user-id']; + if (userIdStr != null) { + userId = int.tryParse(userIdStr); + } + + return UserMentionNode(nodes: nodes, debugHtmlNode: debugHtmlNode, userId: userId); } /// The links found so far in the current block inline container. diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 2816e90233..cc2e67673b 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -21,6 +21,7 @@ import 'katex.dart'; import 'lightbox.dart'; import 'message_list.dart'; import 'poll.dart'; +import 'profile.dart'; import 'scrolling.dart'; import 'store.dart'; import 'text.dart'; @@ -1216,25 +1217,45 @@ class UserMention extends StatelessWidget { @override Widget build(BuildContext context) { final contentTheme = ContentTheme.of(context); - return Container( - decoration: BoxDecoration( - // TODO(#646) different for wildcard mentions - color: contentTheme.colorDirectMentionBackground, - borderRadius: const BorderRadius.all(Radius.circular(3))), - padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), - child: InlineContent( - // If an @-mention is inside a link, let the @-mention override it. - recognizer: null, // TODO(#1867) make @-mentions tappable, for info on user - // One hopes an @-mention can't contain an embedded link. - // (The parser on creating a UserMentionNode has a TODO to check that.) - linkRecognizers: null, - - // TODO(#647) when self-user is non-silently mentioned, make bold, and: - // TODO(#646) when self-user is non-silently mentioned, - // distinguish font color between direct and wildcard mentions - style: ambientTextStyle, - - nodes: node.nodes)); + final userId = node.userId; + + final innerContent = InlineContent( + // If an @-mention is inside a link, let the @-mention override it. + recognizer: null, + // One hopes an @-mention can't contain an embedded link. + // (The parser on creating a UserMentionNode has a TODO to check that.) + linkRecognizers: null, + + style: ambientTextStyle, + + nodes: node.nodes); + + if (userId != null && userId > 0) { + // Wrap with gesture detector if we have a valid user ID + return GestureDetector( + onTap: () => Navigator.push( + context, + ProfilePage.buildRoute(context: context, userId: userId), + ), + child: Container( + decoration: BoxDecoration( + // TODO(#646) different for wildcard mentions + color: contentTheme.colorDirectMentionBackground, + borderRadius: const BorderRadius.all(Radius.circular(3))), + padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), + child: innerContent, + ), + ); + } else { + // Regular container without gesture detector if no valid user ID + return Container( + decoration: BoxDecoration( + // TODO(#646) different for wildcard mentions + color: contentTheme.colorDirectMentionBackground, + borderRadius: const BorderRadius.all(Radius.circular(3))), + padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), + child: innerContent); + } } // This is a more literal translation of Zulip web's CSS. diff --git a/test/widgets/user_mention_test.dart b/test/widgets/user_mention_test.dart new file mode 100644 index 0000000000..516dae56bc --- /dev/null +++ b/test/widgets/user_mention_test.dart @@ -0,0 +1,94 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/model/content.dart'; +import 'package:zulip/widgets/content.dart'; + +import '../example_data.dart' as eg; +import '../model/binding.dart'; +import '../test_navigation.dart'; +import 'test_app.dart'; + +Widget plainContent(String html) { + return Builder(builder: (context) => + DefaultTextStyle( + style: ContentTheme.of(context).textStylePlainParagraph, + child: BlockContentList(nodes: parseContent(html).nodes))); +} + +Future prepareContent(WidgetTester tester, Widget child, { + bool wrapWithPerAccountStoreWidget = false, +}) async { + if (wrapWithPerAccountStoreWidget) { + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + } + addTearDown(testBinding.reset); + + await tester.pumpWidget(TestZulipApp( + accountId: wrapWithPerAccountStoreWidget ? eg.selfAccount.id : null, + child: child)); + await tester.pump(); // global store + if (wrapWithPerAccountStoreWidget) { + await tester.pump(); + } +} + +void main() { + TestZulipBinding.ensureInitialized(); + + group('UserMention tappable functionality', () { + testWidgets('mention with valid user ID has gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsOneWidget); + }); + + testWidgets('mention with user ID navigates to ProfilePage when tapped', (tester) async { + final pushedRoutes = >[]; + final testNavObserver = TestNavigatorObserver() + ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + navigatorObservers: [testNavObserver], + child: plainContent('

@Test User

'), + )); + await tester.pump(); // global store + + await tester.pump(); // Allow any deferred work to complete + + expect(find.byType(GestureDetector), findsOneWidget); + + await tester.tap(find.byType(GestureDetector)); + await tester.pump(); + + // Verify that navigation occurred (at least one route was pushed) + expect(pushedRoutes.length, greaterThanOrEqualTo(1)); + }); + + testWidgets('mention without user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with invalid user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with wildcard user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@all

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with zero user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with negative user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + }); +} \ No newline at end of file From 5aa865ba25ffe7de902668e4222d7bfdd55cffa8 Mon Sep 17 00:00:00 2001 From: loveucifer Date: Mon, 27 Oct 2025 18:23:34 +0530 Subject: [PATCH 2/2] feat: implement tappable @-mentions - Make userId required in UserMentionNode for explicit parameters - Reorder constructor parameters - Refactor UserMention widget to avoid code duplication - Move tests to content_test.dart --- lib/model/content.dart | 2 +- lib/widgets/content.dart | 29 ++++----- test/model/content_test.dart | 30 ++++----- test/widgets/content_test.dart | 57 +++++++++++++++++ test/widgets/user_mention_test.dart | 94 ----------------------------- 5 files changed, 84 insertions(+), 128 deletions(-) delete mode 100644 test/widgets/user_mention_test.dart diff --git a/lib/model/content.dart b/lib/model/content.dart index dfa958aa70..498a5d2d8b 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -926,9 +926,9 @@ enum UserMentionType { user, userGroup } class UserMentionNode extends InlineContainerNode { const UserMentionNode({ + required this.userId, super.debugHtmlNode, required super.nodes, - this.userId, }); final int? userId; diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index cc2e67673b..b3ceb4fa37 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -1218,7 +1218,7 @@ class UserMention extends StatelessWidget { Widget build(BuildContext context) { final contentTheme = ContentTheme.of(context); final userId = node.userId; - + final innerContent = InlineContent( // If an @-mention is inside a link, let the @-mention override it. recognizer: null, @@ -1230,31 +1230,24 @@ class UserMention extends StatelessWidget { nodes: node.nodes); + final widget = Container( + decoration: BoxDecoration( + // TODO(#646) different for wildcard mentions + color: contentTheme.colorDirectMentionBackground, + borderRadius: const BorderRadius.all(Radius.circular(3))), + padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), + child: innerContent); + if (userId != null && userId > 0) { - // Wrap with gesture detector if we have a valid user ID return GestureDetector( onTap: () => Navigator.push( context, ProfilePage.buildRoute(context: context, userId: userId), ), - child: Container( - decoration: BoxDecoration( - // TODO(#646) different for wildcard mentions - color: contentTheme.colorDirectMentionBackground, - borderRadius: const BorderRadius.all(Radius.circular(3))), - padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), - child: innerContent, - ), + child: widget, ); } else { - // Regular container without gesture detector if no valid user ID - return Container( - decoration: BoxDecoration( - // TODO(#646) different for wildcard mentions - color: contentTheme.colorDirectMentionBackground, - borderRadius: const BorderRadius.all(Radius.circular(3))), - padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), - child: innerContent); + return widget; } } diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 84e3baed42..6c2aaa6acb 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -111,105 +111,105 @@ class ContentExample { "@**Greg Price**", expectedText: '@Greg Price', '

@Greg Price

', - const UserMentionNode(nodes: [TextNode('@Greg Price')])); + const UserMentionNode(userId: null, nodes: [TextNode('@Greg Price')])); static final userMentionSilent = ContentExample.inline( 'silent user @-mention', "@_**Greg Price**", expectedText: 'Greg Price', '

Greg Price

', - const UserMentionNode(nodes: [TextNode('Greg Price')])); + const UserMentionNode(userId: null, nodes: [TextNode('Greg Price')])); static final userMentionSilentClassOrderReversed = ContentExample.inline( 'silent user @-mention, class order reversed', "@_**Greg Price**", // (hypothetical server variation) expectedText: 'Greg Price', '

Greg Price

', - const UserMentionNode(nodes: [TextNode('Greg Price')])); + const UserMentionNode(userId: null, nodes: [TextNode('Greg Price')])); static final groupMentionPlain = ContentExample.inline( 'plain group @-mention', "@*test-empty*", expectedText: '@test-empty', '

@test-empty

', - const UserMentionNode(nodes: [TextNode('@test-empty')])); + const UserMentionNode(userId: null, nodes: [TextNode('@test-empty')])); static final groupMentionSilent = ContentExample.inline( 'silent group @-mention', "@_*test-empty*", expectedText: 'test-empty', '

test-empty

', - const UserMentionNode(nodes: [TextNode('test-empty')])); + const UserMentionNode(userId: null, nodes: [TextNode('test-empty')])); static final groupMentionSilentClassOrderReversed = ContentExample.inline( 'silent group @-mention, class order reversed', "@_*test-empty*", // (hypothetical server variation) expectedText: 'test-empty', '

test-empty

', - const UserMentionNode(nodes: [TextNode('test-empty')])); + const UserMentionNode(userId: null, nodes: [TextNode('test-empty')])); static final channelWildcardMentionPlain = ContentExample.inline( 'plain channel wildcard @-mention', "@**all**", expectedText: '@all', '

@all

', - const UserMentionNode(nodes: [TextNode('@all')])); + const UserMentionNode(userId: null, nodes: [TextNode('@all')])); static final channelWildcardMentionSilent = ContentExample.inline( 'silent channel wildcard @-mention', "@_**everyone**", expectedText: 'everyone', '

everyone

', - const UserMentionNode(nodes: [TextNode('everyone')])); + const UserMentionNode(userId: null, nodes: [TextNode('everyone')])); static final channelWildcardMentionSilentClassOrderReversed = ContentExample.inline( 'silent channel wildcard @-mention, class order reversed', "@_**channel**", // (hypothetical server variation) expectedText: 'channel', '

channel

', - const UserMentionNode(nodes: [TextNode('channel')])); + const UserMentionNode(userId: null, nodes: [TextNode('channel')])); static final legacyChannelWildcardMentionPlain = ContentExample.inline( 'legacy plain channel wildcard @-mention', "@**channel**", expectedText: '@channel', '

@channel

', - const UserMentionNode(nodes: [TextNode('@channel')])); + const UserMentionNode(userId: null, nodes: [TextNode('@channel')])); static final legacyChannelWildcardMentionSilent = ContentExample.inline( 'legacy silent channel wildcard @-mention', "@_**stream**", expectedText: 'stream', '

stream

', - const UserMentionNode(nodes: [TextNode('stream')])); + const UserMentionNode(userId: null, nodes: [TextNode('stream')])); static final legacyChannelWildcardMentionSilentClassOrderReversed = ContentExample.inline( 'legacy silent channel wildcard @-mention, class order reversed', "@_**all**", // (hypothetical server variation) expectedText: 'all', '

all

', - const UserMentionNode(nodes: [TextNode('all')])); + const UserMentionNode(userId: null, nodes: [TextNode('all')])); static final topicMentionPlain = ContentExample.inline( 'plain @-topic', "@**topic**", expectedText: '@topic', '

@topic

', - const UserMentionNode(nodes: [TextNode('@topic')])); + const UserMentionNode(userId: null, nodes: [TextNode('@topic')])); static final topicMentionSilent = ContentExample.inline( 'silent @-topic', "@_**topic**", expectedText: 'topic', '

topic

', - const UserMentionNode(nodes: [TextNode('topic')])); + const UserMentionNode(userId: null, nodes: [TextNode('topic')])); static final topicMentionSilentClassOrderReversed = ContentExample.inline( 'silent @-topic, class order reversed', "@_**topic**", // (hypothetical server variation) expectedText: 'topic', '

topic

', - const UserMentionNode(nodes: [TextNode('topic')])); + const UserMentionNode(userId: null, nodes: [TextNode('topic')])); static final emojiUnicode = ContentExample.inline( 'Unicode emoji, encoded in span element', diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 28af496c59..018b065a02 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -1383,4 +1383,61 @@ void main() { check(linkText.textAlign).equals(TextAlign.center); }); }); + + group('UserMention tappable functionality', () { + testWidgets('mention with valid user ID has gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsOneWidget); + }); + + testWidgets('mention with user ID navigates to ProfilePage when tapped', (tester) async { + final pushedRoutes = >[]; + final testNavObserver = TestNavigatorObserver() + ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + navigatorObservers: [testNavObserver], + child: plainContent('

@Test User

'), + )); + await tester.pump(); // global store + + await tester.pump(); // Allow any deferred work to complete + + expect(find.byType(GestureDetector), findsOneWidget); + + await tester.tap(find.byType(GestureDetector)); + await tester.pump(); + + // Verify that navigation occurred (at least one route was pushed) + expect(pushedRoutes.length, greaterThanOrEqualTo(1)); + }); + + testWidgets('mention without user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with invalid user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with wildcard user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@all

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with zero user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + + testWidgets('mention with negative user ID does not have gesture detector', (tester) async { + await prepareContent(tester, plainContent('

@Test User

')); + expect(find.byType(GestureDetector), findsNothing); + }); + }); } diff --git a/test/widgets/user_mention_test.dart b/test/widgets/user_mention_test.dart deleted file mode 100644 index 516dae56bc..0000000000 --- a/test/widgets/user_mention_test.dart +++ /dev/null @@ -1,94 +0,0 @@ -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/model/content.dart'; -import 'package:zulip/widgets/content.dart'; - -import '../example_data.dart' as eg; -import '../model/binding.dart'; -import '../test_navigation.dart'; -import 'test_app.dart'; - -Widget plainContent(String html) { - return Builder(builder: (context) => - DefaultTextStyle( - style: ContentTheme.of(context).textStylePlainParagraph, - child: BlockContentList(nodes: parseContent(html).nodes))); -} - -Future prepareContent(WidgetTester tester, Widget child, { - bool wrapWithPerAccountStoreWidget = false, -}) async { - if (wrapWithPerAccountStoreWidget) { - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - } - addTearDown(testBinding.reset); - - await tester.pumpWidget(TestZulipApp( - accountId: wrapWithPerAccountStoreWidget ? eg.selfAccount.id : null, - child: child)); - await tester.pump(); // global store - if (wrapWithPerAccountStoreWidget) { - await tester.pump(); - } -} - -void main() { - TestZulipBinding.ensureInitialized(); - - group('UserMention tappable functionality', () { - testWidgets('mention with valid user ID has gesture detector', (tester) async { - await prepareContent(tester, plainContent('

@Test User

')); - expect(find.byType(GestureDetector), findsOneWidget); - }); - - testWidgets('mention with user ID navigates to ProfilePage when tapped', (tester) async { - final pushedRoutes = >[]; - final testNavObserver = TestNavigatorObserver() - ..onPushed = (route, prevRoute) => pushedRoutes.add(route); - - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - addTearDown(testBinding.reset); - await tester.pumpWidget(TestZulipApp( - accountId: eg.selfAccount.id, - navigatorObservers: [testNavObserver], - child: plainContent('

@Test User

'), - )); - await tester.pump(); // global store - - await tester.pump(); // Allow any deferred work to complete - - expect(find.byType(GestureDetector), findsOneWidget); - - await tester.tap(find.byType(GestureDetector)); - await tester.pump(); - - // Verify that navigation occurred (at least one route was pushed) - expect(pushedRoutes.length, greaterThanOrEqualTo(1)); - }); - - testWidgets('mention without user ID does not have gesture detector', (tester) async { - await prepareContent(tester, plainContent('

@Test User

')); - expect(find.byType(GestureDetector), findsNothing); - }); - - testWidgets('mention with invalid user ID does not have gesture detector', (tester) async { - await prepareContent(tester, plainContent('

@Test User

')); - expect(find.byType(GestureDetector), findsNothing); - }); - - testWidgets('mention with wildcard user ID does not have gesture detector', (tester) async { - await prepareContent(tester, plainContent('

@all

')); - expect(find.byType(GestureDetector), findsNothing); - }); - - testWidgets('mention with zero user ID does not have gesture detector', (tester) async { - await prepareContent(tester, plainContent('

@Test User

')); - expect(find.byType(GestureDetector), findsNothing); - }); - - testWidgets('mention with negative user ID does not have gesture detector', (tester) async { - await prepareContent(tester, plainContent('

@Test User

')); - expect(find.byType(GestureDetector), findsNothing); - }); - }); -} \ No newline at end of file