Skip to content

Commit f301858

Browse files
committed
emoji: Ensure consistent row height for text-only placeholders
Previously, text-only emoji placeholders in the picker and autocomplete lists used a Text widget. This caused inconsistent row heights when the system text scale factor was high, as the placeholder's height was determined by other font metrics. This new pr change replaces the Text placeholder with a simple SizedBox.square, which provides a fixed-size empty space ,this ensures the height of text-only items perfectly matches items with image emojis . Adds regression tests for both EmojiPickerListEntry and _EmojiAutocompleteItem to verify the consistent height and prevent future regressions. Fixes #1587.
1 parent 11ffac2 commit f301858

File tree

4 files changed

+82
-11
lines changed

4 files changed

+82
-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.square(dimension: _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
@@ -577,12 +577,12 @@ class EmojiPickerListEntry extends StatelessWidget {
577577

578578
// TODO deduplicate this logic with [_EmojiAutocompleteItem]
579579
final emojiDisplay = emoji.emojiDisplay.resolve(store.userSettings);
580-
final Widget? glyph = switch (emojiDisplay) {
580+
final Widget glyph = switch (emojiDisplay) {
581581
ImageEmojiDisplay() =>
582582
ImageEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
583583
UnicodeEmojiDisplay() =>
584584
UnicodeEmojiWidget(size: _emojiSize, emojiDisplay: emojiDisplay),
585-
TextEmojiDisplay() => null, // The text is already shown separately.
585+
TextEmojiDisplay() => const SizedBox.square(dimension: _emojiSize),
586586
};
587587

588588
final label = emoji.aliases.isEmpty
@@ -599,8 +599,7 @@ class EmojiPickerListEntry extends StatelessWidget {
599599
child: Padding(
600600
padding: const EdgeInsets.symmetric(horizontal: 8),
601601
child: Row(spacing: 4, children: [
602-
if (glyph != null)
603-
Padding(
602+
Padding(
604603
padding: const EdgeInsets.all(10),
605604
child: glyph),
606605
Flexible(child: Text(label,

test/widgets/autocomplete_test.dart

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,53 @@ 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+
tester.platformDispatcher.textScaleFactorTestValue = 2.0;
378+
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
379+
380+
final composeInputFinder = await setupToComposeInput(tester);
381+
382+
// Set up a realm emoji so we have a reliable image emoji to test against.
383+
await store.handleEvent(RealmEmojiUpdateEvent(id: 1, realmEmoji: {
384+
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'an_image_emoji'),
385+
}));
386+
await tester.pumpAndSettle(); // Ensure store update is fully processed.
387+
388+
// TODO(#226): Remove this extra edit when this bug is fixed.
389+
await tester.enterText(composeInputFinder, ':an_imag');
390+
await tester.enterText(composeInputFinder, ':an_image');
391+
await tester.pumpAndSettle();
392+
393+
final imageItemFinder = find.ancestor(
394+
of: find.text('an_image_emoji'),
395+
matching: find.byType(InkWell),
396+
);
397+
check(imageItemFinder).findsOne();
398+
final imageItemHeight = tester.getSize(imageItemFinder).height;
399+
400+
//TODO(#226): Remove this extra edit when this bug is fixed.
401+
await store.handleEvent(UserSettingsUpdateEvent(id: 1,
402+
property: UserSettingName.emojiset,
403+
value: Emojiset.text));
404+
405+
// Re-trigger autocomplete to have it rebuild with the new setting.
406+
await tester.enterText(composeInputFinder, ':an_imag');
407+
await tester.enterText(composeInputFinder, ':an_image');
408+
await tester.pumpAndSettle();
409+
410+
final textItemFinder = find.ancestor(
411+
of: find.text('an_image_emoji'),
412+
matching: find.byType(InkWell),
413+
);
414+
check(textItemFinder).findsOne();
415+
final textItemHeight = tester.getSize(textItemFinder).height;
416+
417+
check(textItemHeight).isCloseTo(imageItemHeight, 1.0);
418+
419+
debugNetworkImageHttpClientProvider = null;
420+
});
421+
375422
testWidgets('show, update, choose', (tester) async {
376423
final composeInputFinder = await setupToComposeInput(tester);
377424
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+
tester.platformDispatcher.textScaleFactorTestValue = 2.0;
562+
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
563+
564+
final message = eg.streamMessage();
565+
// Set up the emoji picker, which leaves it open.
566+
await setupEmojiPicker(tester, message: message, narrow: TopicNarrow.ofMessage(message));
567+
568+
// Now, change the setting to text-only emoji.
569+
await store.handleEvent(UserSettingsUpdateEvent(id: 1,
570+
property: UserSettingName.emojiset,
571+
value: Emojiset.text));
572+
// The picker is already open and will rebuild with the new setting.
573+
await tester.pumpAndSettle();
574+
575+
// Find a list entry and check its height.
576+
final listEntryFinder = find.byType(EmojiPickerListEntry).first;
577+
await tester.ensureVisible(listEntryFinder);
578+
579+
final renderBox = tester.renderObject<RenderBox>(listEntryFinder);
580+
check(renderBox.size.height).isLessThan(45.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)