Skip to content

Commit 584240c

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 item in the list view. Essentially, we want the behavior of `SafeArea.minimum` with a `ListView` — 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 — which is achieved through the use `ListView.padding` and `MediaQuery`. Signed-off-by: Zixuan James Li <[email protected]>
1 parent d29f08b commit 584240c

File tree

3 files changed

+78
-18
lines changed

3 files changed

+78
-18
lines changed

lib/widgets/emoji_reaction.dart

+31-18
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:math';
2+
13
import 'package:flutter/material.dart';
24

35
import '../api/exception.dart';
@@ -416,20 +418,21 @@ void showEmojiPickerSheet({
416418
// on my iPhone 13 Pro but is marked as "much slower":
417419
// https://api.flutter.dev/flutter/dart-ui/Clip.html
418420
clipBehavior: Clip.antiAlias,
421+
// This does remove the bottom inset.
422+
// Leave it to the descendent [ListView].
419423
useSafeArea: true,
420424
isScrollControlled: true,
421425
builder: (BuildContext context) {
422-
return SafeArea(
423-
child: Padding(
424-
// By default, when software keyboard is opened, the ListView
425-
// expands behind the software keyboard — resulting in some
426-
// list entries being covered by the keyboard. Add explicit
427-
// bottom padding the size of the keyboard, which fixes this.
428-
padding: EdgeInsets.only(bottom: MediaQuery.viewInsetsOf(context).bottom),
429-
// For _EmojiPickerItem, and RealmContentNetworkImage used in ImageEmojiWidget.
430-
child: PerAccountStoreWidget(
431-
accountId: store.accountId,
432-
child: EmojiPicker(pageContext: pageContext, message: message))));
426+
return Padding(
427+
// By default, when software keyboard is opened, the ListView
428+
// expands behind the software keyboard — resulting in some
429+
// list entries being covered by the keyboard. Add explicit
430+
// bottom padding the size of the keyboard, which fixes this.
431+
padding: EdgeInsets.only(bottom: MediaQuery.viewInsetsOf(context).bottom),
432+
// For _EmojiPickerItem, and RealmContentNetworkImage used in ImageEmojiWidget.
433+
child: PerAccountStoreWidget(
434+
accountId: store.accountId,
435+
child: EmojiPicker(pageContext: pageContext, message: message)));
433436
});
434437
}
435438

@@ -494,6 +497,7 @@ class _EmojiPickerState extends State<EmojiPicker> with PerAccountStoreAwareStat
494497
Widget build(BuildContext context) {
495498
final zulipLocalizations = ZulipLocalizations.of(context);
496499
final designVariables = DesignVariables.of(context);
500+
final mediaQuery = MediaQuery.of(context);
497501

498502
return Column(children: [
499503
Padding(padding: const EdgeInsetsDirectional.only(start: 8, top: 4),
@@ -530,13 +534,22 @@ class _EmojiPickerState extends State<EmojiPicker> with PerAccountStoreAwareStat
530534
Expanded(child: InsetShadowBox(
531535
top: 8, bottom: 8,
532536
color: designVariables.bgContextMenu,
533-
child: ListView.builder(
534-
padding: const EdgeInsets.symmetric(vertical: 8),
535-
itemCount: _resultsToDisplay.length,
536-
itemBuilder: (context, i) => EmojiPickerListEntry(
537-
pageContext: widget.pageContext,
538-
emoji: _resultsToDisplay[i].candidate,
539-
message: widget.message)))),
537+
child: MediaQuery.removePadding(
538+
context: context,
539+
// The bottom is padded below with `padding` within the [ListView].
540+
removeBottom: true,
541+
child: ListView.builder(
542+
padding: EdgeInsets.only(
543+
top: 8,
544+
// This mimics the behavior of [SafeArea.minimum], but with
545+
// the underlying [SliverPadding] added by [ListView], so that we
546+
// can always scroll past the shadow and the device bottom padding.
547+
bottom: max(8, mediaQuery.padding.bottom)),
548+
itemCount: _resultsToDisplay.length,
549+
itemBuilder: (context, i) => EmojiPickerListEntry(
550+
pageContext: widget.pageContext,
551+
emoji: _resultsToDisplay[i].candidate,
552+
message: widget.message))))),
540553
]);
541554
}
542555
}

test/flutter_checks.dart

+6
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ extension ElementChecks on Subject<Element> {
141141
}
142142

143143
extension MediaQueryDataChecks on Subject<MediaQueryData> {
144+
Subject<EdgeInsetsGeometry> get padding => has((x) => x.padding, 'padding');
144145
Subject<TextScaler> get textScaler => has((x) => x.textScaler, 'textScaler');
145146
// TODO more
146147
}
@@ -169,3 +170,8 @@ extension TableChecks on Subject<Table> {
169170
extension IconButtonChecks on Subject<IconButton> {
170171
Subject<bool?> get isSelected => has((x) => x.isSelected, 'isSelected');
171172
}
173+
174+
extension ListViewChecks on Subject<ListView> {
175+
Subject<EdgeInsetsGeometry?> get padding => has((x) => x.padding, 'padding');
176+
}
177+

test/widgets/emoji_reaction_test.dart

+41
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import 'package:zulip/model/store.dart';
1818
import 'package:zulip/widgets/content.dart';
1919
import 'package:zulip/widgets/emoji_reaction.dart';
2020
import 'package:zulip/widgets/icons.dart';
21+
import 'package:zulip/widgets/inset_shadow.dart';
2122
import 'package:zulip/widgets/message_list.dart';
2223

2324
import '../api/fake_api.dart';
@@ -469,6 +470,46 @@ void main() {
469470

470471
debugNetworkImageHttpClientProvider = null;
471472
});
473+
474+
testWidgets('consume bottom padding with less than 8px', (tester) async {
475+
final fakePadding = FakeViewPadding(
476+
left: 20, top: 20, right: 20, bottom: 6 * tester.view.devicePixelRatio);
477+
tester.view.viewPadding = fakePadding;
478+
tester.view.padding = fakePadding;
479+
final message = eg.streamMessage();
480+
await setupEmojiPicker(
481+
tester, message: message, narrow: TopicNarrow.ofMessage(message));
482+
483+
final shadowBox = tester.element(findInPicker(find.byType(InsetShadowBox)));
484+
check(MediaQuery.of(shadowBox).padding).equals(EdgeInsets.only(bottom: 6));
485+
486+
final listView = tester.element(findInPicker(find.byType(ListView)));
487+
check(MediaQuery.of(listView).padding).equals(EdgeInsets.zero);
488+
check(listView.widget).isA<ListView>().padding
489+
.equals(EdgeInsets.only(top: 8, bottom: 8));
490+
491+
debugNetworkImageHttpClientProvider = null;
492+
});
493+
494+
testWidgets('consume bottom padding with more than 8px', (tester) async {
495+
final fakePadding = FakeViewPadding(
496+
left: 20, top: 20, right: 20, bottom: 10 * tester.view.devicePixelRatio);
497+
tester.view.viewPadding = fakePadding;
498+
tester.view.padding = fakePadding;
499+
final message = eg.streamMessage();
500+
await setupEmojiPicker(
501+
tester, message: message, narrow: TopicNarrow.ofMessage(message));
502+
503+
final shadowBox = tester.element(findInPicker(find.byType(InsetShadowBox)));
504+
check(MediaQuery.of(shadowBox)).padding.equals(EdgeInsets.only(bottom: 10));
505+
506+
final listView = tester.element(findInPicker(find.byType(ListView)));
507+
check(MediaQuery.of(listView)).padding.equals(EdgeInsets.zero);
508+
check(listView.widget).isA<ListView>().padding
509+
.equals(EdgeInsets.only(top: 8, bottom: 10));
510+
511+
debugNetworkImageHttpClientProvider = null;
512+
});
472513
});
473514
}
474515

0 commit comments

Comments
 (0)