Skip to content

Commit ec7342a

Browse files
authored
fix: defaultPopupVisible should work (#592)
* fix: merge logic * test: add test case * chore: clean up
1 parent bc399d2 commit ec7342a

File tree

3 files changed

+18
-20
lines changed

3 files changed

+18
-20
lines changed

docs/examples/point.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/* eslint no-console:0 */
22

33
import React from 'react';
4-
import Trigger, { ActionType } from '@rc-component/trigger';
4+
import type { ActionType } from '@rc-component/trigger';
5+
import Trigger from '@rc-component/trigger';
56
import '../../assets/index.less';
67
import './point.less';
78

src/index.tsx

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export type {
3333
};
3434

3535
import UniqueProvider, { type UniqueProviderProps } from './UniqueProvider';
36+
import { useControlledState } from '@rc-component/util';
3637

3738
export { UniqueProvider };
3839
export type { UniqueProviderProps };
@@ -303,12 +304,12 @@ export function generateTrigger(
303304
: null;
304305

305306
// ============================ Open ============================
306-
const [internalOpen, setInternalOpen] = React.useState(
307+
const [internalOpen, setInternalOpen] = useControlledState(
307308
defaultPopupVisible || false,
309+
popupVisible,
308310
);
309311

310-
// Render still use props as first priority
311-
const mergedOpen = popupVisible ?? internalOpen;
312+
const mergedOpen = internalOpen || false;
312313

313314
// ========================== Children ==========================
314315
const child = React.useMemo(() => {
@@ -321,20 +322,9 @@ export function generateTrigger(
321322

322323
const originChildProps = child?.props || {};
323324

324-
// We use effect sync here in case `popupVisible` back to `undefined`
325-
const setMergedOpen = useEvent((nextOpen: boolean) => {
326-
if (openUncontrolled) {
327-
setInternalOpen(nextOpen);
328-
}
329-
});
330-
331325
// Support ref
332326
const isOpen = useEvent(() => mergedOpen);
333327

334-
useLayoutEffect(() => {
335-
setInternalOpen(popupVisible || false);
336-
}, [popupVisible]);
337-
338328
// Extract common options for UniqueProvider
339329
const getUniqueOptions = useEvent((delay: number = 0) => ({
340330
popup,
@@ -385,7 +375,7 @@ export function generateTrigger(
385375
lastTriggerRef.current = [];
386376

387377
const internalTriggerOpen = useEvent((nextOpen: boolean) => {
388-
setMergedOpen(nextOpen);
378+
setInternalOpen(nextOpen);
389379

390380
// Enter or Pointer will both trigger open state change
391381
// We only need take one to avoid duplicated change event trigger

tests/basic.test.jsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,7 @@ describe('Trigger.Basic', () => {
371371
describe('children renderProps', () => {
372372
it('should get current open', () => {
373373
const { container } = render(
374-
<Trigger
375-
popupVisible={true}
376-
popup={<span>Hello!</span>}
377-
>
374+
<Trigger popupVisible={true} popup={<span>Hello!</span>}>
378375
{({ open }) => <button>{String(open)}</button>}
379376
</Trigger>,
380377
);
@@ -994,6 +991,16 @@ describe('Trigger.Basic', () => {
994991
expect(document.querySelector('.rc-trigger-popup-hidden')).toBeTruthy();
995992
});
996993

994+
it('defaultPopupVisible should work', async () => {
995+
render(
996+
<Trigger defaultPopupVisible>
997+
<div className="target" />
998+
</Trigger>,
999+
);
1000+
1001+
expect(document.querySelector('.rc-trigger-popup')).toBeTruthy();
1002+
});
1003+
9971004
describe('click window to hide', () => {
9981005
it('should hide', async () => {
9991006
const onOpenChange = jest.fn();

0 commit comments

Comments
 (0)