Skip to content

Commit

Permalink
Fix regression in emoji picker order mangling after clearing filter (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
t3chguy authored and richvdh committed May 12, 2023
1 parent 3f48834 commit 6711e07
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 20 deletions.
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 !== "") {
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
22 changes: 21 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,26 @@ 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()} />,
);

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

Expand Down

0 comments on commit 6711e07

Please sign in to comment.