mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
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 <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
This commit is contained in:
@@ -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,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -41,7 +41,7 @@ void _showMessageReactionPopover({
|
||||
);
|
||||
}
|
||||
|
||||
class _MessageReactionPopover extends StatelessWidget {
|
||||
class _MessageReactionPopover extends HookWidget {
|
||||
final Rect anchorRect;
|
||||
final EdgeInsets spotlightPadding;
|
||||
final Animation<double> 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,
|
||||
),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -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<void> _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<void> 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<GestureDetector>(
|
||||
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<GestureDetector>(
|
||||
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 {
|
||||
|
||||
Reference in New Issue
Block a user