Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 37 additions & 25 deletions lib/model/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1097,8 +1097,8 @@ class UserMentionNode extends MentionNode {

/// The ID of the user being mentioned.
///
/// This is null for wildcard mentions
/// or when the user ID is unavailable in the HTML (e.g., legacy mentions).
/// This is null when the user ID is unavailable in the HTML,
/// e.g., when the mention is a legacy user mention.
final int? userId;

@override
Expand Down Expand Up @@ -1129,7 +1129,13 @@ class UserGroupMentionNode extends MentionNode {
}
}

// TODO(#646) add WildcardMentionNode
class WildcardMentionNode extends MentionNode {
const WildcardMentionNode({
super.debugHtmlNode,
required super.nodes,
required super.isSilent,
});
}

sealed class EmojiNode extends InlineContentNode {
const EmojiNode({super.debugHtmlNode});
Expand Down Expand Up @@ -1369,29 +1375,35 @@ class _ZulipInlineContentParser {
// tighter on a MentionNode's contents overall.
final nodes = parseInlineContentList(element.nodes);

if (mentionType case 'user-group-mention') {
final userGroupId = int.tryParse(
element.attributes['data-user-group-id'] ?? '',
radix: 10);
if (userGroupId == null) {
switch ((mentionType, element.attributes['data-user-id'])) {
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);
case ('topic-mention', _):
case ('user-mention', _) when hasChannelWildcardClass:
case ('user-mention', '*'): // legacy channel wildcard
return WildcardMentionNode(
nodes: nodes,
isSilent: isSilent,
debugHtmlNode: debugHtmlNode);
case ('user-mention', final userIdString):
final userId = int.tryParse(userIdString ?? '', radix: 10);
return UserMentionNode(
nodes: nodes,
isSilent: isSilent,
userId: userId,
debugHtmlNode: debugHtmlNode);
case _:
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);
}
}

Expand Down
20 changes: 16 additions & 4 deletions lib/widgets/content.dart
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can cut the commit-ID to 9 characters:

…
https://github.com/zulip/zulip/blob/81e0305f9/web/styles/app_variables.css#L2004
https://github.com/zulip/zulip/blob/81e0305f9/web/styles/app_variables.css#L2012
…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also nit: Indent the links with 2-spaces.

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
return ContentTheme._(
colorCodeBlockBackground: const HSLColor.fromAHSL(0.04, 0, 0, 0).toColor(),
colorDirectMentionBackground: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(),
colorGroupMentionBackground: const HSLColor.fromAHSL(0.18, 183, 0.6, 0.45).toColor(),
colorGlobalTimeBackground: const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor(),
colorGlobalTimeBorder: const HSLColor.fromAHSL(1, 0, 0, 0.8).toColor(),
colorLink: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor(),
Expand Down Expand Up @@ -77,6 +78,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
return ContentTheme._(
colorCodeBlockBackground: const HSLColor.fromAHSL(0.04, 0, 0, 1).toColor(),
colorDirectMentionBackground: const HSLColor.fromAHSL(0.25, 240, 0.52, 0.6).toColor(),
colorGroupMentionBackground: const HSLColor.fromAHSL(0.20, 183, 0.52, 0.4).toColor(),
colorGlobalTimeBackground: const HSLColor.fromAHSL(0.2, 0, 0, 0).toColor(),
colorGlobalTimeBorder: const HSLColor.fromAHSL(0.4, 0, 0, 0).toColor(),
colorLink: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor(), // the same as light in Web
Expand Down Expand Up @@ -110,6 +112,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
ContentTheme._({
required this.colorCodeBlockBackground,
required this.colorDirectMentionBackground,
required this.colorGroupMentionBackground,
required this.colorGlobalTimeBackground,
required this.colorGlobalTimeBorder,
required this.colorLink,
Expand Down Expand Up @@ -143,6 +146,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {

final Color colorCodeBlockBackground;
final Color colorDirectMentionBackground;
final Color colorGroupMentionBackground;
final Color colorGlobalTimeBackground;
final Color colorGlobalTimeBorder;
final Color colorLink;
Expand Down Expand Up @@ -204,6 +208,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
ContentTheme copyWith({
Color? colorCodeBlockBackground,
Color? colorDirectMentionBackground,
Color? colorGroupMentionBackground,
Color? colorGlobalTimeBackground,
Color? colorGlobalTimeBorder,
Color? colorLink,
Expand All @@ -227,6 +232,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
return ContentTheme._(
colorCodeBlockBackground: colorCodeBlockBackground ?? this.colorCodeBlockBackground,
colorDirectMentionBackground: colorDirectMentionBackground ?? this.colorDirectMentionBackground,
colorGroupMentionBackground: colorGroupMentionBackground ?? this.colorGroupMentionBackground,
colorGlobalTimeBackground: colorGlobalTimeBackground ?? this.colorGlobalTimeBackground,
colorGlobalTimeBorder: colorGlobalTimeBorder ?? this.colorGlobalTimeBorder,
colorLink: colorLink ?? this.colorLink,
Expand Down Expand Up @@ -257,6 +263,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
return ContentTheme._(
colorCodeBlockBackground: Color.lerp(colorCodeBlockBackground, other.colorCodeBlockBackground, t)!,
colorDirectMentionBackground: Color.lerp(colorDirectMentionBackground, other.colorDirectMentionBackground, t)!,
colorGroupMentionBackground: Color.lerp(colorGroupMentionBackground, other.colorGroupMentionBackground, t)!,
colorGlobalTimeBackground: Color.lerp(colorGlobalTimeBackground, other.colorGlobalTimeBackground, t)!,
colorGlobalTimeBorder: Color.lerp(colorGlobalTimeBorder, other.colorGlobalTimeBorder, t)!,
colorLink: Color.lerp(colorLink, other.colorLink, t)!,
Expand Down Expand Up @@ -1220,12 +1227,18 @@ class Mention extends StatelessWidget {
nodes = [TextNode(node.isSilent ? fullName : '@$fullName')];
}
case UserMentionNode(userId: null):
case WildcardMentionNode():
}

final backgroundPillColor = switch (node) {
UserMentionNode() => contentTheme.colorDirectMentionBackground,
UserGroupMentionNode() || WildcardMentionNode()
=> contentTheme.colorGroupMentionBackground,
};

return Container(
decoration: BoxDecoration(
// TODO(#646) different for wildcard mentions
color: contentTheme.colorDirectMentionBackground,
color: backgroundPillColor,
borderRadius: const BorderRadius.all(Radius.circular(3))),
padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize),
child: InlineContent(
Expand All @@ -1235,8 +1248,7 @@ class Mention extends StatelessWidget {
// (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) distinguish font color between direct and wildcard mentions
// TODO(#647) when self-user is mentioned, make bold, and change font color.
style: ambientTextStyle,

nodes: nodes));
Expand Down
29 changes: 20 additions & 9 deletions test/model/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ class ContentExample {
'<p><span class="silent user-mention" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')], isSilent: true, userId: 2187));

// No silent version of this form, because silent mentions were new
// in zulip/zulip#11221, long after this legacy form was removed.
static final legacyUserMention = ContentExample.inline(
'legacy user @-mention',
"@**Greg Price**", // (before 2017-01)
expectedText: '@Greg Price',
'<p><span class="user-mention" data-user-email="email@example.com">@Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('@Greg Price')], isSilent: false, userId: null));

static final groupMentionPlain = ContentExample.inline(
'plain group @-mention',
"@*test-empty*",
Expand All @@ -153,63 +162,63 @@ class ContentExample {
"@**all**",
expectedText: '@all',
'<p><span class="user-mention channel-wildcard-mention" data-user-id="*">@all</span></p>',
const UserMentionNode(nodes: [TextNode('@all')], isSilent: false, userId: null));
const WildcardMentionNode(nodes: [TextNode('@all')], isSilent: false));

static final channelWildcardMentionSilent = ContentExample.inline(
'silent channel wildcard @-mention',
"@_**everyone**",
expectedText: 'everyone',
'<p><span class="user-mention channel-wildcard-mention silent" data-user-id="*">everyone</span></p>',
const UserMentionNode(nodes: [TextNode('everyone')], isSilent: true, userId: null));
const WildcardMentionNode(nodes: [TextNode('everyone')], isSilent: true));

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

static final legacyChannelWildcardMentionPlain = ContentExample.inline(
'legacy plain channel wildcard @-mention',
"@**channel**",
expectedText: '@channel',
'<p><span class="user-mention" data-user-id="*">@channel</span></p>',
const UserMentionNode(nodes: [TextNode('@channel')], isSilent: false, userId: null));
const WildcardMentionNode(nodes: [TextNode('@channel')], isSilent: false));

static final legacyChannelWildcardMentionSilent = ContentExample.inline(
'legacy silent channel wildcard @-mention',
"@_**stream**",
expectedText: 'stream',
'<p><span class="user-mention silent" data-user-id="*">stream</span></p>',
const UserMentionNode(nodes: [TextNode('stream')], isSilent: true, userId: null));
const WildcardMentionNode(nodes: [TextNode('stream')], isSilent: true));

static final legacyChannelWildcardMentionSilentClassOrderReversed = ContentExample.inline(
'legacy silent channel wildcard @-mention, class order reversed',
"@_**all**", // (hypothetical server variation)
expectedText: 'all',
'<p><span class="silent user-mention" data-user-id="*">all</span></p>',
const UserMentionNode(nodes: [TextNode('all')], isSilent: true, userId: null));
const WildcardMentionNode(nodes: [TextNode('all')], isSilent: true));

static final topicMentionPlain = ContentExample.inline(
'plain @-topic',
"@**topic**",
expectedText: '@topic',
'<p><span class="topic-mention">@topic</span></p>',
const UserMentionNode(nodes: [TextNode('@topic')], isSilent: false, userId: null));
const WildcardMentionNode(nodes: [TextNode('@topic')], isSilent: false));

static final topicMentionSilent = ContentExample.inline(
'silent @-topic',
"@_**topic**",
expectedText: 'topic',
'<p><span class="topic-mention silent">topic</span></p>',
const UserMentionNode(nodes: [TextNode('topic')], isSilent: true, userId: null));
const WildcardMentionNode(nodes: [TextNode('topic')], isSilent: true));

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

static final emojiUnicode = ContentExample.inline(
'Unicode emoji, encoded in span element',
Expand Down Expand Up @@ -1868,6 +1877,8 @@ void main() async {
testParseExample(ContentExample.userMentionSilent);
testParseExample(ContentExample.userMentionSilentClassOrderReversed);

testParseExample(ContentExample.legacyUserMention);

testParseExample(ContentExample.groupMentionPlain);
testParseExample(ContentExample.groupMentionSilent);
testParseExample(ContentExample.groupMentionSilentClassOrderReversed);
Expand Down
49 changes: 47 additions & 2 deletions test/widgets/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_checks/flutter_checks.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:legacy_checks/legacy_checks.dart';
import 'package:url_launcher/url_launcher.dart';
import 'package:zulip/api/model/initial_snapshot.dart';
import 'package:zulip/api/model/model.dart';
Expand Down Expand Up @@ -985,10 +986,16 @@ void main() {
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.userMentionSilent,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.userMentionSilentClassOrderReversed,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.legacyUserMention,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.groupMentionPlain,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.groupMentionSilent,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.groupMentionSilentClassOrderReversed,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.channelWildcardMentionPlain,
wrapWithPerAccountStoreWidget: true);
testContentSmoke(ContentExample.channelWildcardMentionSilent,
Expand Down Expand Up @@ -1066,6 +1073,44 @@ void main() {
// testFontWeight('non-silent self-user mention in bold context',
// expectedWght: 800, // [etc.]

group('pill color', () {
Future<void> checkPillColor(WidgetTester tester, {
required String html,
required Color Function(ContentTheme) expectColor,
}) async {
await prepareContent(tester,
wrapWithPerAccountStoreWidget: true,
plainContent(html));

final renderObject = tester.renderObject<RenderBox>(find.byType(Mention));
final paintBounds = renderObject.paintBounds;
final contentTheme = ContentTheme.of(tester.element(find.byType(Mention)));

check(renderObject).legacyMatcher(equals(paints..rrect(
rrect: RRect.fromRectAndRadius(paintBounds, const Radius.circular(3)),
style: .fill,
color: expectColor(contentTheme))));
}

testWidgets('user mention', (tester) async {
await checkPillColor(tester,
html: ContentExample.userMentionPlain.html,
expectColor: (theme) => theme.colorDirectMentionBackground);
});

testWidgets('user group mention', (tester) async {
await checkPillColor(tester,
html: ContentExample.groupMentionPlain.html,
expectColor: (theme) => theme.colorGroupMentionBackground);
});

testWidgets('wildcard mention', (tester) async {
await checkPillColor(tester,
html: ContentExample.channelWildcardMentionPlain.html,
expectColor: (theme) => theme.colorGroupMentionBackground);
});
});

group('user mention dynamic name resolution', () {
Future<void> prepare({
required WidgetTester tester,
Expand Down Expand Up @@ -1098,8 +1143,8 @@ void main() {
testWidgets('falls back to original text when userId is null', (tester) async {
await prepare(
tester: tester,
html: '<p><span class="user-mention channel-wildcard-mention" data-user-id="*">@all</span></p>');
check(find.text('@all')).findsOne();
html: '<p><span class="user-mention" data-user-email="email@example.com">@Greg Price</span></p>');
check(find.text('@Greg Price')).findsOne();
});

testWidgets('handles silent mentions correctly', (tester) async {
Expand Down
Loading