From 4a101752fbed03420252888bfecc40eff0a78b84 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Thu, 12 Nov 2026 13:38:37 -0800 Subject: [PATCH] fix(mobile): never show emoji shortcodes in reaction pills (#9411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context A reaction slot should never show an emoji shortcode as text. Today it does in two cases: - An agent's reaction is a kind 7 event with content `:shortcode:` and no NIP-30 `emoji` tag, so the pill printed `:approve:` in tiny text instead of the community emoji. - A custom emoji whose image fails to load fell back to the shortcode text. ## Behavior - A tagless `:shortcode:` reaction resolves against the community custom-emoji palette and renders that image. An `emoji` tag on the event still wins. - A shortcode with no palette entry, or whose image fails to load, shows a neutral placeholder glyph (the muted sparkles icon) instead of text. The shortcode survives only as the semantics label. - This applies to every reaction slot: the timeline pills, the combined reaction tray, the Flutter reaction detail sheet, and the native iOS reactions sheet (which is told which shortcodes have no image, so `NativeMessageGlyph` draws the placeholder). System-message rows reuse the same reaction row. - Unicode emoji are unchanged. The icon is the existing `BuzzIcons.sparkles`. Pareto note: the palette lookup is the shared `reactionEmojiUrl` helper, so there is no new resolver, and the pill only watches the palette when the reaction has no tag. ### Validation - `just mobile-check mobile-test`: format and analyze clean; full suite passes (4773 passed, 74 skipped). - New widget tests (journeys, in `reaction_row_test/shortcode_tests.dart`): a tagless known shortcode renders its image; an unknown shortcode and a failed image render the placeholder and never the text; unicode is unchanged; the native sheet payload marks image-less shortcodes. Reverting the fix makes both new journeys fail. - iOS: added a Swift unit test for the native glyph placeholder (`NativeMessageImageLoaderTests`); not run locally, the app run below exercises the same code. - Automated app run on a cloned iOS simulator with a fixture entry point (mocked media, synthetic palette): confirmed the pills before and after, and opened the reaction sheet from a long-press to confirm the placeholder renders there too. The clone was deleted afterwards. ## Before / after Captures use synthetic shortcodes and a synthetic palette. reaction-pills-before reaction-pills-after Reaction sheet after: reaction-sheet-after 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tom Brow Co-authored-by: Tom Brow Co-authored-by: Claude Opus 5.5 Co-authored-by: studio --- mobile/ios/Runner/NativeReactionDetails.swift | 13 +- .../NativeMessageImageLoaderTests.swift | 7 + .../lib/features/channels/reaction_row.dart | 72 ++++++-- .../custom_emoji/custom_emoji_render.dart | 32 +++- .../features/channels/reaction_row_test.dart | 5 + .../reaction_row_test/shortcode_tests.dart | 166 ++++++++++++++++++ 6 files changed, 270 insertions(+), 25 deletions(-) create mode 100644 mobile/test/features/channels/reaction_row_test/shortcode_tests.dart diff --git a/mobile/ios/Runner/NativeReactionDetails.swift b/mobile/ios/Runner/NativeReactionDetails.swift index 673cb9e15b..cf052327b6 100644 --- a/mobile/ios/Runner/NativeReactionDetails.swift +++ b/mobile/ios/Runner/NativeReactionDetails.swift @@ -359,10 +359,11 @@ private final class NativeReactionCloseButton: UIButton { /// Short-lived image loader. Auth is supplied for the specific URL by Buzz's /// existing media auth service; redirects are refused to avoid forwarding it. /// Encoded `bytes` Flutter already downloaded are shown without a fetch. A -/// custom emoji ([isReaction] with a `url`) never shows its `:shortcode:` text, -/// which is illegible at this size: it stays blank while loading and shows -/// sparkles, like the native emoji picker, if the image cannot be loaded. An -/// avatar keeps its initial behind the image. +/// custom emoji ([isReaction] with a `url`, or flagged `isShortcode`) never +/// shows its `:shortcode:` text, which is illegible at this size: it stays blank +/// while loading and shows sparkles, like the native emoji picker, if there is +/// no image or it cannot be loaded. An avatar keeps its initial behind the +/// image. final class NativeMessageGlyph: UIView { private var cancelLoad: (() -> Void)? private let label = UILabel() @@ -371,7 +372,7 @@ final class NativeMessageGlyph: UIView { init(data: [String: Any], size: CGFloat, isReaction: Bool = false) { super.init(frame: .zero) let raw = data["url"] as? String - let customEmoji = isReaction && raw != nil + let customEmoji = isReaction && (raw != nil || data["isShortcode"] as? Bool == true) label.text = customEmoji ? nil : data["emoji"] as? String label.textAlignment = .center label.font = .systemFont(ofSize: size) @@ -388,13 +389,13 @@ final class NativeMessageGlyph: UIView { child.bottomAnchor.constraint(equalTo: bottomAnchor), ]) } - guard let raw else { return } let fallback = { [weak self] in guard let self, customEmoji else { return } image.image = NativeTabler.image("sparkles", size: size) image.tintColor = .secondaryLabel image.contentMode = .center } + guard let raw else { fallback(); return } if let bytes = data["bytes"] as? FlutterStandardTypedData { DispatchQueue.global(qos: .userInitiated).async { [weak self] in let decoded = NativeMessageImageLoader.decode(bytes.data) diff --git a/mobile/ios/RunnerTests/NativeMessageImageLoaderTests.swift b/mobile/ios/RunnerTests/NativeMessageImageLoaderTests.swift index 500c011ac9..95de80e2d9 100644 --- a/mobile/ios/RunnerTests/NativeMessageImageLoaderTests.swift +++ b/mobile/ios/RunnerTests/NativeMessageImageLoaderTests.swift @@ -128,6 +128,13 @@ final class NativeReactionDetailsTests: XCTestCase { XCTAssertEqual(updatedRow.arrangedSubviews[0].subviews.compactMap { $0 as? UILabel }.first?.text, "🎉") } + @MainActor func testShortcodeReactionWithoutImageNeverShowsText() { + let glyph = NativeMessageGlyph( + data: ["emoji": ":ghost:", "isShortcode": true], size: 26, isReaction: true) + XCTAssertNil(glyph.subviews.compactMap { $0 as? UILabel }.first?.text) + XCTAssertNotNil(glyph.subviews.compactMap { $0 as? UIImageView }.first?.image) + } + @MainActor func testTogglesListEveryReactorAndRelabelUpdatedProfiles() { let controller = NativeReactionDetailsViewController(data: [ "reactions": [ diff --git a/mobile/lib/features/channels/reaction_row.dart b/mobile/lib/features/channels/reaction_row.dart index 2f05fb8be3..e7d406f961 100644 --- a/mobile/lib/features/channels/reaction_row.dart +++ b/mobile/lib/features/channels/reaction_row.dart @@ -15,6 +15,8 @@ import '../../shared/hosted/builderlab_account.dart'; import '../../shared/hosted/hosted_relay_auth.dart'; import '../../shared/hosted/native_media_headers.dart'; import '../../shared/relay/relay_provider.dart'; +import '../../shared/custom_emoji/custom_emoji.dart'; +import '../../shared/custom_emoji/custom_emoji_provider.dart'; import '../../shared/custom_emoji/custom_emoji_render.dart'; import '../../shared/emoji/emoji_burst.dart'; import '../../shared/emoji/emoji_avatar.dart'; @@ -425,16 +427,40 @@ class _AddReactionPill extends StatelessWidget { } } -class _ReactionEmoji extends StatelessWidget { +/// A `:shortcode:` reaction body, as opposed to a unicode emoji. +bool _isShortcodeReaction(String emoji) => + emoji.length > 2 && + emoji.startsWith(':') && + emoji.endsWith(':') && + normalizeShortcode(emoji) != null; + +/// The image behind [reaction]: the NIP-30 `emoji` tag on its event, else, for +/// a tagless `:shortcode:` (an agent's reaction, say), the community palette +/// entry. Null for a unicode emoji or a shortcode nothing resolves. +String? _reactionImageUrl( + TimelineReaction reaction, + List palette, +) { + final tagged = reaction.emojiUrl; + if (tagged != null && tagged.isNotEmpty) return tagged; + if (!_isShortcodeReaction(reaction.emoji)) return null; + return reactionEmojiUrl(reaction.emoji, palette); +} + +/// A reaction slot never shows shortcode text: a custom emoji is its image, and +/// a shortcode with no image (unknown, or one that fails to load) is the +/// neutral [CustomEmojiPlaceholder]. +class _ReactionEmoji extends ConsumerWidget { final TimelineReaction reaction; final double size; const _ReactionEmoji({required this.reaction, required this.size}); @override - Widget build(BuildContext context) { - final emojiUrl = reaction.emojiUrl; - if (emojiUrl == null || emojiUrl.isEmpty) { + Widget build(BuildContext context, WidgetRef ref) { + final hasTag = reaction.emojiUrl?.isNotEmpty ?? false; + final isShortcode = _isShortcodeReaction(reaction.emoji); + if (!hasTag && !isShortcode) { return NativeEmojiGlyph( emoji: reaction.emoji, size: size, @@ -442,17 +468,27 @@ class _ReactionEmoji extends StatelessWidget { centerInOpticalBox: true, ); } - final shortcode = reaction.emoji.substring(1, reaction.emoji.length - 1); + final url = _reactionImageUrl( + reaction, + // Only a tagless shortcode needs the palette; other pills don't rebuild + // when it loads. + hasTag ? const [] : ref.watch(customEmojiListProvider), + ); + final shortcode = isShortcode + ? reaction.emoji.substring(1, reaction.emoji.length - 1) + : reaction.emoji; return SizedBox.square( dimension: size, child: FittedBox( fit: BoxFit.scaleDown, - child: CustomEmojiImage( - shortcode: shortcode, - url: emojiUrl, - size: size, - compact: true, - ), + child: url == null + ? CustomEmojiPlaceholder(shortcode: shortcode, size: size) + : CustomEmojiImage( + shortcode: shortcode, + url: url, + size: size, + compact: true, + ), ), ); } @@ -824,7 +860,13 @@ Future _showNativeReactionDetails( 'selected': reaction.reactedByCurrentUser, 'burst': isPositiveEmojiParticle(reaction.emoji), 'users': reaction.userPubkeys.toSet().toList(), - if (reaction.emojiUrl case final String url) ...{ + // `isShortcode` tells the native glyph never to print the text. + if (_isShortcodeReaction(reaction.emoji)) 'isShortcode': true, + if (_reactionImageUrl( + reaction, + container.read(customEmojiListProvider), + ) + case final String url) ...{ 'url': url, 'headers': nativeMediaHeaders(url), // The timeline usually downloaded this emoji already; reuse it so @@ -849,6 +891,11 @@ Future _showNativeReactionDetails( } reactions.addListener(updateReactions); + // A tagless shortcode shows the placeholder until the palette arrives. + final paletteSubscription = container.listen( + customEmojiListProvider, + (_, _) => updateReactions(), + ); // Native headers read the cached assertion synchronously, and that cache // reads as empty inside its refresh margin while the live socket is still // valid. Present at once with what is cached, refresh in the background, @@ -894,6 +941,7 @@ Future _showNativeReactionDetails( timer?.cancel(); subscription.close(); namesSubscription.close(); + paletteSubscription.close(); } }); if (addRequested && context.mounted) onAddReaction?.call(); diff --git a/mobile/lib/shared/custom_emoji/custom_emoji_render.dart b/mobile/lib/shared/custom_emoji/custom_emoji_render.dart index e0b3e29001..dfd572c859 100644 --- a/mobile/lib/shared/custom_emoji/custom_emoji_render.dart +++ b/mobile/lib/shared/custom_emoji/custom_emoji_render.dart @@ -49,13 +49,36 @@ final customEmojiBytesCacheProvider = Provider((ref) { return CustomEmojiBytesCache(); }); +/// The neutral glyph that stands in for a custom emoji with no image: a muted +/// sparkles icon, because a truncated `:shortcode:` is illegible at glyph size. +/// The shortcode survives only as the semantics label. +class CustomEmojiPlaceholder extends StatelessWidget { + final String shortcode; + final double size; + + const CustomEmojiPlaceholder({ + super.key, + required this.shortcode, + required this.size, + }); + + @override + Widget build(BuildContext context) => Icon( + BuzzIcons.sparkles, + size: size, + color: DefaultTextStyle.of(context).style.color?.withValues(alpha: 0.6), + semanticLabel: ':$shortcode:', + ); +} + /// A single custom-emoji image rendered as a square, network-loaded glyph. /// /// Reused everywhere a `:shortcode:` resolves to an image: inline in message /// bodies, reaction pills, status, and the picker. If the image fails to load, /// it falls back to the literal `:shortcode:` text, so a broken URL never /// leaves a blank gap in prose; a [compact] glyph, such as a reaction pill, -/// shows a sparkles icon instead, because a truncated shortcode is illegible. +/// shows the [CustomEmojiPlaceholder] instead. A reaction slot is always +/// [compact]: it never renders shortcode text. class CustomEmojiImage extends ConsumerWidget { final String shortcode; final String url; @@ -84,12 +107,7 @@ class CustomEmojiImage extends ConsumerWidget { semanticLabel: ':$shortcode:', onBytesLoaded: (bytes) => bytesCache.remember(url, bytes), errorBuilder: (_, _, _) => compact - ? Icon( - BuzzIcons.sparkles, - size: size, - color: fallbackStyle.color?.withValues(alpha: 0.6), - semanticLabel: ':$shortcode:', - ) + ? CustomEmojiPlaceholder(shortcode: shortcode, size: size) : Text(':$shortcode:', style: fallbackStyle), ); } diff --git a/mobile/test/features/channels/reaction_row_test.dart b/mobile/test/features/channels/reaction_row_test.dart index f52843c223..96fa3635d3 100644 --- a/mobile/test/features/channels/reaction_row_test.dart +++ b/mobile/test/features/channels/reaction_row_test.dart @@ -1,4 +1,6 @@ import 'package:buzz/features/channels/channel_management_provider.dart'; +import 'package:buzz/shared/custom_emoji/custom_emoji.dart'; +import 'package:buzz/shared/custom_emoji/custom_emoji_provider.dart'; import 'package:buzz/shared/custom_emoji/custom_emoji_render.dart'; import 'package:buzz/shared/emoji/native_emoji_glyph.dart'; import 'package:http/http.dart' as http; @@ -30,10 +32,12 @@ import 'package:flutter/foundation.dart'; import 'package:buzz/shared/widgets/native_message_presentation.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:hooks_riverpod/misc.dart' show Override; import '../../helpers/widget_helpers.dart'; part 'reaction_row_test/combined_tests.dart'; +part 'reaction_row_test/shortcode_tests.dart'; const _alice = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; @@ -150,6 +154,7 @@ class _RecordingReactionActions implements ChannelActions { void main() { combinedReactionTests(); + shortcodeReactionTests(); testWidgets('removing the sole reaction closes the sheet', (tester) async { final reactions = ValueNotifier>([ diff --git a/mobile/test/features/channels/reaction_row_test/shortcode_tests.dart b/mobile/test/features/channels/reaction_row_test/shortcode_tests.dart new file mode 100644 index 0000000000..95492aec78 --- /dev/null +++ b/mobile/test/features/channels/reaction_row_test/shortcode_tests.dart @@ -0,0 +1,166 @@ +part of '../reaction_row_test.dart'; + +/// A reaction slot never renders an emoji shortcode as text. A tagless +/// `:shortcode:` (an agent's reaction) resolves against the community palette; +/// one with no image, or whose image fails to load, is the neutral placeholder. +void shortcodeReactionTests() { + const approveUrl = 'https://relay.example/media/approve.png'; + const brokenUrl = 'https://relay.example/media/broken.png'; + final palette = [ + const CustomEmoji(shortcode: 'approve', url: approveUrl), + const CustomEmoji(shortcode: 'broken', url: brokenUrl), + ]; + final png = base64Decode( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==', + ); + // Tagless, as an agent sends them: no NIP-30 `emoji` tag on the event. + final reactions = [ + _reaction(emoji: ':approve:'), + _reaction(emoji: ':broken:'), + _reaction(emoji: ':ghost:'), + _reaction(emoji: _fire), + ]; + + List overrides(MockClient client) => [ + emojiDatasetOrEmptyProvider.overrideWithValue(_dataset), + customEmojiListProvider.overrideWithValue(palette), + mediaHttpClientProvider.overrideWithValue(client), + ]; + + void expectNoShortcodeText(WidgetTester tester, Finder scope) { + for (final shortcode in [':approve:', ':broken:', ':ghost:']) { + expect( + find.descendant(of: scope, matching: find.text(shortcode)), + findsNothing, + ); + } + } + + testWidgets('a reaction slot never shows a shortcode as text', ( + tester, + ) async { + final client = MockClient( + (request) async => request.url.toString() == approveUrl + ? http.Response.bytes(png, 200) + : http.Response('missing', 404), + ); + addTearDown(client.close); + await tester.pumpWidget( + WidgetHelpers.testable( + overrides: overrides(client), + child: ReactionRow( + messageId: _messageId, + channelId: 'channel', + reactions: reactions, + onToggle: (_) {}, + ), + ), + ); + await tester.pumpAndSettle(); + + // The tagless known shortcode renders the palette's image. + final approve = find.byType(CustomEmojiImage).first; + expect(tester.widget(approve).url, approveUrl); + await tester.runAsync( + () => precacheImage( + tester + .widget( + find.descendant(of: approve, matching: find.byType(Image)), + ) + .image, + tester.element(approve), + ), + ); + await tester.pumpAndSettle(); + expect( + tester + .widget( + find.descendant(of: approve, matching: find.byType(RawImage)), + ) + .image, + isNotNull, + ); + + // A failed image and an unknown shortcode are both the placeholder, and + // the unicode emoji is untouched. + final pills = find.byType(ReactionRow); + expectNoShortcodeText(tester, pills); + expect( + find.descendant(of: pills, matching: find.byIcon(BuzzIcons.sparkles)), + findsNWidgets(2), + ); + expect(find.byType(NativeEmojiGlyph), findsOneWidget); + expect(find.text(_fire), findsOneWidget); + + // The detail sheet follows the same rule. + await tester.longPress(find.byKey(const ValueKey('reaction-pill-:ghost:'))); + await tester.pumpAndSettle(); + final sheet = find.byKey(const ValueKey('reaction-details-sheet')); + expect(sheet, findsOneWidget); + expectNoShortcodeText(tester, sheet); + expect( + find.descendant(of: sheet, matching: find.byType(CustomEmojiImage)), + findsNWidgets(4), + ); + expect( + find.descendant(of: sheet, matching: find.byIcon(BuzzIcons.sparkles)), + findsNWidgets(4), + ); + expect(tester.takeException(), isNull); + }); + + testWidgets('the native sheet is told which shortcodes have no image', ( + tester, + ) async { + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + addTearDown(() => debugDefaultTargetPlatformOverride = null); + Map? payload; + final completion = Completer>(); + tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + NativeMessagePresentation.channel, + (call) async { + if (call.method == 'reactions') { + payload = call.arguments as Map; + return completion.future; + } + return {}; + }, + ); + addTearDown( + () => tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + NativeMessagePresentation.channel, + null, + ), + ); + final client = MockClient((_) async => http.Response('missing', 404)); + addTearDown(client.close); + await tester.pumpWidget( + WidgetHelpers.testable( + overrides: overrides(client), + child: ReactionRow( + messageId: _messageId, + channelId: 'channel', + reactions: reactions, + onToggle: (_) {}, + ), + ), + ); + await tester.pumpAndSettle(); + await tester.longPress(find.byKey(const ValueKey('reaction-pill-:ghost:'))); + await tester.pumpAndSettle(); + + final sent = { + for (final reaction in payload!['reactions'] as List) + (reaction as Map)['emoji']: reaction, + }; + expect(sent[':approve:']!['url'], approveUrl); + expect(sent[':approve:']!['isShortcode'], isTrue); + expect(sent[':ghost:']!.containsKey('url'), isFalse); + expect(sent[':ghost:']!['isShortcode'], isTrue); + expect(sent[_fire]!.containsKey('url'), isFalse); + expect(sent[_fire]!.containsKey('isShortcode'), isFalse); + completion.complete({}); + await tester.pumpAndSettle(); + debugDefaultTargetPlatformOverride = null; + }); +}