mirror of
https://github.com/block/buzz.git
synced 2026-11-13 21:01:53 +00:00
fix(mobile): never show emoji shortcodes in reaction pills (#9411)
## 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. <img width="300" alt="reaction-pills-before" src="https://raw.githubusercontent.com/block/buzz/75b22a00d92df2db389ef90d35d476b285b60374/pr-9411--reaction-pills-before.png"> <img width="300" alt="reaction-pills-after" src="https://raw.githubusercontent.com/block/buzz/75b22a00d92df2db389ef90d35d476b285b60374/pr-9411--reaction-pills-after.png"> Reaction sheet after: <img width="300" alt="reaction-sheet-after" src="https://raw.githubusercontent.com/block/buzz/75b22a00d92df2db389ef90d35d476b285b60374/pr-9411--reaction-sheet-after.png"> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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": [
|
||||
|
||||
@@ -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<CustomEmoji> 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<bool> _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<bool> _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<bool> _showNativeReactionDetails(
|
||||
timer?.cancel();
|
||||
subscription.close();
|
||||
namesSubscription.close();
|
||||
paletteSubscription.close();
|
||||
}
|
||||
});
|
||||
if (addRequested && context.mounted) onAddReaction?.call();
|
||||
|
||||
@@ -49,13 +49,36 @@ final customEmojiBytesCacheProvider = Provider<CustomEmojiBytesCache>((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),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<List<TimelineReaction>>([
|
||||
|
||||
@@ -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<Override> 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<CustomEmojiImage>(approve).url, approveUrl);
|
||||
await tester.runAsync(
|
||||
() => precacheImage(
|
||||
tester
|
||||
.widget<Image>(
|
||||
find.descendant(of: approve, matching: find.byType(Image)),
|
||||
)
|
||||
.image,
|
||||
tester.element(approve),
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester
|
||||
.widget<RawImage>(
|
||||
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<Object?, Object?>? payload;
|
||||
final completion = Completer<Map<String, Object?>>();
|
||||
tester.binding.defaultBinaryMessenger.setMockMethodCallHandler(
|
||||
NativeMessagePresentation.channel,
|
||||
(call) async {
|
||||
if (call.method == 'reactions') {
|
||||
payload = call.arguments as Map<Object?, Object?>;
|
||||
return completion.future;
|
||||
}
|
||||
return <String, Object?>{};
|
||||
},
|
||||
);
|
||||
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(<String, Object?>{});
|
||||
await tester.pumpAndSettle();
|
||||
debugDefaultTargetPlatformOverride = null;
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user