Skip to content

Commit 058422a

Browse files
committed
emoji: Ensure consistent height for text-only list items
Previously, text-only emoji rows in the emoji picker and autocomplete lists were shorter than rows with image emoji, causing layout inconsistency. This commit fixes the layout by conditionally adding a `SizedBox` spacer for text-only items. The spacer's height is set to match the total height of a padded image glyph, ensuring all rows have a consistent height without adding unwanted horizontal padding. This change is applied to both `EmojiPickerListEntry` and `_EmojiAutocompleteItem`. The corresponding regression tests are updated to verify that the height of a text-only item is identical to an image-based item. Fixes #1587.
1 parent 7e98ff1 commit 058422a

File tree

4 files changed

+116
-49
lines changed

4 files changed

+116
-49
lines changed

lib/widgets/autocomplete.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ class _EmojiAutocompleteItem extends StatelessWidget {
377377
ImageEmojiWidget(size: _size, emojiDisplay: emojiDisplay),
378378
UnicodeEmojiDisplay() =>
379379
UnicodeEmojiWidget(size: _size, emojiDisplay: emojiDisplay),
380-
TextEmojiDisplay() => null, // The text is already shown separately.
380+
TextEmojiDisplay() => null,
381381
};
382382

383383
final label = candidate.aliases.isEmpty
@@ -395,11 +395,12 @@ class _EmojiAutocompleteItem extends StatelessWidget {
395395
return Padding(
396396
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
397397
child: Row(children: [
398-
if (glyph != null) ...[
398+
if (glyph != null)
399399
Padding(padding: const EdgeInsets.all(6),
400-
child: glyph),
401-
const SizedBox(width: 6),
402-
],
400+
child: glyph)
401+
else
402+
const SizedBox(height: 36.0, width: 0),
403+
const SizedBox(width: 6),
403404
Expanded(
404405
child: Text(
405406
style: TextStyle(fontSize: 17, height: 18 / 17,

lib/widgets/emoji_reaction.dart

Lines changed: 48 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -597,49 +597,53 @@ class EmojiPickerListEntry extends StatelessWidget {
597597
Navigator.pop(pageContext, emoji);
598598
}
599599

600-
@override
601-
Widget build(BuildContext context) {
602-
final store = PerAccountStoreWidget.of(context);
603-
final designVariables = DesignVariables.of(context);
604-
605-
// TODO deduplicate this logic with [_EmojiAutocompleteItem]
606-
final emojiDisplay = emoji.emojiDisplay.resolve(store.userSettings);
607-
final Widget? glyph = switch (emojiDisplay) {
608-
ImageEmojiDisplay() =>
609-
ImageEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
610-
UnicodeEmojiDisplay() =>
611-
UnicodeEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
612-
TextEmojiDisplay() => null, // The text is already shown separately.
613-
};
614-
615-
final label = emoji.aliases.isEmpty
616-
? emoji.emojiName
617-
: [emoji.emojiName, ...emoji.aliases].join(", "); // TODO(#1080)
618-
619-
return InkWell(
620-
onTap: _onPressed,
621-
splashFactory: NoSplash.splashFactory,
622-
overlayColor: WidgetStateColor.resolveWith((states) =>
623-
states.any((e) => e == WidgetState.pressed)
624-
? designVariables.contextMenuItemBg.withFadedAlpha(0.20)
625-
: Colors.transparent),
626-
child: Padding(
627-
padding: const EdgeInsets.symmetric(horizontal: 8),
628-
child: Row(spacing: 4, children: [
629-
if (glyph != null)
630-
Padding(
631-
padding: const EdgeInsets.all(10),
632-
child: glyph),
633-
Flexible(child: Text(label,
634-
maxLines: 2,
635-
overflow: TextOverflow.ellipsis,
636-
style: TextStyle(
637-
fontSize: 17,
638-
height: 18 / 17,
639-
color: designVariables.textMessage)))
640-
]),
641-
));
642-
}
600+
@override
601+
Widget build(BuildContext context) {
602+
final store = PerAccountStoreWidget.of(context);
603+
final designVariables = DesignVariables.of(context);
604+
605+
// TODO deduplicate this logic with [_EmojiAutocompleteItem]
606+
final emojiDisplay = emoji.emojiDisplay.resolve(store.userSettings);
607+
final Widget? glyph = switch (emojiDisplay) {
608+
ImageEmojiDisplay() =>
609+
ImageEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
610+
UnicodeEmojiDisplay() =>
611+
UnicodeEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
612+
TextEmojiDisplay() => null,// The text is already shown separately.
613+
};
614+
615+
final label = emoji.aliases.isEmpty
616+
? emoji.emojiName
617+
: [emoji.emojiName, ...emoji.aliases].join(", ");// TODO(#1080)
618+
619+
return InkWell(
620+
onTap: _onPressed,
621+
splashFactory: NoSplash.splashFactory,
622+
overlayColor: WidgetStateColor.resolveWith((states) =>
623+
states.any((e) => e == WidgetState.pressed)
624+
? designVariables.contextMenuItemBg.withFadedAlpha(0.20)
625+
: Colors.transparent),
626+
child: Padding(
627+
padding: const EdgeInsets.symmetric(horizontal: 8),
628+
child: Row(spacing: 4, children: [
629+
if (glyph != null)
630+
Padding(
631+
padding: const EdgeInsets.symmetric(vertical: 10.0),
632+
child: glyph)
633+
else
634+
SizedBox(
635+
height: _emojiSize + 8.0 + 8.0,
636+
),
637+
Flexible(child: Text(label,
638+
maxLines: 2,
639+
overflow: TextOverflow.ellipsis,
640+
style: TextStyle(
641+
fontSize: 17,
642+
height: 18 / 17,
643+
color: designVariables.textMessage)))
644+
]),
645+
));
646+
}
643647
}
644648

645649
/// Opens a bottom sheet showing who reacted to the message.
@@ -1084,4 +1088,4 @@ class ViewReactionsUserItem extends StatelessWidget {
10841088
store.userDisplayName(userId))),
10851089
])));
10861090
}
1087-
}
1091+
}

test/widgets/autocomplete_test.dart

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,42 @@ void main() {
372372
expected ? displaySubject.findsOne(): displaySubject.findsNothing();
373373
}
374374

375+
testWidgets('text emoji autocomplete item must have correct height', (tester) async {
376+
// Regression test for #1587
377+
final composeInputFinder = await setupToComposeInput(tester);
378+
379+
await store.handleEvent(RealmEmojiUpdateEvent(id: 1, realmEmoji: {
380+
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'an_image_emoji'),
381+
}));
382+
await tester.pump();
383+
384+
// TODO(#226): Remove this extra edit when this bug is fixed.
385+
await tester.enterText(composeInputFinder, ':an_imag');
386+
await tester.enterText(composeInputFinder, ':an_image');
387+
await tester.pumpAndSettle();
388+
389+
final imageItemFinder = find.widgetWithText(InkWell, 'an_image_emoji');
390+
check(imageItemFinder).findsOne();
391+
final imageItemHeight = tester.getSize(imageItemFinder).height;
392+
393+
// TODO(#226): Remove this extra edit when this bug is fixed.
394+
await store.handleEvent(UserSettingsUpdateEvent(id: 1,
395+
property: UserSettingName.emojiset,
396+
value: Emojiset.text));
397+
398+
await tester.enterText(composeInputFinder, ':an_imag');
399+
await tester.enterText(composeInputFinder, ':an_image');
400+
await tester.pumpAndSettle();
401+
402+
final textItemFinder = find.widgetWithText(InkWell, 'an_image_emoji');
403+
check(textItemFinder).findsOne();
404+
final textItemHeight = tester.getSize(textItemFinder).height;
405+
406+
check(textItemHeight).isCloseTo(imageItemHeight, 1.0);
407+
408+
debugNetworkImageHttpClientProvider = null;
409+
});
410+
375411
testWidgets('show, update, choose', (tester) async {
376412
final composeInputFinder = await setupToComposeInput(tester);
377413
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

test/widgets/emoji_reaction_test.dart

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,32 @@ void main() {
625625
debugNetworkImageHttpClientProvider = null;
626626
});
627627

628+
testWidgets('text emoji list items must have correct height', (tester) async {
629+
// Regression test for #1587.
630+
final message = eg.streamMessage();
631+
await setupEmojiPicker(tester, message: message, narrow: TopicNarrow.ofMessage(message));
632+
633+
// Find a list entry and get its height with the default image-based emoji.
634+
final listEntryFinder = find.byWidgetPredicate(
635+
(w) => w is EmojiPickerListEntry && w.emoji.emojiName == 'zzz');
636+
await tester.ensureVisible(listEntryFinder);
637+
final imageRenderBox = tester.renderObject<RenderBox>(listEntryFinder);
638+
final imageHeight = imageRenderBox.size.height;
639+
640+
// Change the setting to text-only emoji.
641+
await store.handleEvent(UserSettingsUpdateEvent(id: 1,
642+
property: UserSettingName.emojiset,
643+
value: Emojiset.text));
644+
await tester.pumpAndSettle();
645+
646+
// Find the same list entry again and check that its height has not changed.
647+
await tester.ensureVisible(listEntryFinder);
648+
final textRenderBox = tester.renderObject<RenderBox>(listEntryFinder);
649+
check(textRenderBox.size.height).isCloseTo(imageHeight, 1.0);
650+
651+
debugNetworkImageHttpClientProvider = null;
652+
});
653+
628654
testWidgets('with bottom view padding', (tester) async {
629655
await prepare(tester, viewPadding: FakeViewPadding(bottom: 10));
630656

0 commit comments

Comments
 (0)