Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix regression in emoji picker order mangling after clearing filter #10854

Merged
merged 4 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions src/components/views/emojipicker/EmojiPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,25 +268,30 @@ class EmojiPicker extends React.Component<IProps, IState> {
} 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 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;
});

if (lcFilter) {
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
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);

// 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;
});
}

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...
Expand Down
17 changes: 16 additions & 1 deletion test/components/views/emojipicker/EmojiPicker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -24,6 +24,21 @@ import { stubClient } from "../../../test-utils";
describe("EmojiPicker", function () {
stubClient();

it("should not mangle default order after filtering", () => {
const ref = createRef<EmojiPicker>();
const { container } = render(
<EmojiPicker ref={ref} onChoose={(str: string) => false} onFinished={jest.fn()} />,
);

const beforeHtml = container.innerHTML;
//@ts-ignore private access
ref.current!.onChangeFilter("test");
expect(beforeHtml).not.toEqual(container.innerHTML);
//@ts-ignore private access
ref.current!.onChangeFilter("");
expect(beforeHtml).toEqual(container.innerHTML);
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
});

it("sort emojis by shortcode and size", function () {
const ep = new EmojiPicker({ onChoose: (str: string) => false, onFinished: jest.fn() });

Expand Down