Skip to content

Commit 13c33b2

Browse files
apoorvapendsePIG208
andcommitted
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. Co-authored-by: Zixuan James Li <[email protected]> Fixes #1121.
1 parent 24215f6 commit 13c33b2

File tree

2 files changed

+91
-0
lines changed

2 files changed

+91
-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: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,36 @@ void main() {
177177

178178
debugNetworkImageHttpClientProvider = null;
179179
});
180+
181+
testWidgets('options are shown in reversed order', (tester) async {
182+
final users = List.generate(8, (i) => eg.user(fullName: 'A$i', avatarUrl: 'user$i.png'));
183+
final composeInputFinder = await setupToComposeInput(tester, users: users);
184+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
185+
186+
// TODO(#226): Remove this extra edit when this bug is fixed.
187+
await tester.enterText(composeInputFinder, 'hello @');
188+
await tester.enterText(composeInputFinder, 'hello @A');
189+
await tester.pump();
190+
191+
final initialPosition = tester.getTopLeft(find.text(users.first.fullName)).dy;
192+
// Initially, all but the last autocomplete options are visible.
193+
checkUserShown(users.last, store, expected: false);
194+
users.take(7).forEach((user) => checkUserShown(user, store, expected: true));
195+
196+
// Can't scroll down because the options grow from the bottom.
197+
await tester.drag(find.byType(ListView), const Offset(0, -50));
198+
await tester.pump();
199+
check(tester.getTopLeft(find.text(users.first.fullName)).dy)
200+
.equals(initialPosition);
201+
202+
// The last autocomplete option becomes visible after scrolling up.
203+
await tester.drag(find.byType(ListView), const Offset(0, 200));
204+
await tester.pump();
205+
users.skip(1).forEach((user) => checkUserShown(user, store, expected: true));
206+
checkUserShown(users.first, store, expected: false);
207+
208+
debugNetworkImageHttpClientProvider = null;
209+
});
180210
});
181211

182212
group('emoji', () {
@@ -247,6 +277,66 @@ void main() {
247277
debugNetworkImageHttpClientProvider = null;
248278
});
249279

280+
testWidgets('emoji options appear in the reverse order and do not scroll down', (tester) async {
281+
final composeInputFinder = await setupToComposeInput(tester);
282+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
283+
284+
store.setServerEmojiData(
285+
ServerEmojiData(
286+
codeToNames: {
287+
'1f4a4': ['zzz', 'sleepy'], // Unicode emoji for "zzz"
288+
'1f52a': ['biohazard'],
289+
'1f92a': ['zany_face'],
290+
'1f993': ['zebra'],
291+
'0030-fe0f-20e3': ['zero'],
292+
'1f9d0': ['zombie'],
293+
}));
294+
295+
await store.handleEvent(
296+
RealmEmojiUpdateEvent(
297+
id: 1,
298+
realmEmoji: {
299+
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'buzzing')}));
300+
301+
final emojiSequence = ['zulip', 'zany_face', 'zebra', 'zzz', '💤', 'zombie', 'zero', 'buzzing', 'biohazard'];
302+
303+
// Enter a query; options appear, of all three emoji types.
304+
// TODO(#226): Remove this extra edit when this bug is fixed.
305+
await tester.enterText(composeInputFinder, 'hi :');
306+
await tester.enterText(composeInputFinder, 'hi :z');
307+
await tester.pump();
308+
309+
final firstEmojiInitialPosition = tester.getTopLeft(find.text(emojiSequence[0]),warnIfMissed: true).dy;
310+
final listViewFinder = find.byType(ListView);
311+
312+
await tester.drag(listViewFinder, const Offset(0, -50));
313+
await tester.pump();
314+
final firstEmojiPositionAfterScrollDown = tester.getTopLeft(find.text(emojiSequence[0])).dy;
315+
check(
316+
because: "ListView options should not scroll down further than initial position",
317+
firstEmojiInitialPosition
318+
).equals(firstEmojiPositionAfterScrollDown);
319+
320+
check(
321+
because: "The last emoji should not be visible before scrolling up",
322+
find.text(emojiSequence.last)
323+
).findsNothing();
324+
325+
// Scroll up
326+
await tester.drag(listViewFinder, const Offset(0, 50));
327+
await tester.pump();
328+
329+
check(because: "The biohazard emoji should be visible after scrolling up",
330+
find.text(emojiSequence.last)).findsOne();
331+
332+
final firstEmojiPositionAfterScrollUp = tester.getTopLeft(find.text(emojiSequence[0])).dy;
333+
check(because: "Scrolling up should reveal other emoji matches",
334+
firstEmojiPositionAfterScrollUp).isGreaterOrEqual(firstEmojiInitialPosition);
335+
336+
debugNetworkImageHttpClientProvider = null;
337+
338+
});
339+
250340
testWidgets('text emoji means just show text', (tester) async {
251341
final composeInputFinder = await setupToComposeInput(tester);
252342
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

0 commit comments

Comments
 (0)