Skip to content

Commit 9fd82a3

Browse files
authored
fix: Align not waiting (#298)
* fix: align queue * chore: commented * chore: commente
1 parent 81b8507 commit 9fd82a3

File tree

3 files changed

+41
-10
lines changed

3 files changed

+41
-10
lines changed

src/Popup/PopupInner.tsx

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,23 @@ const PopupInner = React.forwardRef<PopupInnerRef, PopupInnerProps>(
102102
const [status, goNextStatus] = useVisibleStatus(visible, doMeasure);
103103

104104
// ======================== Aligns ========================
105-
const [alignInfo, setAlignInfo] = useState<AlignType>(null);
105+
/**
106+
* `alignedClassName` may modify `source` size,
107+
* which means one time align may not move to the correct position at once.
108+
*
109+
* We will reset `alignTimes` for each status switch to `alignPre`
110+
* and let `rc-align` to align for multiple times to ensure get final stable place.
111+
* Currently we mark `alignTimes < 2` repeat align, it will increase if user report for align issue.
112+
*/
113+
const [alignTimes, setAlignTimes] = useState(0);
106114
const prepareResolveRef = useRef<(value?: unknown) => void>();
107115

116+
useLayoutEffect(() => {
117+
if (status === 'alignPre') {
118+
setAlignTimes(0);
119+
}
120+
}, [status]);
121+
108122
// `target` on `rc-align` can accept as a function to get the bind element or a point.
109123
// ref: https://www.npmjs.com/package/rc-align
110124
function getAlignTarget() {
@@ -120,11 +134,13 @@ const PopupInner = React.forwardRef<PopupInnerRef, PopupInnerProps>(
120134

121135
function onInternalAlign(popupDomNode: HTMLElement, matchAlign: AlignType) {
122136
const nextAlignedClassName = getClassNameFromAlign(matchAlign);
137+
123138
if (alignedClassName !== nextAlignedClassName) {
124139
setAlignedClassName(nextAlignedClassName);
125140
}
126141

127-
setAlignInfo(matchAlign);
142+
// We will retry multi times to make sure that the element has been align in the right position.
143+
setAlignTimes((val) => val + 1);
128144

129145
if (status === 'align') {
130146
onAlign?.(popupDomNode, matchAlign);
@@ -133,19 +149,17 @@ const PopupInner = React.forwardRef<PopupInnerRef, PopupInnerProps>(
133149

134150
// Delay to go to next status
135151
useLayoutEffect(() => {
136-
if (alignInfo && status === 'align') {
137-
const nextAlignedClassName = getClassNameFromAlign(alignInfo);
138-
152+
if (status === 'align') {
139153
// Repeat until not more align needed
140-
if (alignedClassName !== nextAlignedClassName) {
154+
if (alignTimes < 2) {
141155
forceAlign();
142156
} else {
143157
goNextStatus(function () {
144158
prepareResolveRef.current?.();
145159
});
146160
}
147161
}
148-
}, [alignInfo]);
162+
}, [alignTimes]);
149163

150164
// ======================== Motion ========================
151165
const motion = { ...getMotion(props) };

src/Popup/useVisibleStatus.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,24 @@ import useState from 'rc-util/lib/hooks/useState';
1212
* motion - play the motion
1313
* stable - everything is done
1414
*/
15-
type PopupStatus = null | 'measure' | 'align' | 'aligned' | 'motion' | 'stable';
15+
type PopupStatus =
16+
| null
17+
| 'measure'
18+
| 'alignPre'
19+
| 'align'
20+
| 'aligned'
21+
| 'motion'
22+
| 'stable';
1623

1724
type Func = () => void;
1825

19-
const StatusQueue: PopupStatus[] = ['measure', 'align', null, 'motion'];
26+
const StatusQueue: PopupStatus[] = [
27+
'measure',
28+
'alignPre',
29+
'align',
30+
null,
31+
'motion',
32+
];
2033

2134
export default (
2235
visible: boolean,

tests/point.test.jsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ describe('Trigger.Point', () => {
3838
function trigger(container, eventName, data) {
3939
const pointRegion = container.querySelector('.point-region');
4040
fireEvent(pointRegion, getMouseEvent(eventName, data));
41-
act(() => jest.runAllTimers());
41+
42+
// React scheduler will not hold when useEffect. We need repeat to tell that times go
43+
for (let i = 0; i < 10; i += 1) {
44+
act(() => jest.runAllTimers());
45+
}
4246
}
4347

4448
it('onClick', async () => {

0 commit comments

Comments
 (0)