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 item in the list view.  Essentially, we want the behavior of
`SilverSafeArea.minimum` — the bottom of the list 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]>

revision

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,
+    // This removes insets from all sides except bottom.
+    // Leave it to the descendent [CustomScrollView].
     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

@@ -169,3 +169,8 @@ extension TableChecks on Subject<Table> {
 extension IconButtonChecks on Subject<IconButton> {
   Subject<bool?> get isSelected => has((x) => x.isSelected, 'isSelected');
 }
+
+extension OffsetChecks on Subject<Offset> {
+  Subject<double> get dx => has((x) => x.dx, 'dx');
+  Subject<double> get dy => has((x) => x.dy, 'dy');
+}

test/widgets/emoji_reaction_test.dart

@@ -469,6 +469,88 @@ void main() {
 
       debugNetworkImageHttpClientProvider = null;
     });
+
+    group('handle view paddings', () {
+      const screenHeight = 400.0;
+
+      late double scrollViewTop;
+      late double scrollViewBottom;
+      final scrollViewFinder = find.descendant(
+        of: find.byType(BottomSheet), matching: find.bySubtype<ScrollView>());
+
+      late Widget firstOption;
+      late Widget lastOption;
+      double getScrollChildrenTop(WidgetTester tester) =>
+        tester.getTopLeft(find.byWidget(firstOption)).dy;
+      double getScrollChildrenBottom(WidgetTester tester) =>
+        tester.getBottomLeft(find.byWidget(lastOption)).dy;
+
+      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.
+        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));
+
+        scrollViewTop = tester.getTopLeft(scrollViewFinder).dy;
+        scrollViewBottom = tester.getBottomLeft(scrollViewFinder).dy;
+        // The scroll view should expand all the way to the bottom of the
+        // screen, even if there is device bottom padding.
+        check(scrollViewBottom).equals(screenHeight);
+
+        final options = tester.widgetList(find.byType(EmojiPickerListEntry));
+        firstOption = options.first;
+        lastOption = options.last;
+      }
+
+      testWidgets('no view padding', (tester) async {
+        await prepare(tester, viewPadding: FakeViewPadding.zero);
+
+        // The top of the list of children is padded by 8px;
+        // the bottom of them is out of view.
+        check(scrollViewTop).equals(getScrollChildrenTop(tester) - 8);
+        check(scrollViewBottom).isLessThan(getScrollChildrenBottom(tester));
+
+        // Scroll to the very bottom of the list with a large offset.
+        await tester.drag(scrollViewFinder, Offset(0, -500));
+        await tester.pump();
+        // The top of the list of children is out of view.
+        // The bottom is padded by 8px, the minimum offset.
+        check(scrollViewTop).isGreaterThan(getScrollChildrenTop(tester));
+        check(scrollViewBottom).equals(getScrollChildrenBottom(tester) + 8);
+
+        debugNetworkImageHttpClientProvider = null;
+      });
+
+      testWidgets('with bottom view padding', (tester) async {
+        await prepare(tester, viewPadding: FakeViewPadding(bottom: 10));
+
+        // The top of the list of children is padded by 8px;
+        // The bottom of them is out of view.
+        check(scrollViewTop).equals(getScrollChildrenTop(tester) - 8);
+        check(scrollViewBottom).isLessThan(getScrollChildrenBottom(tester));
+
+        // Scroll to the very bottom of the list with a large offset.
+        await tester.drag(scrollViewFinder, Offset(0, -500));
+        await tester.pump();
+        // The top of the list of children is out of view;
+        // the bottom is padded by `max(8, 10)` px, where the view bottom
+        // padding is larger.
+        check(scrollViewTop).isGreaterThan(getScrollChildrenTop(tester));
+        check(scrollViewBottom).equals(getScrollChildrenBottom(tester) + 10);
+
+        debugNetworkImageHttpClientProvider = null;
+      });
+    });
   });
 }