paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 05:53:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

typeahead: Rework sort_emojis function.

Browse Source 
When `sort_emojis` function was called from emoji_picker
module, the passed arguments did not contain `reaction_type`
field. As a result the first conditional of `is_popular` function
inside `sort_emojis` always failed -- hence the array
`popular_emoji_matches` was always empty`[]`. This compromised
search especially the order of filtered emojis.

Instead of checking for `reaction_type` === "unicode_emoji" -- we
check `is_realm_emoji` field is false. Since `is_realm_emoji`
field in always present and also results in easier types, this
should be prefered over adding `reaction_type` field to the passed
arguments.

Fixes zulip#30439
This commit is contained in:
Varun Singh
2024-06-24 23:54:18 +05:30
committed by Tim Abbott
parent 99a80639f4
commit a7b3dbfdd7
3 changed files with 29 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
web/shared/src/typeahead.ts
View File

@@ -52,6 +52,11 @@ export type EmojiSuggestion = Emoji & {
type: "emoji";
};
export type BaseEmoji = {emoji_name: string} & (
| {is_realm_emoji: false; emoji_code: string}
| {is_realm_emoji: true; emoji_code?: undefined}
);
export function remove_diacritics(s: string): string {
return s.normalize("NFKD").replace(unicode_marks, "");
}
@@ -314,7 +319,7 @@ export function triage<T>(
};
}
export function sort_emojis<T extends EmojiSuggestion>(objs: T[], query: string): T[] {
export function sort_emojis<T extends BaseEmoji>(objs: T[], query: string): T[] {
// replace spaces with underscores for emoji matching
query = query.replace(/ /g, "_");
query = query.toLowerCase();
@@ -326,11 +331,9 @@ export function sort_emojis<T extends EmojiSuggestion>(objs: T[], query: string)
const popular_set = new Set(popular_emojis);
function is_popular(obj: EmojiSuggestion): boolean {
function is_popular(obj: BaseEmoji): boolean {
return (
obj.reaction_type === "unicode_emoji" &&
popular_set.has(obj.emoji_code) &&
decent_match(obj.emoji_name)
!obj.is_realm_emoji && popular_set.has(obj.emoji_code) && decent_match(obj.emoji_name)
);
}
@@ -364,7 +367,7 @@ export function sort_emojis<T extends EmojiSuggestion>(objs: T[], query: string)
const unicode_emoji_codes = new Set();
const sorted_unique_results: T[] = [];
for (const emoji of sorted_results_with_possible_duplicates) {
if (emoji.reaction_type !== "unicode_emoji") {
if (emoji.is_realm_emoji) {
sorted_unique_results.push(emoji);
} else if (
!unicode_emoji_codes.has(emoji.emoji_code) &&

1
web/tests/composebox_typeahead.test.js
View File

@@ -459,6 +459,7 @@ const make_emoji = (emoji_dict) => ({
emoji_name: emoji_dict.name,
emoji_code: emoji_dict.emoji_code,
reaction_type: "unicode_emoji",
is_realm_emoji: false,
type: "emoji",
});

40
web/tests/typeahead.test.js
View File

@@ -176,10 +176,10 @@ function sort_emojis(emojis, query) {
run_test("sort_emojis: th", () => {
const emoji_list = [
{emoji_name: "mother_nature", reaction_type: "realm_emoji"},
{emoji_name: "thermometer", reaction_type: "realm_emoji"},
{emoji_name: "thumbs_down", reaction_type: "realm_emoji"},
{emoji_name: "thumbs_up", reaction_type: "unicode_emoji", emoji_code: "1f44d"},
{emoji_name: "mother_nature", is_realm_emoji: true},
{emoji_name: "thermometer", is_realm_emoji: true},
{emoji_name: "thumbs_down", is_realm_emoji: true},
{emoji_name: "thumbs_up", is_realm_emoji: false, emoji_code: "1f44d"},
];
assert.deepEqual(sort_emojis(emoji_list, "th"), [
"thumbs_up",
@@ -191,9 +191,9 @@ run_test("sort_emojis: th", () => {
run_test("sort_emojis: sm", () => {
const emoji_list = [
{emoji_name: "big_smile", reaction_type: "realm_emoji"},
{emoji_name: "slight_smile", reaction_type: "unicode_emoji", emoji_code: "1f642"},
{emoji_name: "small_airplane", reaction_type: "realm_emoji"},
{emoji_name: "big_smile", is_realm_emoji: true},
{emoji_name: "slight_smile", is_realm_emoji: false, emoji_code: "1f642"},
{emoji_name: "small_airplane", is_realm_emoji: true},
];
assert.deepEqual(sort_emojis(emoji_list, "sm"), [
"slight_smile",
@@ -204,9 +204,9 @@ run_test("sort_emojis: sm", () => {
run_test("sort_emojis: SM", () => {
const emoji_list = [
{emoji_name: "big_smile", reaction_type: "realm_emoji"},
{emoji_name: "slight_smile", reaction_type: "unicode_emoji", emoji_code: "1f642"},
{emoji_name: "small_airplane", reaction_type: "realm_emoji"},
{emoji_name: "big_smile", is_realm_emoji: true},
{emoji_name: "slight_smile", is_realm_emoji: false, emoji_code: "1f642"},
{emoji_name: "small_airplane", is_realm_emoji: true},
];
assert.deepEqual(sort_emojis(emoji_list, "SM"), [
"slight_smile",
@@ -217,8 +217,8 @@ run_test("sort_emojis: SM", () => {
run_test("sort_emojis: prefix before midphrase, with underscore (traffic_li)", () => {
const emoji_list = [
{emoji_name: "horizontal_traffic_light", reaction_type: "realm_emoji"},
{emoji_name: "traffic_light", reaction_type: "realm_emoji"},
{emoji_name: "horizontal_traffic_light", is_realm_emoji: true},
{emoji_name: "traffic_light", is_realm_emoji: true},
];
assert.deepEqual(sort_emojis(emoji_list, "traffic_li"), [
"traffic_light",
@@ -228,8 +228,8 @@ run_test("sort_emojis: prefix before midphrase, with underscore (traffic_li)", (
run_test("sort_emojis: prefix before midphrase, with space (traffic li)", () => {
const emoji_list = [
{emoji_name: "horizontal_traffic_light", reaction_type: "realm_emoji"},
{emoji_name: "traffic_light", reaction_type: "realm_emoji"},
{emoji_name: "horizontal_traffic_light", is_realm_emoji: true},
{emoji_name: "traffic_light", is_realm_emoji: true},
];
assert.deepEqual(sort_emojis(emoji_list, "traffic li"), [
"traffic_light",
@@ -240,21 +240,20 @@ run_test("sort_emojis: prefix before midphrase, with space (traffic li)", () =>
run_test("sort_emojis: remove duplicates", () => {
// notice the last 2 are aliases of the same emoji (same emoji code)
const emoji_list = [
{emoji_name: "laughter_tears", emoji_code: "1f602", reaction_type: "unicode_emoji"},
{emoji_name: "tear", emoji_code: "1f972", reaction_type: "unicode_emoji"},
{emoji_name: "smile_with_tear", emoji_code: "1f972", reaction_type: "unicode_emoji"},
{emoji_name: "laughter_tears", emoji_code: "1f602", is_realm_emoji: false},
{emoji_name: "tear", emoji_code: "1f972", is_realm_emoji: false},
{emoji_name: "smile_with_tear", emoji_code: "1f972", is_realm_emoji: false},
];
assert.deepEqual(typeahead.sort_emojis(emoji_list, "tear"), [emoji_list[1], emoji_list[0]]);
});
run_test("sort_emojis: prioritise realm emojis", () => {
const emoji_list = [
{emoji_name: "thank_you", emoji_code: "1f64f", reaction_type: "unicode_emoji"},
{emoji_name: "thank_you", emoji_code: "1f64f", is_realm_emoji: false},
{
emoji_name: "thank_you_custom",
url: "something",
is_realm_emoji: true,
reaction_type: "realm_emoji",
},
];
assert.deepEqual(typeahead.sort_emojis(emoji_list, "thank"), [emoji_list[1], emoji_list[0]]);
@@ -262,12 +261,11 @@ run_test("sort_emojis: prioritise realm emojis", () => {
run_test("sort_emojis: prioritise perfect matches", () => {
const emoji_list = [
{emoji_name: "thank_you", emoji_code: "1f64f", reaction_type: "unicode_emoji"},
{emoji_name: "thank_you", emoji_code: "1f64f", is_realm_emoji: false},
{
emoji_name: "thank_you_custom",
url: "something",
is_realm_emoji: true,
reaction_type: "realm_emoji",
},
];
assert.deepEqual(typeahead.sort_emojis(emoji_list, "thank you"), emoji_list);