Handle message action edge cases

Co-authored-by: Kenny Lopez <klopez4212@gmail.com>
Signed-off-by: Kenny Lopez <klopez4212@gmail.com>

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<double>? 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 = <Widget>[
-          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<double>? animation;
   final int index;

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,
     );
   }
 }

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<double>? 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 = <Widget>[
+          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),
+            ],
+          ],
+        );
+      },
+    );
+  }
+}

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,
           ),
         ),
       ),

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 {