Skip to content

Commit c375e7d

Browse files
committed
fix(emojis): correct three picker option/state bugs
- Text-composer middleware: spread options into a fresh object instead of mutating the shared DEFAULT_OPTIONS, so options no longer leak between createTextComposerEmojiMiddleware() calls β€” the no-arg default path kept the wrong trigger/minChars after any earlier customized call. - EmojiPicker: record a "frequently used" emoji only after confirming there is a textarea to insert into, so a no-op selection isn't tracked as used. - useFrequentlyUsedEmoji: build each update from a latest-value ref so two selections in one tick both survive instead of the second dropping the first.
1 parent 79b2a23 commit c375e7d

6 files changed

Lines changed: 77 additions & 9 deletions

File tree

β€Žsrc/plugins/Emojis/EmojiPicker.tsxβ€Ž

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,10 @@ export const EmojiPicker = (props: EmojiPickerProps) => {
164164
referenceElement?.focus();
165165
}}
166166
onEmojiSelect={(emoji) => {
167-
recordUse(emoji.id);
168167
const textarea = textareaRef.current;
169168
if (!textarea) return;
169+
// Record only once we know the emoji is actually being inserted.
170+
recordUse(emoji.id);
170171
textComposer.insertText({ text: emoji.native });
171172
textarea.focus();
172173
if (props.closeOnEmojiSelect) {

β€Žsrc/plugins/Emojis/__tests__/EmojiPicker.test.tsxβ€Ž

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,13 @@ vi.mock('../components', () => ({
3333
),
3434
}));
3535

36+
// Mutable so a test can simulate "no textarea to insert into" (textareaRef.current null).
37+
const { textareaRef } = vi.hoisted(() => ({
38+
textareaRef: { current: null as HTMLTextAreaElement | null },
39+
}));
40+
3641
vi.mock('../../../context', () => ({
37-
useMessageComposerContext: () => ({
38-
textareaRef: { current: document.createElement('textarea') },
39-
}),
42+
useMessageComposerContext: () => ({ textareaRef }),
4043
useTranslationContext: () => ({ t: (key: string) => key }),
4144
}));
4245

@@ -81,6 +84,10 @@ import { EmojiPicker } from '../EmojiPicker';
8184

8285
const openPicker = () => fireEvent.click(screen.getByLabelText('aria/Emoji picker'));
8386

87+
beforeEach(() => {
88+
textareaRef.current = document.createElement('textarea');
89+
});
90+
8491
describe('EmojiPicker session state', () => {
8592
it('keeps skin tone and frequently-used across close and reopen (incl. closeOnEmojiSelect)', () => {
8693
render(<EmojiPicker closeOnEmojiSelect />);
@@ -102,6 +109,18 @@ describe('EmojiPicker session state', () => {
102109
expect(screen.getByTestId('skin-tone')).toHaveTextContent('4');
103110
expect(screen.getByTestId('frequently-used')).toHaveTextContent('rocket');
104111
});
112+
113+
it('does not record a frequently-used emoji when there is no textarea to insert into', () => {
114+
textareaRef.current = null;
115+
render(<EmojiPicker />);
116+
117+
openPicker();
118+
expect(screen.getByTestId('frequently-used').textContent).toBe('');
119+
120+
// Selecting can't insert (no textarea), so it must not be recorded as "used".
121+
fireEvent.click(screen.getByText('select-rocket'));
122+
expect(screen.getByTestId('frequently-used').textContent).toBe('');
123+
});
105124
});
106125

107126
describe('EmojiPicker pickerProps', () => {

β€Žsrc/plugins/Emojis/hooks/__tests__/useFrequentlyUsedEmoji.test.tsβ€Ž

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,27 @@ describe('useFrequentlyUsedEmoji', () => {
2323
rerender({ frequentlyUsedEmoji: ['y', 'x'] });
2424
expect(result.current.frequentlyUsedIds).toEqual(['y', 'x']);
2525
});
26+
27+
it('keeps both when two emoji are recorded before a re-render (uncontrolled)', () => {
28+
const { result } = renderHook(() => useFrequentlyUsedEmoji({}));
29+
// Two selects in the same tick close over the same list β€” the second must still
30+
// build on the first, not overwrite it.
31+
act(() => {
32+
result.current.recordUse('a');
33+
result.current.recordUse('b');
34+
});
35+
expect(result.current.frequentlyUsedIds).toEqual(['b', 'a']);
36+
});
37+
38+
it('accumulates both when two emoji are recorded before a re-render (controlled)', () => {
39+
const onFrequentlyUsedChange = vi.fn();
40+
const { result } = renderHook(() =>
41+
useFrequentlyUsedEmoji({ frequentlyUsedEmoji: [], onFrequentlyUsedChange }),
42+
);
43+
act(() => {
44+
result.current.recordUse('a');
45+
result.current.recordUse('b');
46+
});
47+
expect(onFrequentlyUsedChange).toHaveBeenLastCalledWith(['b', 'a']);
48+
});
2649
});

β€Žsrc/plugins/Emojis/hooks/useFrequentlyUsedEmoji.tsβ€Ž

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useState } from 'react';
1+
import { useCallback, useRef, useState } from 'react';
22

33
export type UseFrequentlyUsedEmojiParams = {
44
/** Controlled ordered list of recently used emoji ids (most recent first). */
@@ -23,16 +23,23 @@ export const useFrequentlyUsedEmoji = ({
2323
const isControlled = Array.isArray(frequentlyUsedEmoji);
2424
const frequentlyUsedIds = isControlled ? frequentlyUsedEmoji : internal;
2525

26+
// Mirror the current list into a ref so several recordUse calls fired before a
27+
// re-render each build on the previous result rather than a stale closure/prop
28+
// (otherwise the second synchronous select would drop the first).
29+
const latestRef = useRef(frequentlyUsedIds);
30+
latestRef.current = frequentlyUsedIds;
31+
2632
const recordUse = useCallback(
2733
(emojiId: string) => {
2834
const next = [
2935
emojiId,
30-
...frequentlyUsedIds.filter((existing) => existing !== emojiId),
36+
...latestRef.current.filter((existing) => existing !== emojiId),
3137
].slice(0, MAX_FREQUENTLY_USED);
38+
latestRef.current = next;
3239
if (!isControlled) setInternal(next);
3340
onFrequentlyUsedChange?.(next);
3441
},
35-
[frequentlyUsedIds, isControlled, onFrequentlyUsedChange],
42+
[isControlled, onFrequentlyUsedChange],
3643
);
3744

3845
return { frequentlyUsedIds, recordUse };

β€Žsrc/plugins/Emojis/middleware/__tests__/textComposerEmojiMiddleware.test.tsβ€Ž

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,21 @@ describe('createTextComposerEmojiMiddleware', () => {
8282
expect(search).toHaveBeenCalled();
8383
expect(items[0]?.id).toBe('custom');
8484
});
85+
86+
it('does not leak options between calls (shared defaults are not mutated)', async () => {
87+
// A call that overrides options must not change the defaults a later no-arg call
88+
// sees. Previously `mergeWith(DEFAULT_OPTIONS, options)` mutated the shared constant.
89+
createTextComposerEmojiMiddleware(undefined, { minChars: 3, trigger: ';' });
90+
91+
const withDefaults = createTextComposerEmojiMiddleware();
92+
const { output } = await runOnChange(withDefaults, {
93+
selection: { end: 4, start: 4 },
94+
suggestions: undefined,
95+
text: ':smi',
96+
});
97+
98+
// Still the default ':' trigger β€” not the ';' leaked from the earlier call.
99+
expect(output?.suggestions?.trigger).toBe(':');
100+
expect(output?.suggestions?.query).toBe('smi');
101+
});
85102
});

β€Žsrc/plugins/Emojis/middleware/textComposerEmojiMiddleware.tsβ€Ž

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import mergeWith from 'lodash.mergewith';
21
import type {
32
Middleware,
43
SearchSourceOptions,
@@ -100,7 +99,9 @@ export const createTextComposerEmojiMiddleware = (
10099
emojiSearchIndex: EmojiSearchIndex = defaultEmojiSearchIndex,
101100
options?: Partial<TextComposerMiddlewareOptions>,
102101
): EmojiMiddleware => {
103-
const finalOptions = mergeWith(DEFAULT_OPTIONS, options ?? {});
102+
// Spread into a fresh object β€” never mutate the shared module-level DEFAULT_OPTIONS
103+
// (options are flat, so a shallow merge is exact).
104+
const finalOptions = { ...DEFAULT_OPTIONS, ...options };
104105
const emojiSearchSource = new EmojiSearchSource(emojiSearchIndex);
105106
emojiSearchSource.activate();
106107

0 commit comments

Comments
Β (0)