From 40f7941f2b581b5866423b7f9937f0cc735d7185 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 8 Jan 2025 22:10:50 +0100 Subject: [PATCH] FIX: emoji-picker minor improvements (#30645) - uses emojiSearch to ensure we have the same result than autocomplete when filtering emojis (for example search aliases were not working correctly because of this) - reset the visible sections when clearing filter to ensure we are not attempting to display all the emojis at once which would be slow - prevents flashing of the full emoji list before showing filtered results - correctly reset recent favorites and only show them when used --- .../app/components/emoji-picker/content.gjs | 204 +++++++++--------- .../discourse/app/services/emoji-store.js | 14 +- .../tests/fixtures/emojis-fixtures.js | 6 + .../components/emoji-picker-test.js | 4 +- .../tests/unit/services/emoji-store-test.js | 34 +-- .../chat-message-actions-desktop.gjs | 6 + 6 files changed, 134 insertions(+), 134 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/emoji-picker/content.gjs b/app/assets/javascripts/discourse/app/components/emoji-picker/content.gjs index 6d81cec5b49..e1f2e9f9582 100644 --- a/app/assets/javascripts/discourse/app/components/emoji-picker/content.gjs +++ b/app/assets/javascripts/discourse/app/components/emoji-picker/content.gjs @@ -7,6 +7,7 @@ import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import { cancel, next, schedule } from "@ember/runloop"; import { service } from "@ember/service"; import { modifier as modifierFn } from "ember-modifier"; +import { emojiSearch } from "pretty-text/emoji"; import { eq, gt, includes, notEq } from "truth-helpers"; import DButton from "discourse/components/d-button"; import FilterInput from "discourse/components/filter-input"; @@ -52,6 +53,7 @@ export default class EmojiPicker extends Component { @service capabilities; @service site; + @tracked isFiltering = false; @tracked filteredEmojis = null; @tracked scrollObserverEnabled = true; @tracked scrollDirection = "up"; @@ -152,9 +154,12 @@ export default class EmojiPicker extends Component { @action didInputFilter(value) { + this.isFiltering = true; if (!value?.length) { cancel(this.debouncedFilterHandler); + this.visibleSections = DEFAULT_VISIBLE_SECTIONS; this.filteredEmojis = null; + this.isFiltering = false; return; } @@ -174,13 +179,14 @@ export default class EmojiPicker extends Component { debouncedDidInputFilter(filter = "") { filter = filter.toLowerCase(); - this.filteredEmojis = this.flatEmojis.filter( - (emoji) => - emoji.name.toLowerCase().includes(filter) || - emoji.search_aliases?.any((alias) => - alias.toLowerCase().includes(filter) - ) - ); + const results = emojiSearch(filter, { + exclude: this.site.denied_emojis, + }).slice(0, 50); + + this.filteredEmojis = + this.flatEmojis.filter((emoji) => results.includes(emoji.name)) ?? []; + + this.isFiltering = false; schedule("afterRender", () => { if (this.scrollableNode) { @@ -450,7 +456,6 @@ export default class EmojiPicker extends Component { @filterAction={{withEventValue this.didInputFilter}} @icons={{hash right="magnifying-glass"}} @containerClass="emoji-picker__filter" - autofocus={{true}} placeholder={{i18n "chat.emoji_picker.search_placeholder"}} /> @@ -491,7 +496,8 @@ export default class EmojiPicker extends Component { {{/each-in}} - {{#if this.sections.length}} + + {{#if this.emojiStore.list}}
- {{#if (notEq this.filteredEmojis null)}} + {{#if this.term.length}}
{{#each this.filteredEmojis as |emoji|}} {{else}} -

- {{i18n "chat.emoji_picker.no_results"}} - {{replaceEmoji ":crying_cat_face:"}} -

+ {{#if this.isFiltering}} + + {{else}} +

+ {{i18n "chat.emoji_picker.no_results"}} + {{replaceEmoji ":crying_cat_face:"}} +

+ {{/if}} {{/each}}
+ {{else}} + {{#each-in this.groups as |section emojis|}} + {{#if emojis}} +
+
+

+ {{i18n + (concat "chat.emoji_picker." section) + translatedFallback=section + }} +

+ {{#if (eq section "favorites")}} + + {{/if}} +
+
+ {{! we always want the first emoji for tabbing}} + {{#let (get emojis "0") as |emoji|}} + {{emoji.name}} + {{/let}} + + {{#if (includes this.visibleSections section)}} + {{#each emojis as |emoji index|}} + {{! first emoji has already been rendered, we don't want to re render or would lose focus}} + {{#if (gt index 0)}} + {{emoji.name}} + {{/if}} + {{/each}} + {{/if}} +
+
+ {{/if}} + {{/each-in}} {{/if}} - - {{#each-in this.groups as |section emojis|}} - {{#if emojis}} -
-
-

- {{i18n - (concat "chat.emoji_picker." section) - translatedFallback=section - }} -

- {{#if (eq section "favorites")}} - - {{/if}} -
-
- {{! we always want the first emoji for tabbing}} - {{#let (get emojis "0") as |emoji|}} - {{emoji.name}} - {{/let}} - - {{#if (includes this.visibleSections section)}} - {{#each emojis as |emoji index|}} - {{! first emoji has already been rendered, we don't want to re render or would lose focus}} - {{#if (gt index 0)}} - {{emoji.name}} - {{/if}} - {{/each}} - {{/if}} -
-
- {{/if}} - {{/each-in}}
{{else}} diff --git a/app/assets/javascripts/discourse/app/services/emoji-store.js b/app/assets/javascripts/discourse/app/services/emoji-store.js index 4a36b67dc58..8ecaa0f6d7e 100644 --- a/app/assets/javascripts/discourse/app/services/emoji-store.js +++ b/app/assets/javascripts/discourse/app/services/emoji-store.js @@ -51,16 +51,16 @@ export default class EmojiStore extends Service { } resetContext(context) { - this.contexts[context] = this.#defaultEmojis; - this.#persistRecentEmojisForContext(this.#defaultEmojis, context); - } - - get #defaultEmojis() { - return this.siteSettings.default_emoji_reactions.split("|").filter(Boolean); + this.contexts[context] = []; + this.#persistRecentEmojisForContext([], context); } #recentEmojisForContext(context) { - return this.contexts[context] ?? this.#defaultEmojis; + return ( + this.contexts[context] ?? + this.store.getObject(this.#emojisStorekeyForContext(context)) ?? + [] + ); } #addEmojiToContext(emoji, context) { diff --git a/app/assets/javascripts/discourse/tests/fixtures/emojis-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/emojis-fixtures.js index 5d3c0e2ab4b..97acaed9eac 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/emojis-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/emojis-fixtures.js @@ -17,6 +17,12 @@ export default { group: "smileys_\u0026_emotion", search_aliases: ["smiley_cat", "star_struck"], }, + { + name: "smiley_cat", + tonable: false, + url: "/images/emoji/twitter/smiley_cat.png?v=12", + group: "smileys_\u0026_emotion", + }, ], "people_&_body": [ { diff --git a/app/assets/javascripts/discourse/tests/integration/components/emoji-picker-test.js b/app/assets/javascripts/discourse/tests/integration/components/emoji-picker-test.js index 05089268702..7fd3078f311 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/emoji-picker-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/emoji-picker-test.js @@ -70,7 +70,7 @@ module("Integration | Component | emoji-picker-content", function (hooks) { assert .dom(".emoji-picker__section.filtered > img") - .exists({ count: 1 }, "it filters the emojis list"); + .exists({ count: 2 }, "it filters the emojis list"); assert .dom('.emoji-picker__section.filtered > img[alt="grinning"]') .exists("it filters the correct emoji"); @@ -81,7 +81,7 @@ module("Integration | Component | emoji-picker-content", function (hooks) { .dom('.emoji-picker__section.filtered > img[alt="grinning"]') .exists("it is case insensitive"); - await fillIn(".filter-input", "smiley_cat"); + await fillIn(".filter-input", "grinning"); assert .dom('.emoji-picker__section.filtered > img[alt="grinning"]') diff --git a/app/assets/javascripts/discourse/tests/unit/services/emoji-store-test.js b/app/assets/javascripts/discourse/tests/unit/services/emoji-store-test.js index 621dd9f863b..562e18595f5 100644 --- a/app/assets/javascripts/discourse/tests/unit/services/emoji-store-test.js +++ b/app/assets/javascripts/discourse/tests/unit/services/emoji-store-test.js @@ -21,14 +21,6 @@ module("Unit | Service | emoji-store", function (hooks) { this.emojiStore.reset(); }); - test(".favoritesForContext", function (assert) { - assert.deepEqual(this.emojiStore.favoritesForContext("topic"), [ - "+1", - "heart", - "tada", - ]); - }); - test(".trackEmojiForContext", function (assert) { this.emojiStore.trackEmojiForContext("grinning", "topic"); const storedEmojis = new KeyValueStore(STORE_NAMESPACE).getObject( @@ -37,13 +29,10 @@ module("Unit | Service | emoji-store", function (hooks) { assert.deepEqual(this.emojiStore.favoritesForContext("topic"), [ "grinning", - "+1", - "heart", - "tada", ]); assert.deepEqual( storedEmojis, - ["grinning", "+1", "heart", "tada"], + ["grinning"], "it persists the tracked emojis" ); }); @@ -73,19 +62,11 @@ module("Unit | Service | emoji-store", function (hooks) { assert.deepEqual(this.emojiStore.favoritesForContext("topic"), [ "grinning", - "+1", - "heart", - "tada", ]); this.emojiStore.trackEmojiForContext("cat", "chat"); - assert.deepEqual(this.emojiStore.favoritesForContext("chat"), [ - "cat", - "+1", - "heart", - "tada", - ]); + assert.deepEqual(this.emojiStore.favoritesForContext("chat"), ["cat"]); }); test(".resetContext", function (assert) { @@ -93,11 +74,7 @@ module("Unit | Service | emoji-store", function (hooks) { this.emojiStore.resetContext("topic"); - assert.deepEqual(this.emojiStore.favoritesForContext("topic"), [ - "+1", - "heart", - "tada", - ]); + assert.deepEqual(this.emojiStore.favoritesForContext("topic"), []); }); test(".diversity", function (assert) { @@ -115,6 +92,7 @@ module("Unit | Service | emoji-store", function (hooks) { }); test("sort emojis by frequency", function (assert) { + this.emojiStore.trackEmojiForContext("grinning", "topic"); this.emojiStore.trackEmojiForContext("cat", "topic"); this.emojiStore.trackEmojiForContext("cat", "topic"); this.emojiStore.trackEmojiForContext("cat", "topic"); @@ -124,9 +102,7 @@ module("Unit | Service | emoji-store", function (hooks) { assert.deepEqual(this.emojiStore.favoritesForContext("topic"), [ "cat", "dog", - "+1", - "heart", - "tada", + "grinning", ]); }); }); diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs index 8c269d5540c..a5fb919a77c 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs @@ -28,6 +28,7 @@ const REDUCED_WIDTH_THRESHOLD = 500; export default class ChatMessageActionsDesktop extends Component { @service chat; @service site; + @service siteSettings; @service emojiStore; @tracked size = FULL; @@ -35,8 +36,13 @@ export default class ChatMessageActionsDesktop extends Component { popper = null; get favoriteReactions() { + const defaultReactions = this.siteSettings.default_emoji_reactions + .split("|") + .filter(Boolean); + return this.emojiStore .favoritesForContext(`channel_${this.message.channel.id}`) + .concat(defaultReactions) .slice(0, 3) .map( (emoji) =>