Skip to content

Commit

Permalink
autocomplete: Put best matches near input field.
Browse files Browse the repository at this point in the history
This commit reverses the list that was originally
presented to the user while showing the typeahead menu.

This makes sense since on mobile its easier to click
on options closer to the input box, i.e.
where your fingers are currently present,
instead of pressing arrow keys on a keyboard which is
true on a desktop setup.

Hence we place the best matching options
not at the top of the typeahead menu, but
instead put them at the bottom for better
reachability and convenience of the user.

Tests have been added to verify the
emoji and mention render behavior.

Fixes #1123.
Fixes #1121.
  • Loading branch information
apoorvapendse committed Dec 16, 2024
1 parent 28b3536 commit bd3a205
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/widgets/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class _AutocompleteFieldState<QueryT extends AutocompleteQuery, ResultT extends
constraints: const BoxConstraints(maxHeight: 300), // TODO not hard-coded
child: ListView.builder(
padding: EdgeInsets.zero,
reverse: true,
shrinkWrap: true,
itemCount: _resultsToDisplay.length,
itemBuilder: _buildItem))));
Expand Down
166 changes: 166 additions & 0 deletions test/widgets/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,77 @@ void main() {

debugNetworkImageHttpClientProvider = null;
});

testWidgets('user options appear in the correct rendering order and do not scroll down', (tester) async {
final user1 = eg.user(userId: 1, fullName: 'Aaditya', avatarUrl: 'user1.png');
final user2 = eg.user(userId: 2, fullName: 'Alya', avatarUrl: 'user2.png');
final user3 = eg.user(userId: 3, fullName: 'Aman', avatarUrl: 'user3.png');
final user4 = eg.user(userId: 4, fullName: 'Anders', avatarUrl: 'user4.png');
final user5 = eg.user(userId: 5, fullName: 'Anthony', avatarUrl: 'user5.png');
final user6 = eg.user(userId: 6, fullName: 'Apoorva', avatarUrl: 'user6.png');
final user7 = eg.user(userId: 7, fullName: 'Asif', avatarUrl: 'user7.png');
final user8 = eg.user(userId: 8, fullName: 'Asim', avatarUrl: 'user8.png');

final composeInputFinder = await setupToComposeInput(tester, users: [user1,user2,user3,user4,user5,user6,user7,user8]);
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

final expectedUserSequence = [user1,user2,user3,user4,user5,user6,user7,user8];
// Options are filtered correctly for query
// TODO(#226): Remove this extra edit when this bug is fixed.
await tester.enterText(composeInputFinder, 'hello @');
await tester.pumpAndSettle();
await tester.enterText(composeInputFinder, 'hello @A');
await tester.pumpAndSettle();
//only first seven users render initially, 8th user has to be accessed by scrolling up
for(int i = 0 ;i < 7 ;i++){
final user = expectedUserSequence[i];
checkUserShown(user, store, expected: true);
}
for(int i = 7; i < expectedUserSequence.length;i++){
final user = expectedUserSequence[i];
checkUserShown(user, store, expected: false);
}
final listViewFinder = find.byType(ListView);
expect(listViewFinder, findsOneWidget, reason: 'ListView should be rendered');

final positions = expectedUserSequence.asMap().entries
.where((entry) {
final index = entry.key;
return index < 7; // Filter only visible users.
})
.map((entry) {
final index = entry.key;
final user = entry.value;
final finder = find.text(user.fullName);
expect(finder, findsOneWidget, reason: 'Each user option should be rendered (index: $index)');
return tester.getTopLeft(finder).dy;
}).toList();

final initialScrollOffset = tester.getTopLeft(listViewFinder).dy;
await tester.drag(listViewFinder, const Offset(0, -50));
await tester.pumpAndSettle();
final scrollOffsetAfterDragDown = tester.getTopLeft(listViewFinder).dy;

expect(scrollOffsetAfterDragDown, initialScrollOffset,
reason: 'ListView should not scroll down because it is already at the bottom');

for(int i = 0 ;i < 6;i++){
expect(positions[i] > positions[i + 1], isTrue,
reason: '${expectedUserSequence[i + 1]} should appear above ${expectedUserSequence[i]} because of reverse order');
}

await tester.drag(listViewFinder, const Offset(0, 200)); //should be capped at prev position
await tester.pumpAndSettle();

checkUserShown(user8, store, expected: true);
checkUserShown(user1, store, expected: false);

//8th user should be above 7th user
expect(tester.getTopLeft(find.text(user8.fullName)).dy < tester.getTopLeft(find.text(user7.fullName)).dy,
isTrue, reason:"8th user should be above 7th user");

debugNetworkImageHttpClientProvider = null;
});
});

group('emoji', () {
Expand Down Expand Up @@ -247,6 +318,101 @@ void main() {
debugNetworkImageHttpClientProvider = null;
});

testWidgets('emoji options appear in the correct rendering order and do not scroll down', (tester) async {
final composeInputFinder = await setupToComposeInput(tester);
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

store.setServerEmojiData(
ServerEmojiData(
codeToNames: {
'1f4a4': ['zzz', 'sleepy'], // Unicode emoji for "zzz"
'1f52a': ['biohazard'],
'1f92a': ['zany_face'],
'1f993': ['zebra'],
'0030-fe0f-20e3': ['zero'],
'1f9d0': ['zombie'],
},
),
);
await store.handleEvent(
RealmEmojiUpdateEvent(
id: 1,
realmEmoji: {
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'buzzing'),
},
),
);

const zulipOptionLabel = 'zulip';
const zanyFaceOptionLabel = 'zany_face';
const zebraOptionLabel = 'zebra';
const zzzOptionLabel = 'zzz, sleepy';
const unicodeGlyph = '💤';
const zombieOptionLabel = 'zombie';
const zeroOptionLabel = 'zero';
const buzzingOptionLabel = 'buzzing';
const biohazardOptionLabel = 'biohazard';

// Adjust the order so the best match appears last
final emojiSequence = [
zulipOptionLabel,
zzzOptionLabel,
unicodeGlyph,
zanyFaceOptionLabel,
zebraOptionLabel,
zeroOptionLabel,
zombieOptionLabel,
buzzingOptionLabel,
// biohazardOptionLabel, this won't be rendered in the list initally since it is the 7th option.
];

await tester.enterText(composeInputFinder, 'hi :');
await tester.enterText(composeInputFinder, 'hi :z');
await tester.pumpAndSettle();

final listViewFinder = find.byType(ListView);
expect(listViewFinder, findsOneWidget, reason: 'ListView should be rendered');

final positions = emojiSequence.map((icon) {
final finder = find.text(icon);
expect(finder, findsOneWidget, reason: 'Each emoji option should be rendered');
return tester.getTopLeft(finder).dy;
}).toList();

for (int i = 0; i < positions.length - 1; i++) {
expect(positions[i] > positions[i + 1], isTrue,
reason: '${emojiSequence[i + 1]} should appear above ${emojiSequence[i]} because of reverse order');
}

final initialScrollOffset = tester.getTopLeft(listViewFinder).dy;
await tester.drag(listViewFinder, const Offset(0, -50));
await tester.pumpAndSettle();
final scrollOffsetAfterDragDown = tester.getTopLeft(listViewFinder).dy;

expect(scrollOffsetAfterDragDown, initialScrollOffset,
reason: 'ListView should not scroll down because it is already at the bottom');

final biohazardFinder = find.text(biohazardOptionLabel);
expect(biohazardFinder, findsNothing, reason: 'The biohazard emoji should not be visible before scrolling up');

// Scroll up
await tester.drag(listViewFinder, const Offset(0, 50));
await tester.pumpAndSettle();

expect(biohazardFinder, findsOneWidget, reason: 'The biohazard emoji should be visible after scrolling up');

final firstEmojiPositionAfterScrollUp = tester.getTopLeft(find.text(emojiSequence[0])).dy;
// print(firstEmojiPositionAfterScrollUp);
// print(positions[0]);
expect(firstEmojiPositionAfterScrollUp >= positions[0], isTrue,
reason: 'Scrolling up should reveal other emoji matches');

expect(tester.getTopLeft(find.text(biohazardOptionLabel)).runtimeType,const Offset(1,2).runtimeType, reason:"7th best result should only be visible on scrolling up");

debugNetworkImageHttpClientProvider = null;

});

testWidgets('text emoji means just show text', (tester) async {
final composeInputFinder = await setupToComposeInput(tester);
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
Expand Down

0 comments on commit bd3a205

Please sign in to comment.