emoji: Fix bottom padding of emoji picker

Previously, the body of the bottom sheet was wrapped in `SafeArea`. This
pads the bottom unconditionally, shifting the bottom of the list view
above the device bottom padding.

This is undesirable, because the area beneath the bottom padding is still
visible, and the list view should extend to the bottom regardless of the
bottom inset.

By just removing the `SafeArea`, the list view extends to the bottom.
However, if the bottom padding is more than 8px, we can't scroll past the
last entry in the list view.  Essentially, we want the behavior of
`SilverSafeArea.minimum` — the bottom edge of the list entries should
always be padded by at least 8px, so that we can scroll past the shadow;
and if the bottom padding is more than 8px, use that as the padding
instead.

Signed-off-by: Zixuan James Li <[email protected]>

Author: PIG208

lib/widgets/emoji_reaction.dart

@@ -417,20 +417,21 @@ void showEmojiPickerSheet({
     // on my iPhone 13 Pro but is marked as "much slower":
     //   https://api.flutter.dev/flutter/dart-ui/Clip.html
     clipBehavior: Clip.antiAlias,
+    // The bottom inset is left for [builder] to handle;
+    // see [EmojiPicker] and its [CustomScrollView] for how we do that.
     useSafeArea: true,
     isScrollControlled: true,
     builder: (BuildContext context) {
-      return SafeArea(
-        child: Padding(
-          // By default, when software keyboard is opened, the ListView
-          // expands behind the software keyboard — resulting in some
-          // list entries being covered by the keyboard. Add explicit
-          // bottom padding the size of the keyboard, which fixes this.
-          padding: EdgeInsets.only(bottom: MediaQuery.viewInsetsOf(context).bottom),
-          // For _EmojiPickerItem, and RealmContentNetworkImage used in ImageEmojiWidget.
-          child: PerAccountStoreWidget(
-            accountId: store.accountId,
-            child: EmojiPicker(pageContext: pageContext, message: message))));
+      return Padding(
+        // By default, when software keyboard is opened, the ListView
+        // expands behind the software keyboard — resulting in some
+        // list entries being covered by the keyboard. Add explicit
+        // bottom padding the size of the keyboard, which fixes this.
+        padding: EdgeInsets.only(bottom: MediaQuery.viewInsetsOf(context).bottom),
+        // For _EmojiPickerItem, and RealmContentNetworkImage used in ImageEmojiWidget.
+        child: PerAccountStoreWidget(
+          accountId: store.accountId,
+          child: EmojiPicker(pageContext: pageContext, message: message)));
     });
 }
 
@@ -531,13 +532,19 @@ class _EmojiPickerState extends State<EmojiPicker> with PerAccountStoreAwareStat
       Expanded(child: InsetShadowBox(
         top: 8, bottom: 8,
         color: designVariables.bgContextMenu,
-        child: ListView.builder(
-          padding: const EdgeInsets.symmetric(vertical: 8),
-          itemCount: _resultsToDisplay.length,
-          itemBuilder: (context, i) => EmojiPickerListEntry(
-            pageContext: widget.pageContext,
-            emoji: _resultsToDisplay[i].candidate,
-            message: widget.message)))),
+        child: CustomScrollView(
+          slivers: [
+            SliverPadding(
+              padding: EdgeInsets.only(top: 8),
+              sliver: SliverSafeArea(
+                minimum: EdgeInsets.only(bottom: 8),
+                sliver: SliverList.builder(
+                  itemCount: _resultsToDisplay.length,
+                  itemBuilder: (context, i) => EmojiPickerListEntry(
+                    pageContext: widget.pageContext,
+                    emoji: _resultsToDisplay[i].candidate,
+                    message: widget.message)))),
+          ]))),
     ]);
   }
 }

test/flutter_checks.dart

@@ -11,6 +11,11 @@ extension PaintChecks on Subject<Paint> {
   Subject<Shader?> get shader => has((x) => x.shader, 'shader');
 }
 
+extension OffsetChecks on Subject<Offset> {
+  Subject<double> get dx => has((x) => x.dx, 'dx');
+  Subject<double> get dy => has((x) => x.dy, 'dy');
+}
+
 extension RectChecks on Subject<Rect> {
   Subject<double> get left => has((d) => d.left, 'left');
   Subject<double> get top => has((d) => d.top, 'top');

test/widgets/emoji_reaction_test.dart

@@ -466,6 +466,92 @@ void main() {
 
       debugNetworkImageHttpClientProvider = null;
     });
+
+    group('handle view paddings', () {
+      const screenHeight = 400.0;
+
+      late Rect scrollViewRect;
+      final scrollViewFinder = findInPicker(find.bySubtype<ScrollView>());
+
+      Rect getListEntriesRect(WidgetTester tester) =>
+        tester.getRect(find.byType(EmojiPickerListEntry).first)
+          .expandToInclude(tester.getRect(find.byType(EmojiPickerListEntry).last));
+
+      Future<void> prepare(WidgetTester tester, {
+        required FakeViewPadding viewPadding,
+      }) async {
+        addTearDown(tester.view.reset);
+        tester.view.physicalSize = Size(640, screenHeight);
+        // This makes it easier to convert between device pixels used for
+        // [FakeViewPadding] and logical pixels used in tests.
+        // If needed, there is a clearer way to implement this generally.
+        // See comment: https://github.com/zulip/zulip-flutter/pull/1315/files#r1962703436
+        tester.view.devicePixelRatio = 1.0;
+
+        tester.view.viewPadding = viewPadding;
+        tester.view.padding = viewPadding;
+
+        final message = eg.streamMessage();
+        await setupEmojiPicker(tester,
+          message: message, narrow: TopicNarrow.ofMessage(message));
+
+        scrollViewRect = tester.getRect(scrollViewFinder);
+        // The scroll view should expand all the way to the bottom of the
+        // screen, even if there is device bottom padding.
+        check(scrollViewRect)
+          ..bottom.equals(screenHeight)
+          // There should always be enough entries to overflow the scroll view.
+          ..height.isLessThan(getListEntriesRect(tester).height);
+      }
+
+      testWidgets('no view padding', (tester) async {
+        await prepare(tester, viewPadding: FakeViewPadding.zero);
+
+        // The top edge of the list entries is padded by 8px from the top edge
+        // of the scroll view; the bottom edge is out of view.
+        Rect listEntriesRect = getListEntriesRect(tester);
+        check(scrollViewRect)
+          ..top.equals(listEntriesRect.top - 8)
+          ..bottom.isLessThan(listEntriesRect.bottom);
+
+        // Scroll to the very bottom of the list with a large offset.
+        await tester.drag(scrollViewFinder, Offset(0, -500));
+        await tester.pump();
+        // The top edge of the list entries is out of view;
+        // the bottom is padded by 8px, the minimum padding, from the bottom
+        // edge of the scroll view.
+        listEntriesRect = getListEntriesRect(tester);
+        check(scrollViewRect)
+          ..top.isGreaterThan(listEntriesRect.top)
+          ..bottom.equals(listEntriesRect.bottom + 8);
+
+        debugNetworkImageHttpClientProvider = null;
+      });
+
+      testWidgets('with bottom view padding', (tester) async {
+        await prepare(tester, viewPadding: FakeViewPadding(bottom: 10));
+
+        // The top edge of the list entries is padded by 8px from the top edge
+        // of the scroll view; the bottom edge is out of view.
+        Rect listEntriesRect = getListEntriesRect(tester);
+        check(scrollViewRect)
+          ..top.equals(listEntriesRect.top - 8)
+          ..bottom.isLessThan(listEntriesRect.bottom);
+
+        // Scroll to the very bottom of the list with a large offset.
+        await tester.drag(scrollViewFinder, Offset(0, -500));
+        await tester.pump();
+        // The top edge of the list entries is out of view;
+        // the bottom edge is padded by 10px from the bottom edge of the scroll
+        // view, because the view bottom padding is larger than the minimum 8px.
+        listEntriesRect = getListEntriesRect(tester);
+        check(scrollViewRect)
+          ..top.isGreaterThan(listEntriesRect.top)
+          ..bottom.equals(listEntriesRect.bottom + 10);
+
+        debugNetworkImageHttpClientProvider = null;
+      });
+    });
   });
 }