Skip to content

Commit e2bd10c

Browse files
committed
autocomplete: Put best matches near input field.
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. Also mentions dependencies on #226 where required. Fixes #1121.
1 parent f2458b1 commit e2bd10c

File tree

2 files changed

+157
-0
lines changed

2 files changed

+157
-0
lines changed

lib/widgets/autocomplete.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ class _AutocompleteFieldState<QueryT extends AutocompleteQuery, ResultT extends
134134
constraints: const BoxConstraints(maxHeight: 300), // TODO not hard-coded
135135
child: ListView.builder(
136136
padding: EdgeInsets.zero,
137+
reverse: true,
137138
shrinkWrap: true,
138139
itemCount: _resultsToDisplay.length,
139140
itemBuilder: _buildItem))));

test/widgets/autocomplete_test.dart

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,71 @@ void main() {
177177

178178
debugNetworkImageHttpClientProvider = null;
179179
});
180+
181+
testWidgets('user options appear in the correct rendering order and do not scroll down', (tester) async {
182+
final List<User> users = [
183+
eg.user(userId: 1, fullName: 'Aaditya', avatarUrl: 'user1.png'),
184+
eg.user(userId: 2, fullName: 'Alya', avatarUrl: 'user2.png'),
185+
eg.user(userId: 3, fullName: 'Aman', avatarUrl: 'user3.png'),
186+
eg.user(userId: 4, fullName: 'Anders', avatarUrl: 'user4.png'),
187+
eg.user(userId: 5, fullName: 'Anthony', avatarUrl: 'user5.png'),
188+
eg.user(userId: 6, fullName: 'Apoorva', avatarUrl: 'user6.png'),
189+
eg.user(userId: 7, fullName: 'Asif', avatarUrl: 'user7.png'),
190+
eg.user(userId: 8, fullName: 'Asim', avatarUrl: 'user8.png')];
191+
192+
final composeInputFinder = await setupToComposeInput(tester, users: users);
193+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
194+
195+
// Options are filtered correctly for query
196+
// TODO(#226): Remove this extra edit when this bug is fixed.
197+
await tester.enterText(composeInputFinder, 'hello @');
198+
await tester.enterText(composeInputFinder, 'hello @A');
199+
await tester.pump();
200+
// Only first seven users render initially, 8th user has to be accessed by scrolling up
201+
final List<double> positions = [];
202+
for (int i = 0 ;i < 7 ;i++) {
203+
final user = users[i];
204+
checkUserShown(user, store, expected: true);
205+
positions.add(tester.getTopLeft(find.text(user.fullName)).dy);
206+
}
207+
for (int i = 7; i < users.length;i++) {
208+
final user = users[i];
209+
checkUserShown(user, store, expected: false);
210+
}
211+
212+
final listViewFinder = find.byType(ListView);
213+
await tester.drag(listViewFinder, const Offset(0, -50));
214+
await tester.pump();
215+
216+
final firstMentionPositionAfterDragDown = tester.getTopLeft(find.text(users.first.fullName)).dy;
217+
check(
218+
because:'ListView options should not scroll down because they are already at the bottom',
219+
positions[0]
220+
).equals(firstMentionPositionAfterDragDown);
221+
222+
for (int i = 0 ;i < 6;i++) {
223+
check(
224+
because: """${users[i + 1]} should appear above
225+
${users[i]} because of reverse order""",
226+
positions[i]).isGreaterThan(positions[i+1]);
227+
}
228+
229+
await tester.drag(listViewFinder, const Offset(0, 200)); // Should be capped at prev position
230+
await tester.pump();
231+
232+
checkUserShown(users.last, store, expected: true);
233+
checkUserShown(users.first, store, expected: false);
234+
235+
// 8th user should be above 7th user
236+
check(because: "8th user should be above 7th user",
237+
tester.getTopLeft(
238+
find.text(users.last.fullName)).dy
239+
).isLessThan(
240+
tester.getTopLeft(
241+
find.text(users[users.length - 2].fullName)
242+
).dy);
243+
debugNetworkImageHttpClientProvider = null;
244+
});
180245
});
181246

182247
group('emoji', () {
@@ -247,6 +312,97 @@ void main() {
247312
debugNetworkImageHttpClientProvider = null;
248313
});
249314

315+
testWidgets('emoji options appear in the correct rendering order and do not scroll down', (tester) async {
316+
final composeInputFinder = await setupToComposeInput(tester);
317+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
318+
319+
store.setServerEmojiData(
320+
ServerEmojiData(
321+
codeToNames: {
322+
'1f4a4': ['zzz', 'sleepy'], // Unicode emoji for "zzz"
323+
'1f52a': ['biohazard'],
324+
'1f92a': ['zany_face'],
325+
'1f993': ['zebra'],
326+
'0030-fe0f-20e3': ['zero'],
327+
'1f9d0': ['zombie'],
328+
}));
329+
330+
await store.handleEvent(
331+
RealmEmojiUpdateEvent(
332+
id: 1,
333+
realmEmoji: {
334+
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'buzzing')}));
335+
336+
const zulipOptionLabel = 'zulip';
337+
const zanyFaceOptionLabel = 'zany_face';
338+
const zebraOptionLabel = 'zebra';
339+
const zzzOptionLabel = 'zzz, sleepy';
340+
const unicodeGlyph = '💤';
341+
const zombieOptionLabel = 'zombie';
342+
const zeroOptionLabel = 'zero';
343+
const buzzingOptionLabel = 'buzzing';
344+
const biohazardOptionLabel = 'biohazard';
345+
346+
// Adjust the order so the best match appears last
347+
final emojiSequence = [
348+
zulipOptionLabel,
349+
zzzOptionLabel,
350+
unicodeGlyph,
351+
zanyFaceOptionLabel,
352+
zebraOptionLabel,
353+
zeroOptionLabel,
354+
zombieOptionLabel,
355+
buzzingOptionLabel,
356+
// biohazardOptionLabel, this won't be rendered in the list initally since it is the 7th option.
357+
];
358+
359+
// Enter a query; options appear, of all three emoji types.
360+
// TODO(#226): Remove this extra edit when this bug is fixed.
361+
await tester.enterText(composeInputFinder, 'hi :');
362+
await tester.enterText(composeInputFinder, 'hi :z');
363+
await tester.pump();
364+
365+
final positions = emojiSequence.map((icon) {
366+
final finder = find.text(icon);
367+
check(because:"Each emoji option should be rendered", finder).findsOne();
368+
return tester.getTopLeft(finder).dy;
369+
}).toList();
370+
371+
for (int i = 0; i < positions.length - 1; i++) {
372+
check(because: "${emojiSequence[i + 1]} should appear above ${emojiSequence[i]} because of reverse order",
373+
positions[i]).isGreaterThan(positions[i + 1]);
374+
}
375+
376+
final listViewFinder = find.byType(ListView);
377+
378+
await tester.drag(listViewFinder, const Offset(0, -50));
379+
await tester.pump();
380+
final firstEmojiPositionAfterScrollDown = tester.getTopLeft(find.text(emojiSequence[0])).dy;
381+
382+
check(
383+
because:"ListView options should not scroll down further than initial position",
384+
positions[0]
385+
).equals(firstEmojiPositionAfterScrollDown);
386+
387+
final biohazardFinder = find.text(biohazardOptionLabel);
388+
check(
389+
because: "The biohazard emoji should not be visible before scrolling up"
390+
,biohazardFinder
391+
).findsNothing();
392+
393+
// Scroll up
394+
await tester.drag(listViewFinder, const Offset(0, 50));
395+
await tester.pump();
396+
397+
check(because:"The biohazard emoji should be visible after scrolling up",biohazardFinder).findsOne();
398+
399+
final firstEmojiPositionAfterScrollUp = tester.getTopLeft(find.text(emojiSequence[0])).dy;
400+
check(because: "Scrolling up should reveal other emoji matches",firstEmojiPositionAfterScrollUp).isGreaterOrEqual(positions[0]);
401+
402+
debugNetworkImageHttpClientProvider = null;
403+
404+
});
405+
250406
testWidgets('text emoji means just show text', (tester) async {
251407
final composeInputFinder = await setupToComposeInput(tester);
252408
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

0 commit comments

Comments
 (0)