Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autocomplete: Put best matches near input field. #1131

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
151 changes: 151 additions & 0 deletions test/widgets/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,67 @@ void main() {

debugNetworkImageHttpClientProvider = null;
});

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

final composeInputFinder = await setupToComposeInput(tester, users:users);
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

final expectedUserSequence = users;
// 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
final List<double> positions = [];
for(int i = 0 ;i < 7 ;i++){
apoorvapendse marked this conversation as resolved.
Show resolved Hide resolved
final user = expectedUserSequence[i];
checkUserShown(user, store, expected: true);
final finder = find.text(user.fullName);
check(because:'Each user option should be rendered (index: $i)',finder).findsOne();
positions.add(tester.getTopLeft(finder).dy);
}
for(int i = 7; i < expectedUserSequence.length;i++){
final user = expectedUserSequence[i];
checkUserShown(user, store, expected: false);
}
final listViewFinder = find.byType(ListView);
check(because: "reason: 'ListView should be rendered'",listViewFinder,).findsOne();

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

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

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

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

checkUserShown(users.last, store, expected: true);
checkUserShown(users.first, store, expected: false);

// 8th user should be above 7th user
check(because: "8th user should be above 7th user",
tester.getTopLeft(find.text(users.last.fullName)).dy ).isLessThan(tester.getTopLeft(find.text(users[users.length - 2].fullName)).dy);
debugNetworkImageHttpClientProvider = null;
});
});

group('emoji', () {
Expand Down Expand Up @@ -247,6 +308,96 @@ 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 :');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a comment here if it's also affected by #226.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not understand?

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

final listViewFinder = find.byType(ListView);
check(because: "reason: 'ListView should be rendered'",listViewFinder,).findsOne();

final positions = emojiSequence.map((icon) {
final finder = find.text(icon);
check(because:"Each emoji option should be rendered", finder).findsOne();
return tester.getTopLeft(finder).dy;
}).toList();

for (int i = 0; i < positions.length - 1; i++) {
check(because: "${emojiSequence[i + 1]} should appear above ${emojiSequence[i]} because of reverse order",
positions[i]).isGreaterThan(positions[i + 1]);
}

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

check(because:"ListView should not scroll down because it is already at the bottom",
scrollOffsetAfterDragDown).equals(initialScrollOffset);

final biohazardFinder = find.text(biohazardOptionLabel);
check(because: "The biohazard emoji should not be visible before scrolling up",biohazardFinder).findsNothing();

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

check(because:"The biohazard emoji should be visible after scrolling up",biohazardFinder).findsOne();

final firstEmojiPositionAfterScrollUp = tester.getTopLeft(find.text(emojiSequence[0])).dy;
check(because: "Scrolling up should reveal other emoji matches",firstEmojiPositionAfterScrollUp).isGreaterOrEqual(positions[0]);

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