From 68ff19fb4b382a8294c2e624d8fe15adea330273 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 11 May 2023 14:24:39 +0100 Subject: [PATCH] Fix regression in emoji picker order mangling after clearing filter (#10854) * Add regression test for emoji picker order mangling after clearing filter * Fix regression in emoji picker order mangling after clearing filter * Iterate * Update src/components/views/emojipicker/EmojiPicker.tsx --- .../views/emojipicker/EmojiPicker.tsx | 37 +++++++++++-------- .../views/emojipicker/EmojiPicker-test.tsx | 22 ++++++++++- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/components/views/emojipicker/EmojiPicker.tsx b/src/components/views/emojipicker/EmojiPicker.tsx index 45464a2a74..3158d52e55 100644 --- a/src/components/views/emojipicker/EmojiPicker.tsx +++ b/src/components/views/emojipicker/EmojiPicker.tsx @@ -268,25 +268,30 @@ class EmojiPicker extends React.Component { } else { emojis = cat.id === "recent" ? this.recentlyUsed : DATA_BY_CATEGORY[cat.id]; } - emojis = emojis.filter((emoji) => this.emojiMatchesFilter(emoji, lcFilter)); - emojis = emojis.sort((a, b) => { - const indexA = a.shortcodes[0].indexOf(lcFilter); - const indexB = b.shortcodes[0].indexOf(lcFilter); - // Prioritize emojis containing the filter in its shortcode - if (indexA == -1 || indexB == -1) { - return indexB - indexA; - } + if (lcFilter !== "") { + emojis = emojis.filter((emoji) => this.emojiMatchesFilter(emoji, lcFilter)); + // Copy the array to not clobber the original unfiltered sorting + emojis = [...emojis].sort((a, b) => { + const indexA = a.shortcodes[0].indexOf(lcFilter); + const indexB = b.shortcodes[0].indexOf(lcFilter); - // If both emojis start with the filter - // put the shorter emoji first - if (indexA == 0 && indexB == 0) { - return a.shortcodes[0].length - b.shortcodes[0].length; - } + // Prioritize emojis containing the filter in its shortcode + if (indexA == -1 || indexB == -1) { + return indexB - indexA; + } + + // If both emojis start with the filter + // put the shorter emoji first + if (indexA == 0 && indexB == 0) { + return a.shortcodes[0].length - b.shortcodes[0].length; + } + + // Prioritize emojis starting with the filter + return indexA - indexB; + }); + } - // Prioritize emojis starting with the filter - return indexA - indexB; - }); this.memoizedDataByCategory[cat.id] = emojis; cat.enabled = emojis.length > 0; // The setState below doesn't re-render the header and we already have the refs for updateVisibility, so... diff --git a/test/components/views/emojipicker/EmojiPicker-test.tsx b/test/components/views/emojipicker/EmojiPicker-test.tsx index 4f8c091bb7..de2469ced6 100644 --- a/test/components/views/emojipicker/EmojiPicker-test.tsx +++ b/test/components/views/emojipicker/EmojiPicker-test.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React from "react"; +import React, { createRef } from "react"; import { render } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; @@ -24,6 +24,26 @@ import { stubClient } from "../../../test-utils"; describe("EmojiPicker", function () { stubClient(); + it("should not mangle default order after filtering", () => { + const ref = createRef(); + const { container } = render( + false} onFinished={jest.fn()} />, + ); + + // Record the HTML before filtering + const beforeHtml = container.innerHTML; + + // Apply a filter and assert that the HTML has changed + //@ts-ignore private access + ref.current!.onChangeFilter("test"); + expect(beforeHtml).not.toEqual(container.innerHTML); + + // Clear the filter and assert that the HTML matches what it was before filtering + //@ts-ignore private access + ref.current!.onChangeFilter(""); + expect(beforeHtml).toEqual(container.innerHTML); + }); + it("sort emojis by shortcode and size", function () { const ep = new EmojiPicker({ onChoose: (str: string) => false, onFinished: jest.fn() });