Skip to content

Commit 24826ef

Browse files
committed
emoji: Ensure consistent height for text-only emoji items.
Text-only emoji list items in the emoji picker and autocomplete views had incorrect row heights, causing visual inconsistencies. This was due to the placeholder not correctly accounting for the vertical padding applied by its parent widget, making text-only rows taller than rows with actual emojis. This commit fixes the issue by calculating the precise height needed for the placeholder to offset the parent padding. This ensures the final rendered height is identical for all emoji types. The change is applied in both and for consistency. Fixes #1587.
1 parent 6a35cc4 commit 24826ef

File tree

4 files changed

+71
-11
lines changed

4 files changed

+71
-11
lines changed

lib/widgets/autocomplete.dart

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,13 @@ class _EmojiAutocompleteItem extends StatelessWidget {
344344

345345
// TODO deduplicate this logic with [EmojiPickerListEntry]
346346
final emojiDisplay = candidate.emojiDisplay.resolve(store.userSettings);
347-
final Widget? glyph = switch (emojiDisplay) {
347+
final Widget glyph = switch (emojiDisplay) {
348348
ImageEmojiDisplay() =>
349349
ImageEmojiWidget(size: _size, emojiDisplay: emojiDisplay),
350350
UnicodeEmojiDisplay() =>
351351
UnicodeEmojiWidget(size: _size, emojiDisplay: emojiDisplay),
352-
TextEmojiDisplay() => null, // The text is already shown separately.
352+
// The text is already shown; just put a placeholder here to set the row height.
353+
TextEmojiDisplay() => const SizedBox(height: _size),
353354
};
354355

355356
final label = candidate.aliases.isEmpty
@@ -367,11 +368,9 @@ class _EmojiAutocompleteItem extends StatelessWidget {
367368
return Padding(
368369
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
369370
child: Row(children: [
370-
if (glyph != null) ...[
371-
Padding(padding: const EdgeInsets.all(6),
372-
child: glyph),
373-
const SizedBox(width: 6),
374-
],
371+
Padding(padding: const EdgeInsets.all(6),
372+
child: glyph),
373+
const SizedBox(width: 6),
375374
Expanded(
376375
child: Text(
377376
style: TextStyle(fontSize: 17, height: 18 / 17,

lib/widgets/emoji_reaction.dart

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -558,12 +558,12 @@ class EmojiPickerListEntry extends StatelessWidget {
558558

559559
// TODO deduplicate this logic with [_EmojiAutocompleteItem]
560560
final emojiDisplay = emoji.emojiDisplay.resolve(store.userSettings);
561-
final Widget? glyph = switch (emojiDisplay) {
561+
final Widget glyph = switch (emojiDisplay) {
562562
ImageEmojiDisplay() =>
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() => const SizedBox(height: _emojiSize),
567567
};
568568

569569
final label = emoji.aliases.isEmpty
@@ -580,8 +580,7 @@ class EmojiPickerListEntry extends StatelessWidget {
580580
child: Padding(
581581
padding: const EdgeInsets.symmetric(horizontal: 8),
582582
child: Row(spacing: 4, children: [
583-
if (glyph != null)
584-
Padding(
583+
Padding(
585584
padding: const EdgeInsets.all(10),
586585
child: glyph),
587586
Flexible(child: Text(label,

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)