diff --git a/lib/model/content.dart b/lib/model/content.dart index 74509735aa..358a02feb1 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -1071,38 +1071,66 @@ class LinkNode extends InlineContainerNode { } } -enum UserMentionType { user, userGroup } +sealed class MentionNode extends InlineContainerNode { + const MentionNode({ + super.debugHtmlNode, + required super.nodes, + required this.isSilent, + }); + + final bool isSilent; // TODO(#647) -class UserMentionNode extends InlineContainerNode { + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(FlagProperty('isSilent', value: isSilent, ifTrue: "is silent")); + } +} + +class UserMentionNode extends MentionNode { const UserMentionNode({ super.debugHtmlNode, required super.nodes, + required super.isSilent, required this.userId, - required this.isSilent, }); /// The ID of the user being mentioned. /// - /// This is null for wildcard mentions, user group mentions, + /// This is null for wildcard mentions /// or when the user ID is unavailable in the HTML (e.g., legacy mentions). final int? userId; - final bool isSilent; // TODO(#647) + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(IntProperty('userId', 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 - // and we show that text in the same style for all types of @-mention. - // We'll need these for implementing the post-2023 Zulip design, though. - // final UserMentionType mentionType; // TODO(#646) +class UserGroupMentionNode extends MentionNode { + const UserGroupMentionNode({ + super.debugHtmlNode, + required super.nodes, + required super.isSilent, + required this.userGroupId, + }); + + /// The ID of the user group being mentioned. + /// + /// This is non-nullable because user group mentions + /// always have data-user-group-id. + final int userGroupId; @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); - properties.add(IntProperty('userId', userId)); - properties.add(FlagProperty('isSilent', value: isSilent, ifTrue: "is silent")); + properties.add(IntProperty('userGroupId', userGroupId)); } } +// TODO(#646) add WildcardMentionNode + sealed class EmojiNode extends InlineContentNode { const EmojiNode({super.debugHtmlNode}); } @@ -1292,7 +1320,7 @@ class _ZulipInlineContentParser { debugSoftFailReason: kDebugMode ? parsed.softFailReason : null); } - UserMentionNode? parseUserMention(dom.Element element) { + MentionNode? parseMention(dom.Element element) { assert(element.localName == 'span'); final debugHtmlNode = kDebugMode ? element : null; @@ -1318,15 +1346,16 @@ class _ZulipInlineContentParser { i++; } + String? mentionType; if (i >= classes.length) return null; if ((classes[i] == 'topic-mention' && !hasChannelWildcardClass) || classes[i] == 'user-mention' || (classes[i] == 'user-group-mention' && !hasChannelWildcardClass)) { // The class we already knew we'd find before we called this function. - // We ignore the distinction between these; see [UserMentionNode]. - // Also, we don't expect "user-group-mention" and "channel-wildcard-mention" + // We don't expect "user-group-mention" and "channel-wildcard-mention" // to be in the list at the same time and neither we expect "topic-mention" // and "channel-wildcard-mention" to be in the list at the same time. + mentionType = classes[i]; i++; } @@ -1335,21 +1364,35 @@ class _ZulipInlineContentParser { return null; } - final userId = switch (element.attributes['data-user-id']) { - // For legacy, user group or wildcard mentions. - null || '*' => null, - final userIdString => int.tryParse(userIdString), - }; - - // TODO assert UserMentionNode can't contain LinkNode; + // TODO assert MentionNode can't contain LinkNode; // either a debug-mode check, or perhaps we can make expectations much - // tighter on a UserMentionNode's contents overall. + // tighter on a MentionNode's contents overall. final nodes = parseInlineContentList(element.nodes); - return UserMentionNode( - nodes: nodes, - userId: userId, - isSilent: isSilent, - debugHtmlNode: debugHtmlNode); + + if (mentionType case 'user-group-mention') { + final userGroupId = int.tryParse( + element.attributes['data-user-group-id'] ?? '', + radix: 10); + if (userGroupId == null) { + return null; + } + return UserGroupMentionNode( + nodes: nodes, + isSilent: isSilent, + userGroupId: userGroupId, + debugHtmlNode: debugHtmlNode); + } else { + final userId = switch (element.attributes['data-user-id']) { + // For legacy or wildcard mentions. + null || '*' => null, + final userIdString => int.tryParse(userIdString, radix: 10), + }; + return UserMentionNode( + nodes: nodes, + isSilent: isSilent, + userId: userId, + debugHtmlNode: debugHtmlNode); + } } /// The links found so far in the current block inline container. @@ -1364,11 +1407,11 @@ class _ZulipInlineContentParser { return result; } - /// Matches all className values that could be a UserMentionNode, + /// Matches all className values that could be a subclass of MentionNode, /// and no className values that could be any other type of node. // Specifically, checks for `user-mention` or `user-group-mention` // or `topic-mention` as a member of the list. - static final _userMentionClassNameRegexp = RegExp( + static final _mentionClassNameRegexp = RegExp( r"(^| )" r"(?:user(?:-group)?|topic)-mention" r"( |$)"); static final _emojiClassNameRegexp = () { @@ -1422,8 +1465,8 @@ class _ZulipInlineContentParser { } if (localName == 'span' - && _userMentionClassNameRegexp.hasMatch(className)) { - return parseUserMention(element) ?? unimplemented(); + && _mentionClassNameRegexp.hasMatch(className)) { + return parseMention(element) ?? unimplemented(); } if (localName == 'span' diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index f0100a90f2..126f930403 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -1096,9 +1096,9 @@ class _InlineContentBuilder { case InlineCodeNode(): return _buildInlineCode(node); - case UserMentionNode(): + case MentionNode(): return WidgetSpan(alignment: PlaceholderAlignment.middle, - child: UserMention(ambientTextStyle: widget.style, node: node)); + child: Mention(ambientTextStyle: widget.style, node: node)); case UnicodeEmojiNode(): return TextSpan(text: node.emojiUnicode, recognizer: _recognizer, @@ -1182,27 +1182,37 @@ class _InlineContentBuilder { const kInlineCodeFontSizeFactor = 0.825; -class UserMention extends StatelessWidget { - const UserMention({ +class Mention extends StatelessWidget { + const Mention({ super.key, required this.ambientTextStyle, required this.node, }); final TextStyle ambientTextStyle; - final UserMentionNode node; + final MentionNode node; @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); final contentTheme = ContentTheme.of(context); + var nodes = node.nodes; - if (node.userId case final userId?) { - final user = store.getUser(userId); - if (user case User(:final fullName)) { - nodes = [TextNode(node.isSilent ? fullName : '@$fullName')]; - } + switch (node) { + case UserGroupMentionNode(:final userGroupId): + final userGroup = store.getGroup(userGroupId); + if (userGroup case UserGroup(:final name)) { + // TODO(#1260) Get display name for system groups using localization + nodes = [TextNode(node.isSilent ? name : '@$name')]; + } + case UserMentionNode(:final userId?): + final user = store.getUser(userId); + if (user case User(:final fullName)) { + nodes = [TextNode(node.isSilent ? fullName : '@$fullName')]; + } + case UserMentionNode(userId: null): } + return Container( decoration: BoxDecoration( // TODO(#646) different for wildcard mentions @@ -1213,12 +1223,11 @@ class UserMention extends StatelessWidget { // 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.) + // (The parser on creating a MentionNode 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 + // TODO(#646) distinguish font color between direct and wildcard mentions style: ambientTextStyle, nodes: nodes)); diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 6baa01ca86..b5da630f7e 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')], userId: 2187, isSilent: false)); + const UserMentionNode(nodes: [TextNode('@Greg Price')], isSilent: false, userId: 2187)); static final userMentionSilent = ContentExample.inline( 'silent user @-mention', "@_**Greg Price**", expectedText: 'Greg Price', '

Greg Price

', - const UserMentionNode(nodes: [TextNode('Greg Price')], userId: 2187, isSilent: true)); + const UserMentionNode(nodes: [TextNode('Greg Price')], isSilent: true, userId: 2187)); 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')], userId: 2187, isSilent: true)); + const UserMentionNode(nodes: [TextNode('Greg Price')], isSilent: true, userId: 2187)); static final groupMentionPlain = ContentExample.inline( 'plain group @-mention', "@*test-empty*", expectedText: '@test-empty', '

@test-empty

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

test-empty

', - const UserMentionNode(nodes: [TextNode('test-empty')], userId: null, isSilent: true)); + const UserGroupMentionNode(nodes: [TextNode('test-empty')], isSilent: true, userGroupId: 186)); 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')], userId: null, isSilent: true)); + const UserGroupMentionNode(nodes: [TextNode('test-empty')], isSilent: true, userGroupId: 186)); static final channelWildcardMentionPlain = ContentExample.inline( 'plain channel wildcard @-mention', "@**all**", expectedText: '@all', '

@all

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

everyone

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

channel

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

@channel

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

stream

', - const UserMentionNode(nodes: [TextNode('stream')], userId: null, isSilent: true)); + const UserMentionNode(nodes: [TextNode('stream')], isSilent: true, userId: null)); 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')], userId: null, isSilent: true)); + const UserMentionNode(nodes: [TextNode('all')], isSilent: true, userId: null)); static final topicMentionPlain = ContentExample.inline( 'plain @-topic', "@**topic**", expectedText: '@topic', '

@topic

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

topic

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

topic

', - const UserMentionNode(nodes: [TextNode('topic')], userId: null, isSilent: true)); + const UserMentionNode(nodes: [TextNode('topic')], isSilent: true, userId: null)); static final emojiUnicode = ContentExample.inline( 'Unicode emoji, encoded in span element', diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 45bd24ca90..17af6893eb 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -1429,7 +1429,7 @@ void main() { checkSenderAndTimestampShown(tester, senderId: message.senderId); check(find.descendant( of: find.byType(BottomSheet), - matching: find.byType(UserMention)) + matching: find.byType(Mention)) ).findsOne(); }); @@ -1452,7 +1452,7 @@ void main() { checkSenderAndTimestampShown(tester, senderId: message.senderId); check(find.descendant( of: find.byType(BottomSheet), - matching: find.byType(UserMention)) + matching: find.byType(Mention)) ).findsOne(); }); diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index ab68e97d7a..195c9c8661 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -980,7 +980,7 @@ void main() { }); }); - group('UserMention', () { + group('Mention', () { testContentSmoke(ContentExample.userMentionPlain, wrapWithPerAccountStoreWidget: true); testContentSmoke(ContentExample.userMentionSilent, @@ -1008,10 +1008,10 @@ void main() { testContentSmoke(ContentExample.topicMentionSilentClassOrderReversed, wrapWithPerAccountStoreWidget: true); - UserMention? findUserMentionInSpan(InlineSpan rootSpan) { - UserMention? result; + Mention? findMentionInSpan(InlineSpan rootSpan) { + Mention? result; rootSpan.visitChildren((span) { - if (span case (WidgetSpan(child: UserMention() && var widget))) { + if (span case (WidgetSpan(child: Mention() && var widget))) { result = widget; return false; } @@ -1020,7 +1020,7 @@ void main() { return result; } - TextStyle textStyleFromWidget(WidgetTester tester, UserMention widget, String mentionText) { + TextStyle textStyleFromWidget(WidgetTester tester, Mention widget, String mentionText) { return mergedStyleOf(tester, findAncestor: find.byWidget(widget), mentionText)!; } @@ -1030,7 +1030,7 @@ void main() { targetHtml: '@Chris Bobbe', wrapWithPerAccountStoreWidget: true, targetFontSizeFinder: (rootSpan) { - final widget = findUserMentionInSpan(rootSpan); + final widget = findMentionInSpan(rootSpan); final style = textStyleFromWidget(tester, widget!, '@Chris Bobbe'); return style.fontSize!; }); @@ -1044,7 +1044,7 @@ void main() { wrapWithPerAccountStoreWidget: true, styleFinder: (tester) { return textStyleFromWidget(tester, - tester.widget(find.byType(UserMention)), 'Greg Price'); + tester.widget(find.byType(Mention)), 'Greg Price'); }); // TODO(#647): @@ -1059,14 +1059,14 @@ void main() { wrapWithPerAccountStoreWidget: true, styleFinder: (tester) { return textStyleFromWidget(tester, - tester.widget(find.byType(UserMention)), 'Chris Bobbe'); + tester.widget(find.byType(Mention)), 'Chris Bobbe'); }); // TODO(#647): // testFontWeight('non-silent self-user mention in bold context', // expectedWght: 800, // [etc.] - group('dynamic name resolution', () { + group('user mention dynamic name resolution', () { Future prepare({ required WidgetTester tester, required String html, @@ -1111,6 +1111,45 @@ void main() { check(find.text('@New Name')).findsNothing(); }); }); + + group('user group mention dynamic name resolution', () { + Future prepare({ + required WidgetTester tester, + required String html, + List? userGroups, + }) async { + final initialSnapshot = eg.initialSnapshot(realmUserGroups: userGroups); + await prepareContent(tester, + wrapWithPerAccountStoreWidget: true, + initialSnapshot: initialSnapshot, + plainContent(html)); + } + + testWidgets('resolves current user group name from store', (tester) async { + await prepare( + tester: tester, + html: '

@old-name

', + userGroups: [eg.userGroup(id: 186, name: 'new-name')]); + check(find.text('@new-name')).findsOne(); + check(find.text('@old-name')).findsNothing(); + }); + + testWidgets('falls back to original text when user group not found', (tester) async { + await prepare( + tester: tester, + html: '

@Unknown Group

'); + check(find.text('@Unknown Group')).findsOne(); + }); + + testWidgets('handles silent mentions correctly', (tester) async { + await prepare( + tester: tester, + html: '

old-name

', + userGroups: [eg.userGroup(id: 186, name: 'new-name')]); + check(find.text('new-name')).findsOne(); + check(find.text('@new-name')).findsNothing(); + }); + }); }); Future tapText(WidgetTester tester, Finder textFinder) async {