Skip to content

Commit 7045afe

Browse files
committed
emoji: Order "popular" emoji canonically amongst themselves
As a bonus, this provides the popular emoji as candidates even when we haven't yet fetched the server's emoji data.
1 parent a885520 commit 7045afe

File tree

2 files changed

+100
-1
lines changed

2 files changed

+100
-1
lines changed

lib/model/emoji.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ class EmojiStoreImpl with EmojiStore {
295295
// but then ranks them first, after only the six "popular" emoji,
296296
// once there's a non-empty query.
297297
// * Web gives the six "popular" emoji a set order amongst themselves,
298-
// like we do after #1112; but in web, this order appears only in the
298+
// like we do here; but in web, this order appears only in the
299299
// emoji picker on an empty query, and is otherwise lost even when the
300300
// emoji are taken out of their home categories and shown instead
301301
// together at the front.
@@ -307,12 +307,18 @@ class EmojiStoreImpl with EmojiStore {
307307

308308
final results = <EmojiCandidate>[];
309309

310+
// Include the "popular" emoji, in their canonical order
311+
// relative to each other.
312+
results.addAll(_popularCandidates);
313+
310314
final namesOverridden = {
311315
for (final emoji in activeRealmEmoji) emoji.name,
312316
'zulip',
313317
};
314318
// TODO(log) if _serverEmojiData missing
315319
for (final entry in (_serverEmojiData ?? {}).entries) {
320+
if (_popularEmojiCodes.contains(entry.key)) continue;
321+
316322
final allNames = entry.value;
317323
final String emojiName;
318324
final List<String>? aliases;

test/model/emoji_test.dart

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ void main() {
7878
});
7979
});
8080

81+
final popularCandidates = EmojiStore.popularEmojiCandidates;
82+
8183
Condition<Object?> isUnicodeCandidate(String? emojiCode, List<String>? names) {
8284
return (it_) {
8385
final it = it_.isA<EmojiCandidate>();
@@ -108,6 +110,9 @@ void main() {
108110
..aliases.isEmpty();
109111
}
110112

113+
List<Condition<Object?>> arePopularCandidates = popularCandidates.map(
114+
(c) => isUnicodeCandidate(c.emojiCode, null)).toList();
115+
111116
group('allEmojiCandidates', () {
112117
// TODO test emojiDisplay of candidates matches emojiDisplayFor
113118

@@ -123,12 +128,47 @@ void main() {
123128
return store;
124129
}
125130

131+
test('popular emoji appear even when no server emoji data', () {
132+
final store = prepare(unicodeEmoji: null);
133+
check(store.allEmojiCandidates()).deepEquals([
134+
...arePopularCandidates,
135+
isZulipCandidate(),
136+
]);
137+
});
138+
139+
test('popular emoji appear in their canonical order', () {
140+
// In the server's emoji data, have the popular emoji in a permuted order,
141+
// and interspersed with other emoji.
142+
final store = prepare(unicodeEmoji: {
143+
'1f603': ['smiley'],
144+
for (final candidate in popularCandidates.skip(3))
145+
candidate.emojiCode: [candidate.emojiName, ...candidate.aliases],
146+
'1f34a': ['orange', 'tangerine', 'mandarin'],
147+
for (final candidate in popularCandidates.take(3))
148+
candidate.emojiCode: [candidate.emojiName, ...candidate.aliases],
149+
'1f516': ['bookmark'],
150+
});
151+
// In the allEmojiCandidates result, the popular emoji come first
152+
// and are in their canonical order, even though the other Unicode emoji
153+
// are in the same order they were given in.
154+
check(store.allEmojiCandidates()).deepEquals([
155+
for (final candidate in popularCandidates)
156+
isUnicodeCandidate(candidate.emojiCode,
157+
[candidate.emojiName, ...candidate.aliases]),
158+
isUnicodeCandidate('1f603', ['smiley']),
159+
isUnicodeCandidate('1f34a', ['orange', 'tangerine', 'mandarin']),
160+
isUnicodeCandidate('1f516', ['bookmark']),
161+
isZulipCandidate(),
162+
]);
163+
});
164+
126165
test('realm emoji included only when active', () {
127166
final store = prepare(realmEmoji: {
128167
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'abc', deactivated: true),
129168
'2': eg.realmEmojiItem(emojiCode: '2', emojiName: 'abcd'),
130169
});
131170
check(store.allEmojiCandidates()).deepEquals([
171+
...arePopularCandidates,
132172
isRealmCandidate(emojiCode: '2', emojiName: 'abcd'),
133173
isZulipCandidate(),
134174
]);
@@ -143,6 +183,7 @@ void main() {
143183
'5': eg.realmEmojiItem(emojiCode: '5', emojiName: 'test', deactivated: true),
144184
});
145185
check(store.allEmojiCandidates()).deepEquals([
186+
...arePopularCandidates,
146187
isRealmCandidate(emojiCode: '4', emojiName: 'try'),
147188
isZulipCandidate(),
148189
]);
@@ -156,6 +197,7 @@ void main() {
156197
'1f603': ['smiley'],
157198
});
158199
check(store.allEmojiCandidates()).deepEquals([
200+
...arePopularCandidates,
159201
isUnicodeCandidate('1f516', ['bookmark']),
160202
isRealmCandidate(emojiCode: '1', emojiName: 'smiley'),
161203
isZulipCandidate(),
@@ -169,6 +211,7 @@ void main() {
169211
'1f41c': ['ant'],
170212
});
171213
check(store.allEmojiCandidates()).deepEquals([
214+
...arePopularCandidates,
172215
isUnicodeCandidate('1f41c', ['ant']),
173216
isZulipCandidate(),
174217
]);
@@ -181,6 +224,7 @@ void main() {
181224
'1f34a': ['orange', 'tangerine', 'mandarin'],
182225
});
183226
check(store.allEmojiCandidates()).deepEquals([
227+
...arePopularCandidates,
184228
isUnicodeCandidate('1f34a', ['orange', 'mandarin']),
185229
isRealmCandidate(emojiCode: '1', emojiName: 'tangerine'),
186230
isZulipCandidate(),
@@ -194,6 +238,7 @@ void main() {
194238
'1f34a': ['orange', 'tangerine', 'mandarin'],
195239
});
196240
check(store.allEmojiCandidates()).deepEquals([
241+
...arePopularCandidates,
197242
isUnicodeCandidate('1f34a', ['tangerine', 'mandarin']),
198243
isRealmCandidate(emojiCode: '1', emojiName: 'orange'),
199244
isZulipCandidate(),
@@ -203,13 +248,15 @@ void main() {
203248
test('updates on setServerEmojiData', () {
204249
final store = prepare();
205250
check(store.allEmojiCandidates()).deepEquals([
251+
...arePopularCandidates,
206252
isZulipCandidate(),
207253
]);
208254

209255
store.setServerEmojiData(ServerEmojiData(codeToNames: {
210256
'1f516': ['bookmark'],
211257
}));
212258
check(store.allEmojiCandidates()).deepEquals([
259+
...arePopularCandidates,
213260
isUnicodeCandidate('1f516', ['bookmark']),
214261
isZulipCandidate(),
215262
]);
@@ -218,13 +265,15 @@ void main() {
218265
test('updates on RealmEmojiUpdateEvent', () {
219266
final store = prepare();
220267
check(store.allEmojiCandidates()).deepEquals([
268+
...arePopularCandidates,
221269
isZulipCandidate(),
222270
]);
223271

224272
store.handleEvent(RealmEmojiUpdateEvent(id: 1, realmEmoji: {
225273
'1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'happy'),
226274
}));
227275
check(store.allEmojiCandidates()).deepEquals([
276+
...arePopularCandidates,
228277
isRealmCandidate(emojiCode: '1', emojiName: 'happy'),
229278
isZulipCandidate(),
230279
]);
@@ -257,6 +306,9 @@ void main() {
257306
isZulipCandidate());
258307
}
259308

309+
List<Condition<Object?>> arePopularResults = popularCandidates.map(
310+
(c) => isUnicodeResult(emojiCode: c.emojiCode)).toList();
311+
260312
PerAccountStore prepare({
261313
Map<String, String> realmEmoji = const {},
262314
Map<String, List<String>>? unicodeEmoji,
@@ -282,6 +334,7 @@ void main() {
282334
await Future(() {});
283335
check(done).isTrue();
284336
check(view.results).deepEquals([
337+
...arePopularResults,
285338
isRealmResult(emojiName: 'happy'),
286339
isZulipResult(),
287340
isUnicodeResult(names: ['bookmark']),
@@ -323,6 +376,45 @@ void main() {
323376
return view.results;
324377
}
325378

379+
test('results preserve order of popular emoji within each rank', () async {
380+
// In other words, the sorting by rank is a stable sort.
381+
382+
// Full results list matches allEmojiCandidates.
383+
check(prepare().allEmojiCandidates())
384+
.deepEquals([...arePopularCandidates, isZulipCandidate()]);
385+
check(await resultsOf(''))
386+
.deepEquals([...arePopularResults, isZulipResult()]);
387+
388+
// Same list written out explicitly, for comparison with the cases below.
389+
check(await resultsOf('')).deepEquals([
390+
isUnicodeResult(names: ['+1', 'thumbs_up', 'like']),
391+
isUnicodeResult(names: ['tada']),
392+
isUnicodeResult(names: ['smile']),
393+
isUnicodeResult(names: ['heart', 'love', 'love_you']),
394+
isUnicodeResult(names: ['working_on_it', 'hammer_and_wrench', 'tools']),
395+
isUnicodeResult(names: ['octopus']),
396+
isZulipResult(),
397+
]);
398+
399+
check(await resultsOf('t')).deepEquals([
400+
// prefix
401+
isUnicodeResult(names: ['+1', 'thumbs_up', 'like']),
402+
isUnicodeResult(names: ['tada']),
403+
isUnicodeResult(names: ['working_on_it', 'hammer_and_wrench', 'tools']),
404+
// other
405+
isUnicodeResult(names: ['heart', 'love', 'love_you']),
406+
isUnicodeResult(names: ['octopus']),
407+
]);
408+
409+
check(await resultsOf('h')).deepEquals([
410+
// prefix
411+
isUnicodeResult(names: ['heart', 'love', 'love_you']),
412+
isUnicodeResult(names: ['working_on_it', 'hammer_and_wrench', 'tools']),
413+
// other
414+
isUnicodeResult(names: ['+1', 'thumbs_up', 'like']),
415+
]);
416+
});
417+
326418
test('results end-to-end', () async {
327419
// (See more detailed rank tests below, on EmojiAutocompleteQuery.)
328420

@@ -331,6 +423,7 @@ void main() {
331423

332424
// Empty query -> base ordering.
333425
check(await resultsOf('', unicodeEmoji: unicodeEmoji)).deepEquals([
426+
...arePopularResults,
334427
isZulipResult(),
335428
isUnicodeResult(names: ['notebook']),
336429
isUnicodeResult(names: ['bookmark']),

0 commit comments

Comments
 (0)