From 2ac288fa806c47a4e3c6dcbe319d95282737ba07 Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 17 Aug 2026 10:48:35 -0600 Subject: [PATCH] Make message action dismissal idempotent Route popover backdrops, action rows, and quick reactions through a single one-shot gate so repeated callbacks cannot pop an underlying route or submit duplicate reactions. Co-authored-by: Carl Signed-off-by: Wes --- .../message_action_popover.dart | 14 ++- .../message_actions/quick_reaction_row.dart | 28 +++++- .../message_actions/reaction_popover.dart | 16 +++- .../channels/message_actions_test.dart | 86 +++++++++++++++++++ 4 files changed, 135 insertions(+), 9 deletions(-) diff --git a/mobile/lib/features/channels/message_actions/message_action_popover.dart b/mobile/lib/features/channels/message_actions/message_action_popover.dart index 7f4dd9436..7be77b31b 100644 --- a/mobile/lib/features/channels/message_actions/message_action_popover.dart +++ b/mobile/lib/features/channels/message_actions/message_action_popover.dart @@ -522,12 +522,15 @@ class _MessageActionsPopover extends HookWidget { final mediaQuery = MediaQuery.of(context); final selectionStarted = useRef(false); - void selectAction(String actionId) { + void select(Object? result, [VoidCallback? effect]) { if (selectionStarted.value) return; selectionStarted.value = true; - Navigator.of(context).pop(actionId); + Navigator.of(context).pop(result); + effect?.call(); } + void selectAction(String actionId) => select(actionId); + return LayoutBuilder( builder: (context, constraints) { final safeLeft = mediaQuery.padding.left + Grid.xxs; @@ -656,8 +659,9 @@ class _MessageActionsPopover extends HookWidget { ), Positioned.fill( child: GestureDetector( + key: const ValueKey('message-actions-backdrop'), behavior: HitTestBehavior.opaque, - onTap: () => Navigator.of(context).pop(), + onTap: () => select(null), ), ), if (showPreview) @@ -725,6 +729,7 @@ class _MessageActionsPopover extends HookWidget { pageContext: pageContext, pageRef: pageRef, popResult: _messageActionReactionSelection, + onSelected: (result, effect) => select(result, effect), ), ), Positioned.fromRect( @@ -812,6 +817,7 @@ class _MessageReactionTray extends StatelessWidget { final BuildContext pageContext; final WidgetRef pageRef; final Object popResult; + final void Function(Object? result, VoidCallback effect) onSelected; const _MessageReactionTray({ required this.animation, @@ -820,6 +826,7 @@ class _MessageReactionTray extends StatelessWidget { required this.pageContext, required this.pageRef, required this.popResult, + required this.onSelected, }); @override @@ -833,6 +840,7 @@ class _MessageReactionTray extends StatelessWidget { pageContext: pageContext, pageRef: pageRef, popResult: popResult, + onSelected: onSelected, ); } } diff --git a/mobile/lib/features/channels/message_actions/quick_reaction_row.dart b/mobile/lib/features/channels/message_actions/quick_reaction_row.dart index 131f272c1..15a71c30f 100644 --- a/mobile/lib/features/channels/message_actions/quick_reaction_row.dart +++ b/mobile/lib/features/channels/message_actions/quick_reaction_row.dart @@ -20,6 +20,10 @@ class _QuickReactionRow extends ConsumerWidget { final Object? popResult; + /// Selects exactly one popover result and side effect. Bottom sheets leave + /// this null and retain their existing local dismissal behavior. + final void Function(Object? result, VoidCallback effect)? onSelected; + const _QuickReactionRow({ required this.message, required this.sheetContext, @@ -27,6 +31,7 @@ class _QuickReactionRow extends ConsumerWidget { required this.pageRef, this.presentationAnimation, this.popResult, + this.onSelected, }); @override @@ -76,8 +81,15 @@ class _QuickReactionRow extends ConsumerWidget { child: _QuickReactionCircle( size: circleSize, onTap: () { - Navigator.of(sheetContext).pop(popResult); - react(emoji[index]); + void effect() => react(emoji[index]); + + final select = onSelected; + if (select != null) { + select(popResult, effect); + } else { + Navigator.of(sheetContext).pop(popResult); + effect(); + } }, child: _QuickReactionGlyph( value: emoji[index], @@ -92,8 +104,16 @@ class _QuickReactionRow extends ConsumerWidget { child: _QuickReactionCircle( size: circleSize, onTap: () { - Navigator.of(sheetContext).pop(popResult); - showEmojiPicker(context: pageContext, onSelect: react); + void effect() => + showEmojiPicker(context: pageContext, onSelect: react); + + final select = onSelected; + if (select != null) { + select(popResult, effect); + } else { + Navigator.of(sheetContext).pop(popResult); + effect(); + } }, child: Icon( LucideIcons.plus, diff --git a/mobile/lib/features/channels/message_actions/reaction_popover.dart b/mobile/lib/features/channels/message_actions/reaction_popover.dart index 57f40fc6e..11ea608b7 100644 --- a/mobile/lib/features/channels/message_actions/reaction_popover.dart +++ b/mobile/lib/features/channels/message_actions/reaction_popover.dart @@ -41,7 +41,7 @@ void _showMessageReactionPopover({ ); } -class _MessageReactionPopover extends StatelessWidget { +class _MessageReactionPopover extends HookWidget { final Rect anchorRect; final EdgeInsets spotlightPadding; final Animation animation; @@ -61,6 +61,14 @@ class _MessageReactionPopover extends StatelessWidget { @override Widget build(BuildContext context) { final mediaQuery = MediaQuery.of(context); + final selectionStarted = useRef(false); + + void select(Object? result, [VoidCallback? effect]) { + if (selectionStarted.value) return; + selectionStarted.value = true; + Navigator.of(context).pop(result); + effect?.call(); + } return LayoutBuilder( builder: (context, constraints) { @@ -125,7 +133,7 @@ class _MessageReactionPopover extends StatelessWidget { Positioned.fill( child: GestureDetector( behavior: HitTestBehavior.opaque, - onTap: () => Navigator.of(context).pop(), + onTap: () => select(null), ), ), Positioned( @@ -141,6 +149,7 @@ class _MessageReactionPopover extends StatelessWidget { message: message, pageContext: pageContext, pageRef: pageRef, + onSelected: (result, effect) => select(result, effect), ), ), ], @@ -159,6 +168,7 @@ class _AnimatedReactionTray extends StatelessWidget { final BuildContext pageContext; final WidgetRef pageRef; final Object? popResult; + final void Function(Object? result, VoidCallback effect)? onSelected; const _AnimatedReactionTray({ required this.trayKey, @@ -169,6 +179,7 @@ class _AnimatedReactionTray extends StatelessWidget { required this.pageContext, required this.pageRef, this.popResult, + this.onSelected, }); @override @@ -187,6 +198,7 @@ class _AnimatedReactionTray extends StatelessWidget { pageRef: pageRef, presentationAnimation: animation, popResult: popResult, + onSelected: onSelected, ), ), ), diff --git a/mobile/test/features/channels/message_actions_test.dart b/mobile/test/features/channels/message_actions_test.dart index ab3930729..1db4968d1 100644 --- a/mobile/test/features/channels/message_actions_test.dart +++ b/mobile/test/features/channels/message_actions_test.dart @@ -1,5 +1,6 @@ import 'dart:ui' as ui; +import 'package:buzz/features/channels/channel_management_provider.dart'; import 'package:buzz/features/channels/message_actions.dart'; import 'package:buzz/features/channels/message_long_press_region.dart'; import 'package:buzz/shared/read_state/read_state_provider.dart'; @@ -219,6 +220,7 @@ Future<_MessageActionsPopoverHarness> _pumpMessageActionsPopover( FocusNode? composerFocusNode, bool composerInitiallyFocused = false, bool launcherOnNestedRoute = false, + ChannelActions Function(Ref ref)? createChannelActions, Rect anchorRect = const Rect.fromLTWH(32, 260, 300, 72), }) async { final sourceHidden = ValueNotifier(false); @@ -268,6 +270,8 @@ Future<_MessageActionsPopoverHarness> _pumpMessageActionsPopover( ), ), reminderServiceProvider.overrideWithValue(reminderService), + if (createChannelActions != null) + channelActionsProvider.overrideWith(createChannelActions), ], child: MaterialApp( theme: AppTheme.light(), @@ -324,6 +328,26 @@ Future _dismissMessageActionsPopover(WidgetTester tester) async { await tester.pumpAndSettle(); } +class _FakeChannelActions extends ChannelActions { + final reactions = <({String eventId, String emoji})>[]; + + _FakeChannelActions(Ref ref) + : super( + ref: ref, + session: ref.read(relaySessionProvider.notifier), + signedEventRelay: SignedEventRelay( + session: ref.read(relaySessionProvider.notifier), + nsec: null, + ), + currentPubkey: 'self', + ); + + @override + Future addReaction(String eventId, String emoji) async { + reactions.add((eventId: eventId, emoji: emoji)); + } +} + void main() { testWidgets( 'message long press keeps taps and scrolling while repeated holds win', @@ -889,6 +913,68 @@ void main() { expect(tester.takeException(), isNull); }); + testWidgets('ignores repeat backdrop taps once dismissal starts', ( + tester, + ) async { + final prefs = await _mockPrefs(); + await _pumpMessageActionsPopover( + tester, + message: _message(), + prefs: prefs, + launcherOnNestedRoute: true, + ); + final backdrop = tester.widget( + find.byKey(const ValueKey('message-actions-backdrop')), + ); + + backdrop.onTap!.call(); + backdrop.onTap!.call(); + await tester.pumpAndSettle(); + + expect( + find.byKey(const ValueKey('message-actions-underlying-page')), + findsOneWidget, + ); + expect( + find.byKey(const ValueKey('message-actions-root-page')), + findsNothing, + ); + expect(tester.takeException(), isNull); + }); + + testWidgets('ignores repeat quick reactions once dismissal starts', ( + tester, + ) async { + final prefs = await _mockPrefs(); + late _FakeChannelActions actions; + await _pumpMessageActionsPopover( + tester, + message: _message(), + prefs: prefs, + launcherOnNestedRoute: true, + createChannelActions: (ref) => actions = _FakeChannelActions(ref), + ); + final reaction = find.byKey(const ValueKey('quick-reaction-\u{1F44D}')); + final detector = tester.widget( + find.descendant(of: reaction, matching: find.byType(GestureDetector)), + ); + + detector.onTap!.call(); + detector.onTap!.call(); + await tester.pumpAndSettle(); + + expect( + find.byKey(const ValueKey('message-actions-underlying-page')), + findsOneWidget, + ); + expect( + find.byKey(const ValueKey('message-actions-root-page')), + findsNothing, + ); + expect(actions.reactions, [(eventId: 'msg-1', emoji: '\u{1F44D}')]); + expect(tester.takeException(), isNull); + }); + testWidgets('fallback action rows grow with accessibility text', ( tester, ) async {