From 661f08ae28db1cbe0458bb7bc663e5e1eb863f39 Mon Sep 17 00:00:00 2001 From: Kenny Lopez Date: Sat, 15 Aug 2026 11:10:08 +0100 Subject: [PATCH] Handle message action edge cases Co-authored-by: Kenny Lopez Signed-off-by: Kenny Lopez --- .../features/channels/message_actions.dart | 119 +----------------- .../message_action_popover.dart | 7 +- .../message_actions/quick_reaction_row.dart | 119 ++++++++++++++++++ .../message_actions/reaction_popover.dart | 3 + .../channels/message_actions_test.dart | 59 ++++++++- 5 files changed, 187 insertions(+), 120 deletions(-) create mode 100644 mobile/lib/features/channels/message_actions/quick_reaction_row.dart diff --git a/mobile/lib/features/channels/message_actions.dart b/mobile/lib/features/channels/message_actions.dart index 83786b66d..a2647a256 100644 --- a/mobile/lib/features/channels/message_actions.dart +++ b/mobile/lib/features/channels/message_actions.dart @@ -39,6 +39,7 @@ import 'thread_follows/thread_follows_provider.dart'; import 'timeline_message.dart'; part 'message_actions/reaction_popover.dart'; +part 'message_actions/quick_reaction_row.dart'; part 'message_actions/message_action_popover.dart'; /// Preview length for reminder targets — matches desktop's @@ -663,127 +664,9 @@ class _FastActionTile extends StatelessWidget { } } -/// The row of one-tap reactions at the top of the action sheet, plus the "+" -/// tile that opens the full picker. -/// /// The emoji shown are the user's own frequently-used set (desktop's /// `useQuickReactionEmojis` behaviour), topped up with [defaultQuickEmojis] so /// the row is full on a fresh install. -class _QuickReactionRow extends ConsumerWidget { - final TimelineMessage message; - - /// The sheet's context, popped before the reaction fires. - final BuildContext sheetContext; - - /// The long-pressed message's page context — survives the sheet pop, so the - /// picker opened from "+" isn't torn down with the sheet. - final BuildContext pageContext; - - /// The long-pressed message's page ref. The picker callback outlives this - /// bottom sheet, so it must not read through the sheet's disposed ref. - final WidgetRef pageRef; - - /// Drives the staged glyph reveal when this row is shown in the popover. - /// The bottom sheet leaves this null and retains its existing static row. - final Animation? presentationAnimation; - - const _QuickReactionRow({ - required this.message, - required this.sheetContext, - required this.pageContext, - required this.pageRef, - this.presentationAnimation, - }); - - @override - Widget build(BuildContext context, WidgetRef ref) { - final customEmoji = ref.watch(customEmojiListProvider); - final emoji = quickReactionEmoji( - ref.watch(recentEmojiProvider), - customShortcodes: { - for (final entry in customEmoji) entry.shortcode.toLowerCase(), - }, - ); - final customByShortcode = { - for (final entry in customEmoji) entry.shortcode.toLowerCase(): entry, - }; - - void react(String value) { - // The generic picker is also used for composing and statuses. Record - // recency here, at the reaction call site, so only reactions drive the - // quick-reaction row. - pageRef.read(recentEmojiProvider.notifier).record(value); - // The sheet is on its way out, so the burst can't come from this tile — - // hand it to the pill that's about to appear in the timeline. - armReactionBurst(pageRef, message, value); - pageRef.read(channelActionsProvider).addReaction(message.id, value); - } - - return LayoutBuilder( - builder: (context, constraints) { - const desiredCircleSize = 52.0; - const minimumCircleSize = 44.0; - final itemCount = emoji.length + 1; - final gapCount = itemCount - 1; - final circleSize = - ((constraints.maxWidth - (Grid.twelve * gapCount)) / itemCount) - .clamp(minimumCircleSize, desiredCircleSize) - .toDouble(); - final gap = - ((constraints.maxWidth - (circleSize * itemCount)) / gapCount) - .clamp(0.0, Grid.twelve) - .toDouble(); - final circles = [ - for (var index = 0; index < emoji.length; index++) - _ReactionItemReveal( - key: ValueKey('quick-reaction-${emoji[index]}'), - animation: presentationAnimation, - index: index, - child: _QuickReactionCircle( - size: circleSize, - onTap: () { - Navigator.of(sheetContext).pop(); - react(emoji[index]); - }, - child: _QuickReactionGlyph( - value: emoji[index], - customByShortcode: customByShortcode, - ), - ), - ), - _ReactionItemReveal( - key: const ValueKey('quick-reaction-more'), - animation: presentationAnimation, - index: emoji.length, - child: _QuickReactionCircle( - size: circleSize, - onTap: () { - Navigator.of(sheetContext).pop(); - showEmojiPicker(context: pageContext, onSelect: react); - }, - child: Icon( - LucideIcons.plus, - size: 24, - color: context.colors.onSurfaceVariant, - ), - ), - ), - ]; - - return Row( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - for (var index = 0; index < circles.length; index++) ...[ - circles[index], - if (index < circles.length - 1) SizedBox(width: gap), - ], - ], - ); - }, - ); - } -} - class _ReactionItemReveal extends StatelessWidget { final Animation? animation; final int index; 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 10bb7200c..1f4c684dc 100644 --- a/mobile/lib/features/channels/message_actions/message_action_popover.dart +++ b/mobile/lib/features/channels/message_actions/message_action_popover.dart @@ -7,6 +7,7 @@ const _messageActionMenuMaxWidth = 288.0; const _messageActionPreviewMaxWidth = 358.0; const _messageActionPreviewInset = Grid.xxs; const _messageActionGap = Grid.twelve; +const _messageActionReactionSelection = '__reaction__'; const _messageActionTransitionDuration = _reactionPopoverDuration; const _iosMessageActionTransitionDuration = Duration(milliseconds: 220); const _iosNativeMessageActionSurfaceChannel = MethodChannel( @@ -551,7 +552,7 @@ class _MessageActionsPopover extends StatelessWidget { math.max(anchorRect.height, 1); final previewScale = math.min( 1.0, - math.max(0.1, math.min(previewWidthRatio, previewHeightRatio)), + math.max(0.0, math.min(previewWidthRatio, previewHeightRatio)), ); final previewSize = Size( (anchorRect.width * previewScale) + previewInsetExtent, @@ -679,6 +680,7 @@ class _MessageActionsPopover extends StatelessWidget { message: message, pageContext: pageContext, pageRef: pageRef, + popResult: _messageActionReactionSelection, ), ), Positioned.fromRect( @@ -766,6 +768,7 @@ class _MessageReactionTray extends StatelessWidget { final TimelineMessage message; final BuildContext pageContext; final WidgetRef pageRef; + final Object popResult; const _MessageReactionTray({ required this.animation, @@ -773,6 +776,7 @@ class _MessageReactionTray extends StatelessWidget { required this.message, required this.pageContext, required this.pageRef, + required this.popResult, }); @override @@ -785,6 +789,7 @@ class _MessageReactionTray extends StatelessWidget { message: message, pageContext: pageContext, pageRef: pageRef, + popResult: popResult, ); } } diff --git a/mobile/lib/features/channels/message_actions/quick_reaction_row.dart b/mobile/lib/features/channels/message_actions/quick_reaction_row.dart new file mode 100644 index 000000000..131f272c1 --- /dev/null +++ b/mobile/lib/features/channels/message_actions/quick_reaction_row.dart @@ -0,0 +1,119 @@ +part of '../message_actions.dart'; + +class _QuickReactionRow extends ConsumerWidget { + final TimelineMessage message; + + /// The sheet's context, popped before the reaction fires. + final BuildContext sheetContext; + + /// The long-pressed message's page context — survives the sheet pop, so the + /// picker opened from "+" isn't torn down with the sheet. + final BuildContext pageContext; + + /// The long-pressed message's page ref. The picker callback outlives this + /// bottom sheet, so it must not read through the sheet's disposed ref. + final WidgetRef pageRef; + + /// Drives the staged glyph reveal when this row is shown in the popover. + /// The bottom sheet leaves this null and retains its existing static row. + final Animation? presentationAnimation; + + final Object? popResult; + + const _QuickReactionRow({ + required this.message, + required this.sheetContext, + required this.pageContext, + required this.pageRef, + this.presentationAnimation, + this.popResult, + }); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final customEmoji = ref.watch(customEmojiListProvider); + final emoji = quickReactionEmoji( + ref.watch(recentEmojiProvider), + customShortcodes: { + for (final entry in customEmoji) entry.shortcode.toLowerCase(), + }, + ); + final customByShortcode = { + for (final entry in customEmoji) entry.shortcode.toLowerCase(): entry, + }; + + void react(String value) { + // The generic picker is also used for composing and statuses. Record + // recency here, at the reaction call site, so only reactions drive the + // quick-reaction row. + pageRef.read(recentEmojiProvider.notifier).record(value); + // The sheet is on its way out, so the burst can't come from this tile — + // hand it to the pill that's about to appear in the timeline. + armReactionBurst(pageRef, message, value); + pageRef.read(channelActionsProvider).addReaction(message.id, value); + } + + return LayoutBuilder( + builder: (context, constraints) { + const desiredCircleSize = 52.0; + const minimumCircleSize = 44.0; + final itemCount = emoji.length + 1; + final gapCount = itemCount - 1; + final circleSize = + ((constraints.maxWidth - (Grid.twelve * gapCount)) / itemCount) + .clamp(minimumCircleSize, desiredCircleSize) + .toDouble(); + final gap = + ((constraints.maxWidth - (circleSize * itemCount)) / gapCount) + .clamp(0.0, Grid.twelve) + .toDouble(); + final circles = [ + for (var index = 0; index < emoji.length; index++) + _ReactionItemReveal( + key: ValueKey('quick-reaction-${emoji[index]}'), + animation: presentationAnimation, + index: index, + child: _QuickReactionCircle( + size: circleSize, + onTap: () { + Navigator.of(sheetContext).pop(popResult); + react(emoji[index]); + }, + child: _QuickReactionGlyph( + value: emoji[index], + customByShortcode: customByShortcode, + ), + ), + ), + _ReactionItemReveal( + key: const ValueKey('quick-reaction-more'), + animation: presentationAnimation, + index: emoji.length, + child: _QuickReactionCircle( + size: circleSize, + onTap: () { + Navigator.of(sheetContext).pop(popResult); + showEmojiPicker(context: pageContext, onSelect: react); + }, + child: Icon( + LucideIcons.plus, + size: 24, + color: context.colors.onSurfaceVariant, + ), + ), + ), + ]; + + return Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + for (var index = 0; index < circles.length; index++) ...[ + circles[index], + if (index < circles.length - 1) SizedBox(width: gap), + ], + ], + ); + }, + ); + } +} diff --git a/mobile/lib/features/channels/message_actions/reaction_popover.dart b/mobile/lib/features/channels/message_actions/reaction_popover.dart index 692f2dc5a..57f40fc6e 100644 --- a/mobile/lib/features/channels/message_actions/reaction_popover.dart +++ b/mobile/lib/features/channels/message_actions/reaction_popover.dart @@ -158,6 +158,7 @@ class _AnimatedReactionTray extends StatelessWidget { final TimelineMessage message; final BuildContext pageContext; final WidgetRef pageRef; + final Object? popResult; const _AnimatedReactionTray({ required this.trayKey, @@ -167,6 +168,7 @@ class _AnimatedReactionTray extends StatelessWidget { required this.message, required this.pageContext, required this.pageRef, + this.popResult, }); @override @@ -184,6 +186,7 @@ class _AnimatedReactionTray extends StatelessWidget { pageContext: pageContext, pageRef: pageRef, presentationAnimation: animation, + popResult: popResult, ), ), ), diff --git a/mobile/test/features/channels/message_actions_test.dart b/mobile/test/features/channels/message_actions_test.dart index 7ea58011c..a4cd51228 100644 --- a/mobile/test/features/channels/message_actions_test.dart +++ b/mobile/test/features/channels/message_actions_test.dart @@ -216,6 +216,7 @@ Future<_MessageActionsPopoverHarness> _pumpMessageActionsPopover( EdgeInsets viewInsets = EdgeInsets.zero, FocusNode? composerFocusNode, bool composerInitiallyFocused = false, + Rect anchorRect = const Rect.fromLTWH(32, 260, 300, 72), }) async { final sourceHidden = ValueNotifier(false); @@ -258,7 +259,7 @@ Future<_MessageActionsPopoverHarness> _pumpMessageActionsPopover( allMessages: allMessages, currentPubkey: 'self', isMember: true, - anchorRect: const Rect.fromLTWH(32, 260, 300, 72), + anchorRect: anchorRect, captureAnchorSnapshot: _testMessageSnapshot, onPopoverPresented: () => sourceHidden.value = true, onPopoverDismissed: () => sourceHidden.value = false, @@ -605,6 +606,41 @@ void main() { await _dismissMessageActionsPopover(tester); }); + testWidgets('keeps tall message previews within the visible viewport', ( + tester, + ) async { + const keyboardInset = 300.0; + final prefs = await _mockPrefs(); + await _pumpMessageActionsPopover( + tester, + message: _message(pubkey: 'self'), + prefs: prefs, + canManageMessage: true, + allMessages: [_message(pubkey: 'self')], + reminderService: _stubReminderService(), + viewInsets: const EdgeInsets.only(bottom: keyboardInset), + anchorRect: const Rect.fromLTWH(32, 40, 300, 2000), + ); + + final logicalHeight = + tester.view.physicalSize.height / tester.view.devicePixelRatio; + final visibleBottom = logicalHeight - keyboardInset - Grid.xxs; + expect( + tester + .getRect(find.byKey(const ValueKey('message-action-preview'))) + .top, + greaterThanOrEqualTo(Grid.xxs), + ); + expect( + tester + .getRect(find.byKey(const ValueKey('message-action-surface'))) + .bottom, + lessThanOrEqualTo(visibleBottom), + ); + + await _dismissMessageActionsPopover(tester); + }); + testWidgets('restores composer focus only after a dismissed popover', ( tester, ) async { @@ -644,6 +680,27 @@ void main() { expect(focusNode.hasFocus, isFalse); }); + testWidgets('does not restore composer focus after opening reactions', ( + tester, + ) async { + final focusNode = FocusNode(); + addTearDown(focusNode.dispose); + final prefs = await _mockPrefs(); + + await _pumpMessageActionsPopover( + tester, + message: _message(), + prefs: prefs, + composerFocusNode: focusNode, + composerInitiallyFocused: true, + ); + + await tester.tap(find.byKey(const ValueKey('quick-reaction-more'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 400)); + expect(focusNode.hasFocus, isFalse); + }); + testWidgets('does not restore composer focus after selecting an action', ( tester, ) async {