Skip to content

Commit 42b4a93

Browse files
committed
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]>
1 parent f8dd2c4 commit 42b4a93

File tree

3 files changed

+111
-18
lines changed

3 files changed

+111
-18
lines changed

lib/widgets/emoji_reaction.dart

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -417,20 +417,21 @@ void showEmojiPickerSheet({
417417
// on my iPhone 13 Pro but is marked as "much slower":
418418
// https://api.flutter.dev/flutter/dart-ui/Clip.html
419419
clipBehavior: Clip.antiAlias,
420+
// The bottom inset is left for [builder] to handle;
421+
// see [EmojiPicker] and its [CustomScrollView] for how we do that.
420422
useSafeArea: true,
421423
isScrollControlled: true,
422424
builder: (BuildContext context) {
423-
return SafeArea(
424-
child: Padding(
425-
// By default, when software keyboard is opened, the ListView
426-
// expands behind the software keyboard — resulting in some
427-
// list entries being covered by the keyboard. Add explicit
428-
// bottom padding the size of the keyboard, which fixes this.
429-
padding: EdgeInsets.only(bottom: MediaQuery.viewInsetsOf(context).bottom),
430-
// For _EmojiPickerItem, and RealmContentNetworkImage used in ImageEmojiWidget.
431-
child: PerAccountStoreWidget(
432-
accountId: store.accountId,
433-
child: EmojiPicker(pageContext: pageContext, message: message))));
425+
return Padding(
426+
// By default, when software keyboard is opened, the ListView
427+
// expands behind the software keyboard — resulting in some
428+
// list entries being covered by the keyboard. Add explicit
429+
// bottom padding the size of the keyboard, which fixes this.
430+
padding: EdgeInsets.only(bottom: MediaQuery.viewInsetsOf(context).bottom),
431+
// For _EmojiPickerItem, and RealmContentNetworkImage used in ImageEmojiWidget.
432+
child: PerAccountStoreWidget(
433+
accountId: store.accountId,
434+
child: EmojiPicker(pageContext: pageContext, message: message)));
434435
});
435436
}
436437

@@ -531,13 +532,19 @@ class _EmojiPickerState extends State<EmojiPicker> with PerAccountStoreAwareStat
531532
Expanded(child: InsetShadowBox(
532533
top: 8, bottom: 8,
533534
color: designVariables.bgContextMenu,
534-
child: ListView.builder(
535-
padding: const EdgeInsets.symmetric(vertical: 8),
536-
itemCount: _resultsToDisplay.length,
537-
itemBuilder: (context, i) => EmojiPickerListEntry(
538-
pageContext: widget.pageContext,
539-
emoji: _resultsToDisplay[i].candidate,
540-
message: widget.message)))),
535+
child: CustomScrollView(
536+
slivers: [
537+
SliverPadding(
538+
padding: EdgeInsets.only(top: 8),
539+
sliver: SliverSafeArea(
540+
minimum: EdgeInsets.only(bottom: 8),
541+
sliver: SliverList.builder(
542+
itemCount: _resultsToDisplay.length,
543+
itemBuilder: (context, i) => EmojiPickerListEntry(
544+
pageContext: widget.pageContext,
545+
emoji: _resultsToDisplay[i].candidate,
546+
message: widget.message)))),
547+
]))),
541548
]);
542549
}
543550
}

test/flutter_checks.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,8 @@ extension TableChecks on Subject<Table> {
169169
extension IconButtonChecks on Subject<IconButton> {
170170
Subject<bool?> get isSelected => has((x) => x.isSelected, 'isSelected');
171171
}
172+
173+
extension OffsetChecks on Subject<Offset> {
174+
Subject<double> get dx => has((x) => x.dx, 'dx');
175+
Subject<double> get dy => has((x) => x.dy, 'dy');
176+
}

test/widgets/emoji_reaction_test.dart

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,87 @@ void main() {
469469

470470
debugNetworkImageHttpClientProvider = null;
471471
});
472+
473+
group('handle view paddings', () {
474+
const screenHeight = 400.0;
475+
476+
late double scrollViewTopEdgeOffset;
477+
late double scrollViewBottomEdgeOffset;
478+
final scrollViewFinder = findInPicker(find.bySubtype<ScrollView>());
479+
480+
final listEntryFinder = find.byType(EmojiPickerListEntry);
481+
double getListEntriesTopEdgeOffset(WidgetTester tester) =>
482+
tester.getTopLeft(listEntryFinder.first).dy;
483+
double getListEntriesBottomEdgeOffset(WidgetTester tester) =>
484+
tester.getBottomLeft(listEntryFinder.last).dy;
485+
486+
Future<void> prepare(WidgetTester tester, {
487+
required FakeViewPadding viewPadding,
488+
}) async {
489+
addTearDown(tester.view.reset);
490+
tester.view.physicalSize = Size(640, screenHeight);
491+
// This makes it easier to convert between device pixels used for
492+
// [FakeViewPadding] and logical pixels used in tests.
493+
tester.view.devicePixelRatio = 1.0;
494+
495+
tester.view.viewPadding = viewPadding;
496+
tester.view.padding = viewPadding;
497+
498+
final message = eg.streamMessage();
499+
await setupEmojiPicker(tester,
500+
message: message, narrow: TopicNarrow.ofMessage(message));
501+
502+
scrollViewTopEdgeOffset = tester.getTopLeft(scrollViewFinder).dy;
503+
scrollViewBottomEdgeOffset = tester.getBottomLeft(scrollViewFinder).dy;
504+
final scrollViewHeight = tester.getSize(scrollViewFinder).height;
505+
// The scroll view should expand all the way to the bottom of the
506+
// screen, even if there is device bottom padding.
507+
check(scrollViewBottomEdgeOffset).equals(screenHeight);
508+
// There should always be enough entries to overflow the scroll view.
509+
check(getListEntriesBottomEdgeOffset(tester) - getListEntriesTopEdgeOffset(tester))
510+
.isGreaterThan(scrollViewHeight);
511+
}
512+
513+
testWidgets('no view padding', (tester) async {
514+
await prepare(tester, viewPadding: FakeViewPadding.zero);
515+
516+
// The top edge of the list entries is padded by 8px from the top edge
517+
// of the scroll view; the bottom edge is out of view.
518+
check(scrollViewTopEdgeOffset).equals(getListEntriesTopEdgeOffset(tester) - 8);
519+
check(scrollViewBottomEdgeOffset).isLessThan(getListEntriesBottomEdgeOffset(tester));
520+
521+
// Scroll to the very bottom of the list with a large offset.
522+
await tester.drag(scrollViewFinder, Offset(0, -500));
523+
await tester.pump();
524+
// The top edge of the list entries is out of view;
525+
// the bottom is padded by 8px, the minimum padding, from the bottom
526+
// edge of the scroll view.
527+
check(scrollViewTopEdgeOffset).isGreaterThan(getListEntriesTopEdgeOffset(tester));
528+
check(scrollViewBottomEdgeOffset).equals(getListEntriesBottomEdgeOffset(tester) + 8);
529+
530+
debugNetworkImageHttpClientProvider = null;
531+
});
532+
533+
testWidgets('with bottom view padding', (tester) async {
534+
await prepare(tester, viewPadding: FakeViewPadding(bottom: 10));
535+
536+
// The top edge of the list entries is padded by 8px from the top edge
537+
// of the scroll view; the bottom edge is out of view.
538+
check(scrollViewTopEdgeOffset).equals(getListEntriesTopEdgeOffset(tester) - 8);
539+
check(scrollViewBottomEdgeOffset).isLessThan(getListEntriesBottomEdgeOffset(tester));
540+
541+
// Scroll to the very bottom of the list with a large offset.
542+
await tester.drag(scrollViewFinder, Offset(0, -500));
543+
await tester.pump();
544+
// The top edge of the list entries is out of view;
545+
// the bottom edge is padded by 10px from the bottom edge of the scroll
546+
// view, because the view bottom padding is larger than the minimum 8px.
547+
check(scrollViewTopEdgeOffset).isGreaterThan(getListEntriesTopEdgeOffset(tester));
548+
check(scrollViewBottomEdgeOffset).equals(getListEntriesBottomEdgeOffset(tester) + 10);
549+
550+
debugNetworkImageHttpClientProvider = null;
551+
});
552+
});
472553
});
473554
}
474555

0 commit comments

Comments
 (0)