From 2f9a8f6f6c5fb744ae54c9eff2c96e6d1ec69486 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 8 Jul 2025 23:15:26 -0700 Subject: [PATCH] msglist: In keyword search narrow, highlight keywords in message content Fixes #1692. --- lib/model/content.dart | 8 +++++++ lib/model/message_list.dart | 41 +++++++++++++++++++++++++++++++++- lib/widgets/content.dart | 14 ++++++++++++ test/model/content_test.dart | 9 ++++++++ test/widgets/content_test.dart | 2 ++ 5 files changed, 73 insertions(+), 1 deletion(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 79f3181963..037aea16a9 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -913,6 +913,10 @@ class GlobalTimeNode extends InlineContentNode { } } +class HighlightNode extends InlineContainerNode { + const HighlightNode({super.debugHtmlNode, required super.nodes}); +} + //////////////////////////////////////////////////////////////// /// Parser for the inline-content subtrees within Zulip content HTML. @@ -1087,6 +1091,10 @@ class _ZulipInlineContentParser { return GlobalTimeNode(datetime: datetime, debugHtmlNode: debugHtmlNode); } + if (localName == 'span' && className == 'highlight') { + return HighlightNode(nodes: nodes(), debugHtmlNode: debugHtmlNode); + } + if (localName == 'audio' && className.isEmpty) { final srcAttr = element.attributes['src']; if (srcAttr == null) return unimplemented(); diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 9128617b13..9d2f5c2235 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -118,6 +118,12 @@ mixin _MessageSequence { @visibleForTesting bool get oneMessagePerBlock; + /// Whether the narrow includes a keyword search. + /// + /// If false, messages won't be expected to have + /// [Message.matchContent] or [Message.matchTopic]. + bool get hasKeywordSearchFilter; + /// A sequence number for invalidating stale fetches. int generation = 0; @@ -257,10 +263,25 @@ mixin _MessageSequence { } } + final Map _matchContentByMessageId = {}; + + void _captureMatchContentAndTopic(List messages) { + if (!hasKeywordSearchFilter) return; + + for (final message in messages) { + final Message(:matchContent, :matchTopic) = message; + if (matchContent != null) { + _matchContentByMessageId[message.id] = matchContent; + } + // TODO matchTopic + } + } + ZulipMessageContent _parseMessageContent(Message message) { final poll = message.poll; if (poll != null) return PollContent(poll); - return parseContent(message.content); + final content = _matchContentByMessageId[message.id] ?? message.content; + return parseContent(content); } /// Update data derived from the content of the index-th message. @@ -419,6 +440,7 @@ mixin _MessageSequence { contents.clear(); items.clear(); middleItem = 0; + _matchContentByMessageId.clear(); } /// Redo all computations from scratch, based on [messages]. @@ -659,6 +681,16 @@ class MessageListView with ChangeNotifier, _MessageSequence { || KeywordSearchNarrow() => true, }; + @override bool get hasKeywordSearchFilter => switch (narrow) { + CombinedFeedNarrow() + || ChannelNarrow() + || TopicNarrow() + || DmNarrow() + || MentionsNarrow() + || StarredMessagesNarrow() => false, + KeywordSearchNarrow() => true, + }; + /// Whether [message] should actually appear in this message list, /// given that it does belong to the narrow. /// @@ -793,6 +825,8 @@ class MessageListView with ChangeNotifier, _MessageSequence { _adjustNarrowForTopicPermalink(result.messages.firstOrNull); + _captureMatchContentAndTopic(result.messages); + store.reconcileMessages(result.messages); store.recentSenders.handleMessages(result.messages); // TODO(#824) @@ -877,6 +911,8 @@ class MessageListView with ChangeNotifier, _MessageSequence { result.messages.removeLast(); } + _captureMatchContentAndTopic(result.messages); + store.reconcileMessages(result.messages); store.recentSenders.handleMessages(result.messages); // TODO(#824) @@ -913,6 +949,8 @@ class MessageListView with ChangeNotifier, _MessageSequence { result.messages.removeAt(0); } + _captureMatchContentAndTopic(result.messages); + store.reconcileMessages(result.messages); store.recentSenders.handleMessages(result.messages); // TODO(#824) @@ -1149,6 +1187,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { void messageContentChanged(int messageId) { final index = _findMessageWithId(messageId); if (index != -1) { + _matchContentByMessageId.remove(messageId); _reparseContent(index); } } diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index b49fdb4d9c..77fa51254b 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -48,6 +48,7 @@ class ContentTheme extends ThemeExtension { colorDirectMentionBackground: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(), colorGlobalTimeBackground: const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor(), colorGlobalTimeBorder: const HSLColor.fromAHSL(1, 0, 0, 0.8).toColor(), + colorHighlightBackground: const Color(0xfffcef9f), colorLink: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor(), colorMathBlockBorder: const HSLColor.fromAHSL(0.15, 240, 0.8, 0.5).toColor(), colorMessageMediaContainerBackground: const Color.fromRGBO(0, 0, 0, 0.03), @@ -82,6 +83,7 @@ class ContentTheme extends ThemeExtension { colorDirectMentionBackground: const HSLColor.fromAHSL(0.25, 240, 0.52, 0.6).toColor(), colorGlobalTimeBackground: const HSLColor.fromAHSL(0.2, 0, 0, 0).toColor(), colorGlobalTimeBorder: const HSLColor.fromAHSL(0.4, 0, 0, 0).toColor(), + colorHighlightBackground: const Color(0xffffe757).withValues(alpha: 0.35), colorLink: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor(), // the same as light in Web colorMathBlockBorder: const HSLColor.fromAHSL(1, 240, 0.4, 0.4).toColor(), colorMessageMediaContainerBackground: const HSLColor.fromAHSL(0.03, 0, 0, 1).toColor(), @@ -115,6 +117,7 @@ class ContentTheme extends ThemeExtension { required this.colorDirectMentionBackground, required this.colorGlobalTimeBackground, required this.colorGlobalTimeBorder, + required this.colorHighlightBackground, required this.colorLink, required this.colorMathBlockBorder, required this.colorMessageMediaContainerBackground, @@ -148,6 +151,10 @@ class ContentTheme extends ThemeExtension { final Color colorDirectMentionBackground; final Color colorGlobalTimeBackground; final Color colorGlobalTimeBorder; + + // From Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=10904-102278&m=dev + final Color colorHighlightBackground; + final Color colorLink; final Color colorMathBlockBorder; // TODO(#46) this won't be needed final Color colorMessageMediaContainerBackground; @@ -209,6 +216,7 @@ class ContentTheme extends ThemeExtension { Color? colorDirectMentionBackground, Color? colorGlobalTimeBackground, Color? colorGlobalTimeBorder, + Color? colorHighlightBackground, Color? colorLink, Color? colorMathBlockBorder, Color? colorMessageMediaContainerBackground, @@ -232,6 +240,7 @@ class ContentTheme extends ThemeExtension { colorDirectMentionBackground: colorDirectMentionBackground ?? this.colorDirectMentionBackground, colorGlobalTimeBackground: colorGlobalTimeBackground ?? this.colorGlobalTimeBackground, colorGlobalTimeBorder: colorGlobalTimeBorder ?? this.colorGlobalTimeBorder, + colorHighlightBackground: colorHighlightBackground ?? this.colorHighlightBackground, colorLink: colorLink ?? this.colorLink, colorMathBlockBorder: colorMathBlockBorder ?? this.colorMathBlockBorder, colorMessageMediaContainerBackground: colorMessageMediaContainerBackground ?? this.colorMessageMediaContainerBackground, @@ -262,6 +271,7 @@ class ContentTheme extends ThemeExtension { colorDirectMentionBackground: Color.lerp(colorDirectMentionBackground, other.colorDirectMentionBackground, t)!, colorGlobalTimeBackground: Color.lerp(colorGlobalTimeBackground, other.colorGlobalTimeBackground, t)!, colorGlobalTimeBorder: Color.lerp(colorGlobalTimeBorder, other.colorGlobalTimeBorder, t)!, + colorHighlightBackground: Color.lerp(colorHighlightBackground, other.colorHighlightBackground, t)!, colorLink: Color.lerp(colorLink, other.colorLink, t)!, colorMathBlockBorder: Color.lerp(colorMathBlockBorder, other.colorMathBlockBorder, t)!, colorMessageMediaContainerBackground: Color.lerp(colorMessageMediaContainerBackground, other.colorMessageMediaContainerBackground, t)!, @@ -1278,6 +1288,10 @@ class _InlineContentBuilder { return WidgetSpan(alignment: PlaceholderAlignment.middle, child: GlobalTime(node: node, ambientTextStyle: widget.style)); + case HighlightNode(): + return _buildNodes(node.nodes, + style: TextStyle(backgroundColor: ContentTheme.of(_context!).colorHighlightBackground)); + case UnimplementedInlineContentNode(): return _errorUnimplemented(node, context: _context!); } diff --git a/test/model/content_test.dart b/test/model/content_test.dart index f9b461e17c..cf3f1e7ade 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -261,6 +261,13 @@ class ContentExample { GlobalTimeNode( datetime: DateTime.parse("2024-03-07T23:00:00Z"))); + static final highlight = ContentExample.inline( + 'highlight (for search)', + null, // keyword highlighting is done by the server; no Markdown representation + expectedText: 'keyword', + '

keyword

', + const HighlightNode(nodes: [TextNode('keyword')])); + static final messageLink = ContentExample.inline( 'message link', '#**api design>notation for near links@1972281**', @@ -1823,6 +1830,8 @@ void main() async { ); }); + testParseExample(ContentExample.highlight); + // // Block content. // diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 9ed263fce9..9633bfff4f 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -1167,6 +1167,8 @@ void main() { }); }); + testContentSmoke(ContentExample.highlight); + group('InlineAudio', () { Future prepare(WidgetTester tester, String html) async { await prepareContent(tester, plainContent(html),