Skip to content

Commit 7e79586

Browse files
authored
fix(trigger): avoid render-based reset for interaction-level deduplication (#601)
* fix(trigger): avoid render-based reset for interaction-level deduplication * fix(trigger): dedupe open change callbacks
1 parent 7b1b680 commit 7e79586

File tree

2 files changed

+158
-16
lines changed

2 files changed

+158
-16
lines changed

src/index.tsx

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export type {
3636

3737
import UniqueProvider, { type UniqueProviderProps } from './UniqueProvider';
3838
import { useControlledState } from '@rc-component/util';
39+
import { flushSync } from 'react-dom';
3940

4041
export { UniqueProvider };
4142
export type { UniqueProviderProps };
@@ -374,23 +375,14 @@ export function generateTrigger(
374375
const openRef = React.useRef(mergedOpen);
375376
openRef.current = mergedOpen;
376377

377-
const lastTriggerRef = React.useRef<boolean[]>([]);
378-
lastTriggerRef.current = [];
379-
380378
const internalTriggerOpen = useEvent((nextOpen: boolean) => {
381-
setInternalOpen(nextOpen);
382-
383-
// Enter or Pointer will both trigger open state change
384-
// We only need take one to avoid duplicated change event trigger
385-
// Use `lastTriggerRef` to record last open type
386-
if (
387-
(lastTriggerRef.current[lastTriggerRef.current.length - 1] ??
388-
mergedOpen) !== nextOpen
389-
) {
390-
lastTriggerRef.current.push(nextOpen);
391-
onOpenChange?.(nextOpen);
392-
onPopupVisibleChange?.(nextOpen);
393-
}
379+
flushSync(() => {
380+
if (mergedOpen !== nextOpen) {
381+
setInternalOpen(nextOpen);
382+
onOpenChange?.(nextOpen);
383+
onPopupVisibleChange?.(nextOpen);
384+
}
385+
});
394386
});
395387

396388
// Trigger for delay

tests/open-change.test.tsx

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import { act, cleanup, fireEvent, render } from '@testing-library/react';
2+
import { spyElementPrototypes } from '@rc-component/util/lib/test/domHook';
3+
import * as React from 'react';
4+
import Trigger from '../src';
5+
6+
const flush = async () => {
7+
for (let i = 0; i < 10; i += 1) {
8+
act(() => {
9+
jest.runAllTimers();
10+
});
11+
12+
await act(async () => {
13+
await Promise.resolve();
14+
});
15+
}
16+
};
17+
18+
describe('Trigger.OpenChange', () => {
19+
let eleRect = {
20+
width: 100,
21+
height: 100,
22+
};
23+
24+
let spanRect = {
25+
x: 0,
26+
y: 0,
27+
left: 0,
28+
top: 0,
29+
width: 1,
30+
height: 1,
31+
};
32+
33+
let popupRect = {
34+
x: 0,
35+
y: 0,
36+
left: 0,
37+
top: 0,
38+
width: 100,
39+
height: 100,
40+
};
41+
42+
beforeAll(() => {
43+
// Keep consistent with other tests to avoid layout related crash in jsdom
44+
spyElementPrototypes(HTMLElement, {
45+
clientWidth: {
46+
get: () => eleRect.width,
47+
},
48+
clientHeight: {
49+
get: () => eleRect.height,
50+
},
51+
offsetWidth: {
52+
get: () => eleRect.width,
53+
},
54+
offsetHeight: {
55+
get: () => eleRect.height,
56+
},
57+
offsetParent: {
58+
get: () => document.body,
59+
},
60+
});
61+
62+
spyElementPrototypes(HTMLDivElement, {
63+
getBoundingClientRect() {
64+
return popupRect;
65+
},
66+
});
67+
68+
spyElementPrototypes(HTMLSpanElement, {
69+
getBoundingClientRect() {
70+
return spanRect;
71+
},
72+
});
73+
});
74+
75+
beforeEach(() => {
76+
eleRect = { width: 100, height: 100 };
77+
spanRect = { x: 0, y: 0, left: 0, top: 0, width: 1, height: 1 };
78+
popupRect = { x: 0, y: 0, left: 0, top: 0, width: 100, height: 100 };
79+
jest.useFakeTimers();
80+
});
81+
82+
afterEach(() => {
83+
cleanup();
84+
jest.useRealTimers();
85+
});
86+
87+
it('should not trigger duplicated open callbacks when pointer and focus happen in same interaction', async () => {
88+
const onOpenChange = jest.fn();
89+
const onPopupVisibleChange = jest.fn();
90+
91+
const { container } = render(
92+
<Trigger
93+
// 同时开启 hover + focus,制造 “一次交互两种事件都试图 open”
94+
action={['hover', 'focus']}
95+
popup={<strong>trigger</strong>}
96+
onOpenChange={onOpenChange}
97+
onPopupVisibleChange={onPopupVisibleChange}
98+
>
99+
<span className="target" />
100+
</Trigger>,
101+
);
102+
103+
const target = container.querySelector('.target') as HTMLElement;
104+
105+
act(() => {
106+
fireEvent.pointerEnter(target);
107+
fireEvent.focus(target);
108+
});
109+
110+
await flush();
111+
112+
expect(onOpenChange).toHaveBeenCalledTimes(1);
113+
expect(onOpenChange).toHaveBeenLastCalledWith(true);
114+
115+
expect(onPopupVisibleChange).toHaveBeenCalledTimes(1);
116+
expect(onPopupVisibleChange).toHaveBeenLastCalledWith(true);
117+
});
118+
119+
it('should not trigger duplicated close callbacks when pointerleave and blur happen in same interaction', async () => {
120+
const onOpenChange = jest.fn();
121+
const onPopupVisibleChange = jest.fn();
122+
123+
const { container } = render(
124+
<Trigger
125+
action={['hover', 'focus']}
126+
popup={<strong>trigger</strong>}
127+
defaultPopupVisible
128+
onOpenChange={onOpenChange}
129+
onPopupVisibleChange={onPopupVisibleChange}
130+
>
131+
<span className="target" />
132+
</Trigger>,
133+
);
134+
135+
const target = container.querySelector('.target') as HTMLElement;
136+
137+
act(() => {
138+
fireEvent.pointerLeave(target);
139+
fireEvent.blur(target);
140+
});
141+
142+
await flush();
143+
144+
expect(onOpenChange).toHaveBeenCalledTimes(1);
145+
expect(onOpenChange).toHaveBeenLastCalledWith(false);
146+
147+
expect(onPopupVisibleChange).toHaveBeenCalledTimes(1);
148+
expect(onPopupVisibleChange).toHaveBeenLastCalledWith(false);
149+
});
150+
});

0 commit comments

Comments
 (0)