diff --git a/mobile/lib/features/channels/channel_detail_page/message_bubble.dart b/mobile/lib/features/channels/channel_detail_page/message_bubble.dart index 1fe305fdff..34dc1f7724 100644 --- a/mobile/lib/features/channels/channel_detail_page/message_bubble.dart +++ b/mobile/lib/features/channels/channel_detail_page/message_bubble.dart @@ -338,21 +338,6 @@ class _MessageBubble extends HookConsumerWidget { ), ), ), - if (message.edited) ...[ - const SizedBox(width: Grid.half), - Text( - '(edited)', - style: context - .textTheme - .labelSmall - ?.copyWith( - color: context - .colors - .onSurfaceVariant, - fontStyle: FontStyle.italic, - ), - ), - ], ], ), ), @@ -364,6 +349,7 @@ class _MessageBubble extends HookConsumerWidget { child: MessageContent( key: swipeContentKey, content: message.content, + edited: message.edited, bubble: true, hasReactions: message.reactions.isNotEmpty, diff --git a/mobile/lib/features/channels/message_content.dart b/mobile/lib/features/channels/message_content.dart index 4542e17d4d..7b232f68e0 100644 --- a/mobile/lib/features/channels/message_content.dart +++ b/mobile/lib/features/channels/message_content.dart @@ -46,6 +46,7 @@ import 'message_content/markdown_escapes.dart'; import 'message_content/preview_text.dart'; import 'message_media.dart'; import 'message_content/bubble_segments.dart'; +import 'message_content/edited_marker.dart'; import 'message_content/buzz_link_label.dart'; import 'message_content/channel_token_pattern.dart'; import 'message_mention_pill.dart'; @@ -178,6 +179,10 @@ class MessageContent extends HookConsumerWidget { /// it contains. final bool scaleEmojiOnly; + /// Marks the body as replaced by an edit, after its last line: inline after + /// prose, else on its own line after a closing block or trailing media. + final bool edited; + /// Allows a multi-image carousel to reclaim leading space reserved by the /// surrounding message layout, while keeping its image count aligned with /// the message body. @@ -210,6 +215,7 @@ class MessageContent extends HookConsumerWidget { this.textAlign, this.maxLines, this.scaleEmojiOnly = false, + this.edited = false, this.mediaCarouselLeadingOverflow = 0, this.mediaCarouselTrailingOverflow = 0, }); @@ -401,6 +407,13 @@ class MessageContent extends HookConsumerWidget { onMentionTap: onMentionTap, onChannelTap: resolvedChannelTap, ); + final editedStyle = editedMarkerStyle(context, own: bubble && ownMessage); + // An authored slot is drawn as written; the marker takes its own line. + final markerSlot = edited && !finalContent.contains(editedSlot); + final components = useMemoized( + () => [if (markerSlot) EditedMarkerMd(editedStyle), ...inlineComponents], + [inlineComponents, editedStyle, markerSlot], + ); Widget renderMarkdown(String text) => KeyedSubtree( key: ValueKey( @@ -456,12 +469,22 @@ class MessageContent extends HookConsumerWidget { }, textAlign: textAlign ?? (bubble ? TextAlign.start : null), maxLines: maxLines, - inlineComponents: inlineComponents, + inlineComponents: components, ), ); final segments = bubble ? messageBubbleSegments(finalContent, imetaByUrl) : const <({String text, bool attachment})>[]; + // Media that closes the body takes the marker after it instead. + final editedInBody = + edited && + trailingGallery == null && + (bubble + ? segments.isNotEmpty && !segments.last.attachment + : finalContent.trim().isNotEmpty); + Widget renderBody(String text, {required bool last}) => editedInBody && last + ? markedEditedBody(text, renderMarkdown, editedStyle, slot: markerSlot) + : renderMarkdown(text); final markdown = bubble ? Column( crossAxisAlignment: ownMessage @@ -525,7 +548,10 @@ class MessageContent extends HookConsumerWidget { ), child: GptMarkdownTheme( gptThemeData: bubbleMarkdownTheme, - child: renderMarkdown(segment.text), + child: renderBody( + segment.text, + last: index == segments.length - 1, + ), ), ), ), @@ -535,8 +561,12 @@ class MessageContent extends HookConsumerWidget { ), ], ) - : renderMarkdown(finalContent); - if (trailingGallery == null) return markdown; + : renderBody(finalContent, last: true); + if (trailingGallery == null) { + return edited && !editedInBody + ? withEditedMarkerBelow(markdown, end: ownMessage) + : markdown; + } return Column( crossAxisAlignment: ownMessage @@ -590,6 +620,7 @@ class MessageContent extends HookConsumerWidget { ), ), ), + if (edited) const EditedMarkerBelow(), ], ); } diff --git a/mobile/lib/features/channels/message_content/edited_marker.dart b/mobile/lib/features/channels/message_content/edited_marker.dart new file mode 100644 index 0000000000..bb99b38100 --- /dev/null +++ b/mobile/lib/features/channels/message_content/edited_marker.dart @@ -0,0 +1,144 @@ +import 'package:flutter/material.dart'; +import 'package:gpt_markdown/gpt_markdown.dart'; + +import '../../../shared/theme/theme.dart'; + +/// Marks where an edited body draws its "(edited)" marker. A private-use +/// character. A body that already has one is drawn as written, and its +/// marker takes its own line. +const editedSlot = ''; + +/// The visible text of the edited marker. +const editedMarkerText = '(edited)'; + +// Last lines that a following " (edited)" would join or turn into Markdown +// structure: indented code, a table row, a rule or setext underline, `$$` +// display math, or a bare list, quote, checkbox or radio marker. A heading +// takes its own line as on desktop. Escapes are already shielded here, so +// `\]` is text, not math. Each pattern starts at the line start or at a +// fixed character, so no run of text is rescanned and a match stays linear in +// the line, whatever an author puts in it. +final _block = RegExp( + r'^(?:\s{4}|\t)|^\s*\||\|\s*$|\$\$\s*$' + r'|^\s*(?:([-*_])(?:\s*\1){2,}|=+)\s*$' + r'|^\s*#{1,6}(?:\s|$)' + r'|^\s*(?:[-*+]|>|\d+\.|\[[ xX]\]|\([ xX]\))\s*$', +); + +/// [markdown] with the edited slot joined to its last line, or null when the +/// marker must take its own line instead. It joins prose (a paragraph, list +/// item or quote). Bodies with a code fence anywhere never get the +/// slot: gpt_markdown's fence rules differ from CommonMark, so the slot could +/// land inside code or open a new block. Bodies with an authored slot never +/// get one either, so that character is not drawn as the marker. +String? withEditedSlot(String markdown) { + final body = markdown.trimRight(); + if (body.isEmpty || body.contains('```') || body.contains(editedSlot)) { + return null; + } + final last = body.substring(body.lastIndexOf('\n') + 1); + if (_block.hasMatch(last) || _endsInLink(last)) return null; + return '$body $editedSlot'; +} + +/// Whether [line] ends in a URL or a Markdown link. Links render as widget +/// spans, so screen readers would read a joined marker before the link. A +/// scan, not a regular expression: an unanchored one restarts at every +/// character and takes quadratic time on a long authored line. +bool _endsInLink(String line) { + final text = line.trimRight(); + // A link: `](`, then no `)` up to the final one. + if (text.length > 2 && text.endsWith(')')) { + final previous = text.lastIndexOf(')', text.length - 2); + if (text.substring(previous + 1, text.length - 1).contains('](')) { + return true; + } + } + // A URL: `://` in the last word, after a scheme that has a letter. + final word = text.substring(text.lastIndexOf(_space) + 1); + for (var i = word.indexOf('://'); i > 0; i = word.indexOf('://', i + 1)) { + for (var j = i - 1; j >= 0 && _schemeChar.hasMatch(word[j]); j--) { + if (_letter.hasMatch(word[j])) return true; + } + } + return false; +} + +final _space = RegExp(r'\s'); +final _schemeChar = RegExp(r'[a-zA-Z0-9+.-]'); +final _letter = RegExp(r'[a-zA-Z]'); + +/// Metadata after an edited body: the accent ink on your own bubble, else +/// the muted metadata colour. +TextStyle editedMarkerStyle(BuildContext context, {required bool own}) => + // Pin the traits a heading or an open would pass down to the slot. + messageTimestampTextStyle.copyWith( + fontStyle: FontStyle.normal, + decoration: TextDecoration.none, + color: own + ? ChatColors.of(context).foreground + : context.colors.onSurfaceVariant, + ); + +/// [markdown] drawn by [render] with the edited marker after its last line: +/// joined to prose, else on its own line inside the same text bubble. [slot] +/// says whether the message's renderer draws [editedSlot] as the marker; it +/// does not when any part of the message has an authored slot. +Widget markedEditedBody( + String markdown, + Widget Function(String) render, + TextStyle style, { + required bool slot, +}) { + final slotted = slot ? withEditedSlot(markdown) : null; + if (slotted != null) return render(slotted); + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + render(markdown), + Text(editedMarkerText, style: style), + ], + ); +} + +/// [child], then the edited marker below it, outside every text bubble. +Widget withEditedMarkerBelow(Widget child, {required bool end}) => Column( + crossAxisAlignment: end ? CrossAxisAlignment.end : CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [child, const EditedMarkerBelow()], +); + +/// The edited marker after trailing media, outside every text bubble. +class EditedMarkerBelow extends StatelessWidget { + /// Creates the marker. + const EditedMarkerBelow({super.key}); + + @override + Widget build(BuildContext context) => Padding( + padding: const EdgeInsets.symmetric(horizontal: Grid.xs), + child: Text( + editedMarkerText, + style: editedMarkerStyle(context, own: false), + ), + ); +} + +/// Draws [editedSlot] inside Markdown as the edited marker. +class EditedMarkerMd extends InlineMd { + /// The marker's text style. + final TextStyle style; + + /// Creates the component with its resolved [style]. + EditedMarkerMd(this.style); + + @override + RegExp get exp => RegExp(editedSlot); + + @override + InlineSpan span( + BuildContext context, + String text, + GptMarkdownConfig config, + ) => TextSpan(text: editedMarkerText, style: style); +} diff --git a/mobile/lib/features/channels/thread_detail_page/thread_message.dart b/mobile/lib/features/channels/thread_detail_page/thread_message.dart index cbebc7b5ad..0eff48db94 100644 --- a/mobile/lib/features/channels/thread_detail_page/thread_message.dart +++ b/mobile/lib/features/channels/thread_detail_page/thread_message.dart @@ -305,28 +305,13 @@ class _ThreadMessage extends HookConsumerWidget { ), ), ), - if (message.edited) ...[ - const SizedBox(width: Grid.half), - Text( - '(edited)', - style: context - .textTheme - .labelSmall - ?.copyWith( - color: context - .colors - .onSurfaceVariant, - fontStyle: - FontStyle.italic, - ), - ), - ], ], ), ), MessageContent( key: swipeContentKey, content: message.content, + edited: message.edited, bubble: true, hasReactions: message.reactions.isNotEmpty, diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 51fc26e75b..7757af969f 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -12694,37 +12694,104 @@ void main() { }); group('Edits', () { - testWidgets('edited message shows updated content and (edited) label', ( + testWidgets('edited messages show updated content and (edited) label', ( tester, ) async { final messages = [ _textMsg( - id: 'msg1', + id: threadIdRoot, pubkey: 'alice', content: 'Original text', createdAt: 1000, ), + // A grouped follow-up draws no author header, so the marker must + // follow the body, not the header. + _textMsg( + id: 'msg2', + pubkey: 'alice', + content: 'Draft', + createdAt: 1010, + ), + _textMsg( + id: 'msg3', + pubkey: 'alice', + content: 'Plain', + createdAt: 1020, + ), _edit( id: 'edit1', - targetId: 'msg1', + targetId: threadIdRoot, content: 'Edited text', createdAt: 1100, ), + _edit(id: 'edit2', targetId: 'msg2', content: 'Final', createdAt: 1110), + _textMsg( + id: 'reply1', + pubkey: 'bob', + content: 'Typo', + createdAt: 1030, + extraTags: const [ + ['e', threadIdRoot, '', 'reply'], + ], + ), + _edit( + id: 'edit3', + targetId: 'reply1', + content: 'Fixed', + createdAt: 1120, + ), ]; + final clipboardTexts = []; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(SystemChannels.platform, (call) async { + if (call.method == 'Clipboard.setData') { + clipboardTexts.add((call.arguments as Map)['text'] as String); + } + return null; + }); + addTearDown( + () => TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(SystemChannels.platform, null), + ); await tester.pumpWidget( _buildTestable( messages: messages, users: { 'alice': const UserProfile(pubkey: 'alice', displayName: 'Alice'), + 'bob': const UserProfile(pubkey: 'bob', displayName: 'Bob'), }, ), ); await tester.pumpAndSettle(); - expect(findRichText('Edited text'), findsOneWidget); + expect(findRichText('Edited text (edited)'), findsOneWidget); expect(findRichText('Original text'), findsNothing); - expect(find.text('(edited)'), findsOneWidget); + expect(findRichText('Final (edited)'), findsOneWidget); + expect(findRichText('Plain'), findsOneWidget); + expect(findRichText('Plain (edited)'), findsNothing); + + // Copy takes the edited text, never the marker. + await tester.longPress(findRichText('Edited text (edited)')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Copy text')); + await tester.pumpAndSettle(); + expect(clipboardTexts, ['Edited text']); + + // The thread marks its edited head and its edited reply. + await tester.tap( + find.byKey(const ValueKey('thread-summary-$threadIdRoot')), + ); + await tester.pumpAndSettle(); + expect(find.byType(ThreadDetailPage), findsOneWidget); + final thread = find.byType(ThreadDetailPage); + for (final text in ['Edited text (edited)', 'Fixed (edited)']) { + expect( + find.descendant(of: thread, matching: findRichText(text)), + findsOneWidget, + reason: text, + ); + } }); testWidgets('latest edit wins when multiple edits exist', (tester) async { diff --git a/mobile/test/features/channels/message_content/edited_marker_test.dart b/mobile/test/features/channels/message_content/edited_marker_test.dart new file mode 100644 index 0000000000..cd36897a36 --- /dev/null +++ b/mobile/test/features/channels/message_content/edited_marker_test.dart @@ -0,0 +1,92 @@ +import 'package:buzz/features/channels/message_content/edited_marker.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test( + 'the edited slot joins prose and nothing gpt_markdown could restyle', + () { + for (final (markdown, expected) in [ + ('Hi how are ya.', 'Hi how are ya. $editedSlot'), + ('Trailing space \n', 'Trailing space $editedSlot'), + ('- one\n- two', '- one\n- two $editedSlot'), + ('> quoted', '> quoted $editedSlot'), + ('# Title\nbody', '# Title\nbody $editedSlot'), + ('Visit https://x.io today', 'Visit https://x.io today $editedSlot'), + ('#######', '####### $editedSlot'), + ('wavy\n~~~', 'wavy\n~~~ $editedSlot'), + ('ok\n--', 'ok\n-- $editedSlot'), + ('a | b', 'a | b $editedSlot'), + ('Read [docs](x) now', 'Read [docs](x) now $editedSlot'), + ('done :)', 'done :) $editedSlot'), + (')', ') $editedSlot'), + ('see ://x', 'see ://x $editedSlot'), + ('see 1://x', 'see 1://x $editedSlot'), + ]) { + expect(withEditedSlot(markdown), expected, reason: markdown); + } + for (final markdown in [ + '', + '```npm install```', + 'Run this:\n```npm install```', + 'Intro\n\n```\ncode\n```\nafter', + '````\ncode', + ' indented code', + 'hi\n\tcode', + '| a | b |\n| - | - |\n| 1 | 2 |', + 'a or b |', + 'Above\n\n---', + 'Above\n\n* * *', + 'Title\n===', + r'$$x^2$$', + for (final marker in [ + '-', + '*', + '+', + '#', + '###', + '>', + '1.', + '[ ]', + '[x]', + ]) + 'ok\n$marker', + 'pick\n( )', + '# Heading', + '### Notes ', + 'ok\n\nhttps://example.com/file.pdf', + 'see https://example.com.', + 'Read [the docs](https://x.io)', + 'Read [the docs](https://x.io) ', + '[a](b) [c](d)', + 'go to(https://x.io', + 'see x+1.y-z://host', + 'buzz://message?channel=c&id=e', + 'kept $editedSlot as written', + ]) { + expect(withEditedSlot(markdown), isNull, reason: markdown); + } + }, + ); + + test('a long authored last line takes linear time', () { + // Each body defeats an unanchored pattern at every start position, which + // took seconds at 40,000 characters and grows with the square. + const n = 256 * 1024; + for (final markdown in [ + '${'a' * n}:// x y', + '${'](' * (n ~/ 2)}x y', + '${'a:' * (n ~/ 2)}//', + '${'|' * n} x', + '${r'$' * n} x', + '${'- ' * (n ~/ 2)}x', + ]) { + final timer = Stopwatch()..start(); + withEditedSlot(markdown); + expect( + timer.elapsedMilliseconds, + lessThan(500), + reason: markdown.substring(0, 8), + ); + } + }); +} diff --git a/mobile/test/features/channels/message_content_bubble_cases.dart b/mobile/test/features/channels/message_content_bubble_cases.dart index 601b27207a..d43dae6a86 100644 --- a/mobile/test/features/channels/message_content_bubble_cases.dart +++ b/mobile/test/features/channels/message_content_bubble_cases.dart @@ -318,3 +318,127 @@ void bubbleContentTests() { expect(tester.takeException(), isNull); }); } + +void editedMarkerTests() { + Iterable plainTexts(WidgetTester tester) => tester + .widgetList(find.byType(RichText)) + .map((text) => text.text.toPlainText()); + TextSpan? markerSpan(WidgetTester tester) { + TextSpan? found; + for (final text in tester.widgetList(find.byType(RichText))) { + text.text.visitChildren((span) { + if (span is TextSpan && span.text == '(edited)') found = span; + return found == null; + }); + } + return found; + } + + testWidgets('an edit is marked after the body, never inside other text', ( + tester, + ) async { + Future show(String content, {bool edited = true, bool own = false}) => + tester + .pumpWidget( + _testable( + MessageContent( + content: content, + bubble: true, + edited: edited, + ownMessage: own, + ), + ), + ) + .then((_) => tester.pumpAndSettle()); + + // Prose: inline after the last line, in the muted metadata colour. + await show('First\n\nHi how are ya.'); + expect( + plainTexts(tester), + anyElement(endsWith('\nHi how are ya. (edited)')), + ); + final context = tester.element(find.byType(MessageContent)); + expect(markerSpan(tester)?.style?.color, context.colors.onSurfaceVariant); + + // Your own accent bubble draws it in the bubble's ink. + await show('Mine', own: true); + expect(plainTexts(tester), contains('Mine (edited)')); + expect( + markerSpan(tester)?.style?.color, + ChatColors.of(tester.element(find.byType(MessageContent))).foreground, + ); + + // Bodies the slot could restyle render exactly as unedited, plus the + // marker on its own line inside the same text bubble. + for (final content in [ + '```npm install```', + 'Run this:\n```npm install```', + 'Intro\n\n```\ncode\n```', + '````\ncode\n````', + '```\nunclosed', + '| a | b |\n| - | - |\n| 1 | 2 |', + 'Above\n\n---', + 'ok\n-', + 'ok\n#', + 'todo\n[x]', + '# Heading', + 'ok\n\nhttps://example.com/file.pdf', + ]) { + await show(content, edited: false); + final unedited = plainTexts(tester).toList(); + await show(content); + expect(plainTexts(tester), [...unedited, '(edited)'], reason: content); + expect( + find.descendant( + of: find.byKey(const ValueKey('message-text-bubble-0')), + matching: find.text('(edited)'), + ), + findsOneWidget, + reason: content, + ); + } + + // An escaped bracket is text, not display math, so the marker joins it. + await show(r'see \[x\]'); + expect(plainTexts(tester), contains('see [x] (edited)')); + + // Inside an unclosed or a heading the marker keeps its own style. + await show('see this'); + expect(markerSpan(tester)?.style?.decoration, TextDecoration.none); + + // Trailing media: the marker follows it, outside every text bubble. + await show('![](https://example.com/a.png)'); + expect(find.text('(edited)'), findsOneWidget); + expect( + find.descendant( + of: find.byWidgetPredicate( + (widget) => + widget.key is ValueKey && + (widget.key! as ValueKey).value.startsWith( + 'message-text-bubble-', + ), + ), + matching: find.text('(edited)'), + ), + findsNothing, + ); + + // Unedited bodies stay unmarked. An authored slot character is drawn as + // written, and only an edit adds the marker, on its own line. + bool shows(String text) => tester + .widgetList(find.byType(RichText)) + .any((rich) => rich.text.toPlainText().contains(text)); + await show('Plain', edited: false); + expect(markerSpan(tester), isNull); + await show('Sneaky $editedSlot', edited: false); + expect(shows('Sneaky $editedSlot'), isTrue); + expect(markerSpan(tester), isNull); + await show('Sneaky $editedSlot'); + expect(shows('Sneaky $editedSlot'), isTrue); + expect(find.text('(edited)'), findsOneWidget); + // Also when the authored slot is in an earlier part of the message. + await show('Sneaky $editedSlot\n![](https://example.com/a.png)\nafter'); + expect(shows('Sneaky $editedSlot'), isTrue); + expect(find.text('(edited)'), findsOneWidget); + }); +} diff --git a/mobile/test/features/channels/message_content_test.dart b/mobile/test/features/channels/message_content_test.dart index b29be5732b..67d6626a10 100644 --- a/mobile/test/features/channels/message_content_test.dart +++ b/mobile/test/features/channels/message_content_test.dart @@ -26,6 +26,7 @@ import 'package:buzz/features/channels/channel.dart'; import 'package:buzz/features/channels/channels_provider.dart'; import 'package:buzz/features/channels/message_content.dart'; import 'package:buzz/features/channels/message_long_press_region.dart'; +import 'package:buzz/features/channels/message_content/edited_marker.dart'; import 'package:buzz/features/channels/media_viewer_page.dart'; import 'package:buzz/features/channels/voice_note_attachment.dart'; import 'package:buzz/features/channels/voice_note_waveform.dart'; @@ -463,6 +464,7 @@ void main() { carouselPreviewTests(); stackJourneyTests(); bubbleContentTests(); + editedMarkerTests(); test('wide voice-note waveforms distribute bars across their full width', () { const width = 320.0; const sampleCount = 48; diff --git a/mobile/test/features/channels/thread_reply_refresh_cases.dart b/mobile/test/features/channels/thread_reply_refresh_cases.dart index bd5c747a25..f57fbac693 100644 --- a/mobile/test/features/channels/thread_reply_refresh_cases.dart +++ b/mobile/test/features/channels/thread_reply_refresh_cases.dart @@ -221,17 +221,16 @@ void threadReplyRefreshTests() { await tester.pumpAndSettle(); final thread = find.byType(ThreadDetailPage); expect( - find.descendant(of: thread, matching: find.text('Fixed reply')), + find.descendant( + of: thread, + matching: findRichText('Fixed reply (edited)'), + ), findsOneWidget, ); expect( - find.descendant(of: thread, matching: find.text('Cached reply')), + find.descendant(of: thread, matching: findRichText('Cached reply')), findsNothing, ); - expect( - find.descendant(of: thread, matching: find.text('(edited)')), - findsOneWidget, - ); }); }