diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index c418337e62..0f6d490a97 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/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))); }); } @@ -529,15 +530,21 @@ class _EmojiPickerState extends State with PerAccountStoreAwareStat style: const TextStyle(fontSize: 20, height: 30 / 20))), ])), Expanded(child: InsetShadowBox( - top: 8, bottom: 8, + top: 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)))), + ]))), ]); } } diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 08dd99f630..d2d4c91581 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -11,6 +11,11 @@ extension PaintChecks on Subject { Subject get shader => has((x) => x.shader, 'shader'); } +extension OffsetChecks on Subject { + Subject get dx => has((x) => x.dx, 'dx'); + Subject get dy => has((x) => x.dy, 'dy'); +} + extension RectChecks on Subject { Subject get left => has((d) => d.left, 'left'); Subject get top => has((d) => d.top, 'top'); diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 7ed54590ef..d86368493e 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -351,6 +351,9 @@ void main() { final searchFieldFinder = find.widgetWithText(TextField, 'Search emoji'); + Finder findInPicker(Finder finder) => + find.descendant(of: find.byType(EmojiPicker), matching: finder); + Condition conditionEmojiListEntry({ required ReactionType emojiType, required String emojiCode, @@ -429,9 +432,7 @@ void main() { await setupEmojiPicker(tester, message: message, narrow: TopicNarrow.ofMessage(message)); connection.prepare(json: {}); - await tester.tap(find.descendant( - of: find.byType(BottomSheet), - matching: find.text('\u{1f4a4}'))); // 'zzz' emoji + await tester.tap(findInPicker(find.text('\u{1f4a4}'))); // 'zzz' emoji await tester.pump(Duration.zero); check(connection.lastRequest).isA() @@ -453,9 +454,8 @@ void main() { connection.prepare( delay: const Duration(seconds: 2), apiException: eg.apiBadRequest(message: 'Invalid message(s)')); - await tester.tap(find.descendant( - of: find.byType(BottomSheet), - matching: find.text('\u{1f4a4}'))); // 'zzz' emoji + + await tester.tap(findInPicker(find.text('\u{1f4a4}'))); // 'zzz' emoji await tester.pump(); // register tap await tester.pump(const Duration(seconds: 1)); // emoji picker animates away await tester.pump(const Duration(seconds: 1)); // error arrives; error dialog shows @@ -466,6 +466,92 @@ void main() { debugNetworkImageHttpClientProvider = null; }); + + group('handle view paddings', () { + const screenHeight = 400.0; + + late Rect scrollViewRect; + final scrollViewFinder = findInPicker(find.bySubtype()); + + Rect getListEntriesRect(WidgetTester tester) => + tester.getRect(find.byType(EmojiPickerListEntry).first) + .expandToInclude(tester.getRect(find.byType(EmojiPickerListEntry).last)); + + Future 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; + }); + }); }); }