Skip to content

Commit ffe4843

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 6a35cc4 commit ffe4843

File tree

4 files changed

+73
-8
lines changed

4 files changed

+73
-8
lines changed

lib/widgets/autocomplete.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ class _EmojiAutocompleteItem extends StatelessWidget {
349349
ImageEmojiWidget(size: _size, emojiDisplay: emojiDisplay),
350350
UnicodeEmojiDisplay() =>
351351
UnicodeEmojiWidget(size: _size, emojiDisplay: emojiDisplay),
352-
TextEmojiDisplay() => null, // The text is already shown separately.
352+
TextEmojiDisplay() => null,
353353
};
354354

355355
final label = candidate.aliases.isEmpty
@@ -367,11 +367,12 @@ class _EmojiAutocompleteItem extends StatelessWidget {
367367
return Padding(
368368
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
369369
child: Row(children: [
370-
if (glyph != null) ...[
370+
if (glyph != null)
371371
Padding(padding: const EdgeInsets.all(6),
372-
child: glyph),
373-
const SizedBox(width: 6),
374-
],
372+
child: glyph)
373+
else
374+
const SizedBox(height: 36.0, width: 0),
375+
const SizedBox(width: 6),
375376
Expanded(
376377
child: Text(
377378
style: TextStyle(fontSize: 17, height: 18 / 17,

lib/widgets/emoji_reaction.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ class EmojiPickerListEntry extends StatelessWidget {
563563
ImageEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
564564
UnicodeEmojiDisplay() =>
565565
UnicodeEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
566-
TextEmojiDisplay() => null, // The text is already shown separately.
566+
TextEmojiDisplay() => null,
567567
};
568568

569569
final label = emoji.aliases.isEmpty
@@ -583,7 +583,9 @@ class EmojiPickerListEntry extends StatelessWidget {
583583
if (glyph != null)
584584
Padding(
585585
padding: const EdgeInsets.all(10),
586-
child: glyph),
586+
child: glyph)
587+
else
588+
const SizedBox(height: 40.0, width: 0),
587589
Flexible(child: Text(label,
588590
maxLines: 2,
589591
overflow: TextOverflow.ellipsis,
@@ -594,4 +596,4 @@ class EmojiPickerListEntry extends StatelessWidget {
594596
]),
595597
));
596598
}
597-
}
599+
}

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
@@ -556,6 +556,32 @@ void main() {
556556
debugNetworkImageHttpClientProvider = null;
557557
});
558558

559+
testWidgets('text emoji list items must have correct height', (tester) async {
560+
// Regression test for #1587.
561+
final message = eg.streamMessage();
562+
await setupEmojiPicker(tester, message: message, narrow: TopicNarrow.ofMessage(message));
563+
564+
// Find a list entry and get its height with the default image-based emoji.
565+
final listEntryFinder = find.byWidgetPredicate(
566+
(w) => w is EmojiPickerListEntry && w.emoji.emojiName == 'zzz');
567+
await tester.ensureVisible(listEntryFinder);
568+
final imageRenderBox = tester.renderObject<RenderBox>(listEntryFinder);
569+
final imageHeight = imageRenderBox.size.height;
570+
571+
// Change the setting to text-only emoji.
572+
await store.handleEvent(UserSettingsUpdateEvent(id: 1,
573+
property: UserSettingName.emojiset,
574+
value: Emojiset.text));
575+
await tester.pumpAndSettle();
576+
577+
// Find the same list entry again and check that its height has not changed.
578+
await tester.ensureVisible(listEntryFinder);
579+
final textRenderBox = tester.renderObject<RenderBox>(listEntryFinder);
580+
check(textRenderBox.size.height).isCloseTo(imageHeight, 1.0);
581+
582+
debugNetworkImageHttpClientProvider = null;
583+
});
584+
559585
testWidgets('with bottom view padding', (tester) async {
560586
await prepare(tester, viewPadding: FakeViewPadding(bottom: 10));
561587

0 commit comments

Comments
 (0)