client: improve emoji picker search #101

Manually merged
Johann150 merged 3 commits from emoji-picker into main 2022-09-03 14:39:10 +00:00
Owner

This can be merged as-is, it works quite well on several production instances (including MAB).

Remaining TODO: Levenshtein distance or similar "scoring" effect for sorting of the matches. I don't want to do it in this PR, so I mention it for future reference.

We can consider this PR to also be a test run for whether standard git trailers work in the automatic PR merge thing of gitea.

Changelog: Added

This can be merged as-is, it works quite well on several production instances (including MAB). Remaining TODO: Levenshtein distance or similar "scoring" effect for sorting of the matches. I don't want to do it in this PR, so I mention it for future reference. We can consider this PR to also be a test run for whether standard git trailers work in the automatic PR merge thing of gitea. Changelog: Added
toast added 1 commit 2022-09-01 13:09:20 +00:00
client: optimize, simplify and smartify emoji picker search
Some checks failed
ci/woodpecker/push/lint-foundkey-js Pipeline was successful
ci/woodpecker/push/lint-backend Pipeline was successful
ci/woodpecker/push/build Pipeline was successful
ci/woodpecker/push/lint-client Pipeline was successful
ci/woodpecker/push/test Pipeline was successful
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful
ci/woodpecker/pr/lint-backend Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/lint-client Pipeline failed
ci/woodpecker/pr/test Pipeline failed
538e81db56
The query is split up on spaces, and we search for each of those terms,
in order, anywhere in the emoji name or any aliases/keywords.

This is done in a single filter pass against a compiled regex,
making the process reasonably performant.
Based on rough estimates, it should be between 2 and 5x faster
than the old implementation, depending on several factors.

There is a natural space left in to sort by relevancy (not done yet).
It should also be easy to make the number of matches shown configurable.
The number of matches is relevant, especially pre-sort.
Another consideration is to delay the calculation by up to 300ms.
toast added 1 commit 2022-09-01 17:20:44 +00:00
client: make emoji picker suggestion count configurable
Some checks failed
ci/woodpecker/push/lint-foundkey-js Pipeline was successful
ci/woodpecker/push/lint-client Pipeline was successful
ci/woodpecker/push/build Pipeline was successful
ci/woodpecker/push/lint-backend Pipeline was successful
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful
ci/woodpecker/pr/lint-backend Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/push/test Pipeline was successful
ci/woodpecker/pr/lint-client Pipeline failed
ci/woodpecker/pr/test Pipeline failed
33ed6e98a7
toast added 1 commit 2022-09-01 17:39:59 +00:00
client: delay/batch emoji picker searches
Some checks failed
ci/woodpecker/push/lint-backend Pipeline was successful
ci/woodpecker/push/lint-client Pipeline was successful
ci/woodpecker/push/build Pipeline was successful
ci/woodpecker/push/lint-foundkey-js Pipeline was successful
ci/woodpecker/push/test Pipeline was successful
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful
ci/woodpecker/pr/lint-backend Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/lint-client Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ed8e346ff9
This is particularly important for users that set limit to 0 (unlimited).
norm approved these changes 2022-09-01 19:33:42 +00:00
norm left a comment
Owner

Works for me.

Works for me.
Johann150 reviewed 2022-09-02 07:08:07 +00:00
@ -130,0 +138,4 @@
|| emoji.keywords?.some(match) // unicode emoji
);
// TODO: sort matches by distance to query
if (max <= 0 || matches.length < max) return matches;
Owner

Checking for matches.length < max is not really necessary, since slice is specified to deal with cases where the 2nd argument is greater than the length but this works too.

Checking for `matches.length < max` is not really necessary, since `slice` is specified to deal with cases where the 2nd argument is greater than the length but this works too.
Author
Owner

I was getting some weird edge-cases with a trailing "undefined" element without it.
I can try removing it again if you'd like, but the cost of the check is basically zero relative to the rendering perf anyway.

I was getting some weird edge-cases with a trailing "undefined" element without it. I can try removing it again if you'd like, but the cost of the check is basically zero relative to the rendering perf anyway.
Johann150 marked this conversation as resolved
@ -36,2 +36,4 @@
<option value="quiet">{{ i18n.ts._serverDisconnectedBehavior.quiet }}</option>
</FormSelect>
<FormRange v-model="maxCustomEmojiPicker" :min="0" :max="24" :step="1" class="_formBlock">
Owner

If this defines a maximum, maybe the setting should also have a maximum (for which it would have to be a range element).

If this defines a maximum, maybe the setting should also have a maximum (for which it would have to be a range element).
Author
Owner

It doesn't - the maximum is actually 0 (unlimited). I set the max in the view because I can't imagine someone wanting >24 but <inf, given the practical implications of 6 rows.
It's basically like this so the slider is actually useful rather than, you know. Setting it to 999 or whatever.
Could make it a freeform entry but then I'd have to do parsing...
Nothing stopping a value of 25 though, it just wouldn't make much sense.
What do you think?

It doesn't - the maximum is actually 0 (unlimited). I set the max in the view because I can't imagine someone wanting >24 but <inf, given the practical implications of 6 rows. It's basically like this so the slider is actually useful rather than, you know. Setting it to 999 or whatever. Could make it a freeform entry but then I'd have to do parsing... Nothing stopping a value of 25 though, it just wouldn't make much sense. What do you think?
Johann150 marked this conversation as resolved
@ -38,0 +40,4 @@
<template #label>{{ i18n.ts.maxCustomEmojiPicker }}</template>
<template #caption>{{ i18n.ts.maxCustomEmojiPickerDescription }}</template>
</FormRange>
<FormRange v-model="maxUnicodeEmojiPicker" :min="0" :max="24" :step="1" class="_formBlock">
Owner

Same here of course.

Same here of course.
Johann150 marked this conversation as resolved
Johann150 approved these changes 2022-09-03 14:33:27 +00:00
Johann150 referenced this pull request from a commit 2022-09-03 14:39:06 +00:00
Johann150 manually merged commit 5e320e49ab into main 2022-09-03 14:39:10 +00:00
Johann150 deleted branch emoji-picker 2022-09-03 20:45:49 +00:00
Sign in to join this conversation.
No reviewers
No labels
feature
fix
upkeep
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#101
No description provided.