From 44f6b24e349b35150247e450a4c4178abd45dabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 14 Mar 2024 09:31:49 +0100 Subject: [PATCH] FIX: clicking "more..." in emoji autocomplete (#26160) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should open the emoji picker. But it wasn't 😅 The `handleOutsideClick` event was listening too early and would catch the click on the "more..." option in the autocomplete as a click outside the emoji picker and would immediately close it 🤦 The fix was to defer registering to this event. --- .../discourse/app/components/emoji-picker.js | 63 ++++++++----------- .../discourse/tests/acceptance/emoji-test.js | 30 ++++++--- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/emoji-picker.js b/app/assets/javascripts/discourse/app/components/emoji-picker.js index 5fd56992c62..c02e2e9be4f 100644 --- a/app/assets/javascripts/discourse/app/components/emoji-picker.js +++ b/app/assets/javascripts/discourse/app/components/emoji-picker.js @@ -19,14 +19,10 @@ import discourseComputed, { } from "discourse-common/utils/decorators"; function customEmojis() { - const list = extendedEmojiList(); const groups = []; - for (const [code, emoji] of list.entries()) { - groups[emoji.group] = groups[emoji.group] || []; - groups[emoji.group].push({ - code, - src: emojiUrlFor(code), - }); + for (const [code, emoji] of extendedEmojiList()) { + groups[emoji.group] ||= []; + groups[emoji.group].push({ code, src: emojiUrlFor(code) }); } return groups; } @@ -48,9 +44,7 @@ export default Component.extend({ init() { this._super(...arguments); - this.set("customEmojis", customEmojis()); - if ("IntersectionObserver" in window) { this._sectionObserver = this._setupSectionObserver(); } @@ -58,7 +52,6 @@ export default Component.extend({ didInsertElement() { this._super(...arguments); - this.appEvents.on("emoji-picker:close", this, "onClose"); }, @@ -83,9 +76,7 @@ export default Component.extend({ willDestroyElement() { this._super(...arguments); - - this._sectionObserver && this._sectionObserver.disconnect(); - + this._sectionObserver?.disconnect(); this.appEvents.off("emoji-picker:close", this, "onClose"); }, @@ -95,7 +86,6 @@ export default Component.extend({ schedule("afterRender", () => { this._applyFilter(this.initialFilter); - document.addEventListener("click", this.handleOutsideClick); const emojiPicker = document.querySelector(".emoji-picker"); if (!emojiPicker) { @@ -147,9 +137,10 @@ export default Component.extend({ // of blocking the rendering of the picker discourseLater(() => { schedule("afterRender", () => { + document.addEventListener("click", this.handleOutsideClick); + if (!this.site.isMobileDevice || this.isEditorFocused) { - const filter = emojiPicker.querySelector("input.filter"); - filter && filter.focus(); + emojiPicker.querySelector("input.filter")?.focus(); if (this._sectionObserver) { emojiPicker @@ -170,7 +161,7 @@ export default Component.extend({ onClose(event) { event?.stopPropagation(); document.removeEventListener("click", this.handleOutsideClick); - this.onEmojiPickerClose && this.onEmojiPickerClose(event); + this.onEmojiPickerClose?.(event); }, diversityScales: computed("selectedDiversity", function () { @@ -239,10 +230,11 @@ export default Component.extend({ @action onCategorySelection(sectionName, event) { event?.preventDefault(); - const section = document.querySelector( - `.emoji-picker-emoji-area .section[data-section="${sectionName}"]` - ); - section && section.scrollIntoView(); + document + .querySelector( + `.emoji-picker-emoji-area .section[data-section="${sectionName}"]` + ) + ?.scrollIntoView(); }, @action @@ -264,7 +256,7 @@ export default Component.extend({ if (event.key === "Escape") { this.onClose(event); - const path = event.path || (event.composedPath && event.composedPath()); + const path = event.path || event.composedPath?.(); const fromChatComposer = path.find((e) => e?.classList?.contains("chat-composer-container") @@ -325,8 +317,10 @@ export default Component.extend({ const emojiBelow = [...emojis] .filter((c) => c.offsetTop > active.offsetTop) .find((c) => c.offsetLeft === active.offsetLeft); - this._updateEmojiPreview(emojiBelow.title); - emojiBelow?.focus(); + if (emojiBelow) { + this._updateEmojiPreview(emojiBelow.title); + emojiBelow.focus(); + } } if (event.key === "ArrowUp") { @@ -420,12 +414,9 @@ export default Component.extend({ _applyDiversity(diversity) { const emojiPickerArea = document.querySelector(".emoji-picker-emoji-area"); - - emojiPickerArea && - emojiPickerArea.querySelectorAll(".emoji.diversity").forEach((img) => { - const code = this._codeWithDiversity(img.title, diversity); - img.src = emojiUrlFor(code); - }); + emojiPickerArea?.querySelectorAll(".emoji.diversity").forEach((img) => { + img.src = emojiUrlFor(this._codeWithDiversity(img.title, diversity)); + }); }, _setupSectionObserver() { @@ -442,14 +433,13 @@ export default Component.extend({ return; } - const button = categoryButtons.querySelector( - `.category-button[data-section="${sectionName}"]` - ); - categoryButtons .querySelectorAll(".category-button") .forEach((b) => b.classList.remove("current")); - button && button.classList.add("current"); + + categoryButtons + .querySelector(`.category-button[data-section="${sectionName}"]`) + ?.classList?.add("current"); } }); }, @@ -475,8 +465,7 @@ export default Component.extend({ @bind handleOutsideClick(event) { - const emojiPicker = document.querySelector(".emoji-picker"); - if (emojiPicker && !emojiPicker.contains(event.target)) { + if (!document.querySelector(".emoji-picker")?.contains(event.target)) { this.onClose(event); } }, diff --git a/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js b/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js index 0addc91713b..507fcbbda3b 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js @@ -1,4 +1,4 @@ -import { click, visit } from "@ember/test-helpers"; +import { click, fillIn, visit } from "@ember/test-helpers"; import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { test } from "qunit"; import { @@ -8,7 +8,6 @@ import { query, simulateKey, simulateKeys, - visible, } from "discourse/tests/helpers/qunit-helpers"; acceptance("Emoji", function (needs) { @@ -20,7 +19,6 @@ acceptance("Emoji", function (needs) { await simulateKeys(query(".d-editor-input"), "a :blonde_wo\t"); - assert.ok(visible(".d-editor-preview")); assert.strictEqual( normalizeHtml(query(".d-editor-preview").innerHTML.trim()), normalizeHtml( @@ -29,13 +27,31 @@ acceptance("Emoji", function (needs) { ); }); + test("emoji can be picked from the emoji-picker using the mouse", async function (assert) { + await visit("/t/internationalization-localization/280"); + await click("#topic-footer-buttons .btn.create"); + + await simulateKeys(query(".d-editor-input"), "an :arrow"); + // the 6th item in the list is the "more..." + await click(".autocomplete.ac-emoji ul li:nth-of-type(6)"); + + assert.dom(".emoji-picker.opened.has-filter").exists(); + await click(".emoji-picker .results img:first-of-type"); + + assert.strictEqual( + normalizeHtml(query(".d-editor-preview").innerHTML.trim()), + normalizeHtml( + `

an :arrow_backward:

` + ) + ); + }); + test("skin toned emoji is cooked properly", async function (assert) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-buttons .btn.create"); - await simulateKeys(query(".d-editor-input"), "a :blonde_woman:t5:"); + await fillIn(query(".d-editor-input"), "a :blonde_woman:t5:"); - assert.ok(visible(".d-editor-preview")); assert.strictEqual( normalizeHtml(query(".d-editor-preview").innerHTML.trim()), normalizeHtml( @@ -44,9 +60,7 @@ acceptance("Emoji", function (needs) { ); }); - needs.settings({ - emoji_autocomplete_min_chars: 2, - }); + needs.settings({ emoji_autocomplete_min_chars: 2 }); test("siteSetting:emoji_autocomplete_min_chars", async function (assert) { await visit("/t/internationalization-localization/280");