Skip to content

Commit eeb8f6d

Browse files
committed
fix(emojis): stop the emoji grid re-scrolling when categories change
The scroll-to-category effect depended on `scrollToCategory`, whose identity changes with `categories`. Selecting an emoji rebuilds the frequently-used category, so the effect re-fired and yanked the grid back to the last nav-clicked category. Key the effect on the `scrollTarget` nonce alone via a latest-ref, and defer a frame so a freshly-mounted (search β†’ browse) grid has laid out before scrolling.
1 parent 9f29cfb commit eeb8f6d

2 files changed

Lines changed: 92 additions & 30 deletions

File tree

β€Žsrc/plugins/Emojis/components/EmojiGrid.tsxβ€Ž

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,24 @@ export const EmojiGrid = () => {
5252
[categories],
5353
);
5454

55+
// Latest-ref so the scroll effect can key on the `scrollTarget` nonce alone. Depending
56+
// on `scrollToCategory` (whose identity changes with `categories`) would re-fire the
57+
// scroll on unrelated updates β€” e.g. selecting an emoji updates frequently-used, which
58+
// rebuilds `categories` and would otherwise yank the grid back to the last-scrolled
59+
// category.
60+
const scrollToCategoryRef = useRef(scrollToCategory);
61+
scrollToCategoryRef.current = scrollToCategory;
62+
5563
// Nav clicks (and any consumer) request a scroll via `scrollTarget`; the nonce makes a
56-
// repeat request to the same category re-fire.
64+
// repeat request to the same category re-fire. Deferred a frame so a freshly-mounted
65+
// (search β†’ browse) grid has laid out before we scroll to the requested category.
5766
useEffect(() => {
58-
if (scrollTarget) scrollToCategory(scrollTarget.categoryId);
59-
}, [scrollTarget, scrollToCategory]);
67+
if (!scrollTarget) return;
68+
const frame = requestAnimationFrame(() =>
69+
scrollToCategoryRef.current(scrollTarget.categoryId),
70+
);
71+
return () => cancelAnimationFrame(frame);
72+
}, [scrollTarget]);
6073

6174
// Keyboard nav can target a category that virtualization has unmounted; give it the
6275
// category order + a way to scroll one into view so focus can traverse the whole set.

β€Žsrc/plugins/Emojis/components/__tests__/EmojiGrid.test.tsxβ€Ž

Lines changed: 76 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,47 @@
11
import React from 'react';
22
import { render, screen } from '@testing-library/react';
33

4-
// Capture the Virtuoso scroll callbacks so scroll-spy is testable, and render items
5-
// synchronously so the browse view's a11y tree is assertable (jsdom has no layout).
4+
// Capture the Virtuoso scroll callbacks + imperative scroll so scroll-spy and
5+
// scroll-to-category are testable, and render items synchronously so the browse view's
6+
// a11y tree is assertable (jsdom has no layout).
67
const { virtuoso } = vi.hoisted(() => ({
7-
virtuoso: {} as {
8+
virtuoso: { scrollToIndex: vi.fn() } as {
89
atBottomStateChange?: (atBottom: boolean) => void;
910
rangeChanged?: (range: { endIndex: number; startIndex: number }) => void;
11+
scrollToIndex: ReturnType<typeof vi.fn>;
1012
},
1113
}));
1214

13-
vi.mock('react-virtuoso', () => ({
14-
Virtuoso: ({
15-
atBottomStateChange,
16-
data = [],
17-
itemContent,
18-
rangeChanged,
19-
}: {
20-
atBottomStateChange?: (atBottom: boolean) => void;
21-
data?: unknown[];
22-
itemContent?: (index: number, item: unknown) => React.ReactNode;
23-
rangeChanged?: (range: { endIndex: number; startIndex: number }) => void;
24-
}) => {
25-
virtuoso.atBottomStateChange = atBottomStateChange;
26-
virtuoso.rangeChanged = rangeChanged;
27-
return (
28-
<div>
29-
{data.map((item, index) => (
30-
<React.Fragment key={index}>{itemContent?.(index, item)}</React.Fragment>
31-
))}
32-
</div>
33-
);
34-
},
35-
}));
15+
vi.mock('react-virtuoso', async () => {
16+
const { forwardRef, useImperativeHandle } = await import('react');
17+
return {
18+
Virtuoso: forwardRef(function Virtuoso(
19+
{
20+
atBottomStateChange,
21+
data = [],
22+
itemContent,
23+
rangeChanged,
24+
}: {
25+
atBottomStateChange?: (atBottom: boolean) => void;
26+
data?: unknown[];
27+
itemContent?: (index: number, item: unknown) => React.ReactNode;
28+
rangeChanged?: (range: { endIndex: number; startIndex: number }) => void;
29+
},
30+
ref,
31+
) {
32+
virtuoso.atBottomStateChange = atBottomStateChange;
33+
virtuoso.rangeChanged = rangeChanged;
34+
useImperativeHandle(ref, () => ({ scrollToIndex: virtuoso.scrollToIndex }));
35+
return (
36+
<div>
37+
{data.map((item, index) => (
38+
<React.Fragment key={index}>{itemContent?.(index, item)}</React.Fragment>
39+
))}
40+
</div>
41+
);
42+
}),
43+
};
44+
});
3645

3746
const { ctx, preview } = vi.hoisted(() => ({
3847
ctx: {} as Record<string, unknown>,
@@ -62,7 +71,7 @@ const base = () => ({
6271
categories: [] as EmojiPickerCategory[],
6372
isSearching: false,
6473
resolveNative: (e: { skins: { native: string }[] }) => e.skins[0]?.native ?? '',
65-
scrollTarget: null,
74+
scrollTarget: null as { categoryId: string; nonce: number } | null,
6675
searchResults: null,
6776
selectEmoji: vi.fn(),
6877
setActiveCategory: vi.fn(),
@@ -116,3 +125,43 @@ describe('EmojiGrid scroll-spy', () => {
116125
expect(ctx.setActiveCategory).toHaveBeenLastCalledWith('flags');
117126
});
118127
});
128+
129+
describe('EmojiGrid scroll-to-category', () => {
130+
beforeEach(() => {
131+
// Run the scroll's requestAnimationFrame defer synchronously so it's assertable.
132+
vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => {
133+
cb(0);
134+
return 0;
135+
});
136+
vi.spyOn(window, 'cancelAnimationFrame').mockImplementation(() => undefined);
137+
virtuoso.scrollToIndex.mockClear();
138+
});
139+
afterEach(() => {
140+
vi.restoreAllMocks();
141+
});
142+
143+
it('scrolls to a requested category once, and not when categories change', () => {
144+
ctx.categories = [
145+
{ emojis: [emoji('grinning', 'πŸ˜€')], id: 'people', label: 'People' },
146+
{ emojis: [emoji('checkered_flag', '🏁')], id: 'flags', label: 'Flags' },
147+
];
148+
ctx.scrollTarget = { categoryId: 'flags', nonce: 1 };
149+
const { rerender } = render(<EmojiGrid />);
150+
expect(virtuoso.scrollToIndex).toHaveBeenCalledTimes(1);
151+
expect(virtuoso.scrollToIndex).toHaveBeenLastCalledWith({ align: 'start', index: 1 });
152+
153+
// A categories change (e.g. frequently-used updating after a selection) must NOT
154+
// re-fire the scroll β€” otherwise the grid jumps back to the last-requested category.
155+
ctx.categories = [
156+
{ emojis: [emoji('clock', 'πŸ•')], id: 'frequent', label: 'Frequently used' },
157+
...(ctx.categories as EmojiPickerCategory[]),
158+
];
159+
rerender(<EmojiGrid />);
160+
expect(virtuoso.scrollToIndex).toHaveBeenCalledTimes(1);
161+
162+
// A fresh scroll request (new nonce) does scroll again.
163+
ctx.scrollTarget = { categoryId: 'flags', nonce: 2 };
164+
rerender(<EmojiGrid />);
165+
expect(virtuoso.scrollToIndex).toHaveBeenCalledTimes(2);
166+
});
167+
});

0 commit comments

Comments
Β (0)