feat: Picker's onOpen & onClose should decouple with onEnter & onExit (…

…#3228)

* feat(PickerTrigger): add `onOpen` & `onClose`

* feat(Picker): remove onOpen/onClose inside Picker

* fix(Picker): 修复picker重复调用onClose

* fix(Picker): fix datepicker onClose not triggered

* refactor(Picker): bind onOpen/onClose with open state

* test(Picker): add test case for onOpen/onClose with open props

* test(OverlayTrigger): add open control with onOpen/onClose

* test: add tests for the open state of all pickers

* fix: update testPickers.tsx

---------

Co-authored-by: simonguo <simonguo.2009@gmail.com>

src/Cascader/Cascader.tsx

@@ -178,8 +178,6 @@ const Cascader = React.forwardRef(<T extends DataItemValue>(props: CascaderProps
     onChange,
     onSelect,
     onSearch,
-    onClose,
-    onOpen,
     getChildren,
     menuClassName: DEPRECATED_menuClassName,
     menuStyle: DEPRECATED_menuStyle,
@@ -322,8 +320,6 @@ const Cascader = React.forwardRef(<T extends DataItemValue>(props: CascaderProps
   });
 
   const { active, handleEntered, handleExited } = useActive({
-    onOpen,
-    onClose,
     onEntered,
     onExited,
     target,

src/Cascader/test/CascaderSpec.tsx

@@ -23,7 +23,7 @@ describe('Cascader', () => {
       return screen.getByRole('combobox');
     }
   });
-  testPickers(Cascader);
+  testPickers(Cascader, { ariaHaspopup: 'tree' });
   testControlledUnControlled(Cascader, {
     componentProps: { data: items, defaultOpen: true },
     value: '1',

src/CheckPicker/CheckPicker.tsx

@@ -118,8 +118,6 @@ const CheckPicker = React.forwardRef(
       onClean,
       onChange,
       onSelect,
-      onClose,
-      onOpen,
       ...rest
     } = props;
 
@@ -220,9 +218,6 @@ const CheckPicker = React.forwardRef(
       onMenuKeyDown: onFocusItem,
       onMenuPressEnter: handleMenuPressEnter,
       onMenuPressBackspace: handleClean,
-      onClose: () => {
-        setFocusItemValue(null);
-      },
       ...rest
     });
 
@@ -252,14 +247,12 @@ const CheckPicker = React.forwardRef(
 
     const handleEntered = useEventCallback(() => {
       setActive(true);
-      onOpen?.();
     });
 
     const handleExited = useEventCallback(() => {
       resetSearch();
       setFocusItemValue(null);
       setActive(false);
-      onClose?.();
     });
 
     const selectedItems =

src/CheckPicker/test/CheckPickerSpec.tsx

@@ -392,6 +392,24 @@ describe('CheckPicker', () => {
     expect(screen.getAllByRole('checkbox')[2]).to.be.checked;
   });
 
+  it('Should trigger onOpen & onClose with open props set', () => {
+    const onOpenSpy = sinon.spy();
+    const onCloseSpy = sinon.spy();
+    const { rerender } = render(
+      <CheckPicker data={data} onOpen={onOpenSpy} onClose={onCloseSpy} open />
+    );
+
+    fireEvent.click(screen.getByRole('combobox'));
+
+    expect(onCloseSpy).to.have.been.calledOnce;
+
+    rerender(<CheckPicker data={data} onOpen={onOpenSpy} onClose={onCloseSpy} open={false} />);
+
+    fireEvent.click(screen.getByRole('combobox'));
+
+    expect(onOpenSpy).to.have.been.calledOnce;
+  });
+
   it('Should render a button by toggleAs={Button}', () => {
     render(<CheckPicker open data={data} toggleAs={Button} />);
     expect(screen.getByRole('combobox')).to.have.class('rs-btn');

src/CheckTreePicker/CheckTreePicker.tsx

@@ -150,12 +150,10 @@ const CheckTreePicker: PickerComponent<CheckTreePickerProps> = React.forwardRef(
     onEntered,
     onChange,
     onClean,
-    onClose,
     onExited,
     onSearch,
     onSelect,
     onSelectItem,
-    onOpen,
     onScroll,
     onExpand,
     renderValue,
@@ -425,17 +423,13 @@ const CheckTreePicker: PickerComponent<CheckTreePickerProps> = React.forwardRef(
   });
 
   const handleOpen = useEventCallback(() => {
-    trigger.current?.open?.();
     setFocusItemValue(activeNode?.[valueKey]);
     focusActiveNode();
-    onOpen?.();
     setActive(true);
   });
 
   const handleClose = useEventCallback(() => {
-    trigger.current?.close?.();
     setSearchKeyword('');
-    onClose?.();
     setFocusItemValue(null);
     setActive(false);
 
@@ -577,7 +571,6 @@ const CheckTreePicker: PickerComponent<CheckTreePickerProps> = React.forwardRef(
     searchInput,
     active,
     onExit: handleClean,
-    onClose: handleClose,
     onMenuKeyDown: event => {
       onMenuKeyDown(event, {
         down: () => handleFocusItem(KEY_VALUES.DOWN),

src/CheckTreePicker/test/CheckTreePickerSpec.tsx

@@ -23,7 +23,7 @@ describe('CheckTreePicker', () => {
       return screen.getByRole('combobox');
     }
   });
-  testPickers(CheckTreePicker, { virtualized: true });
+  testPickers(CheckTreePicker, { virtualized: true, ariaHaspopup: 'tree' });
   testControlledUnControlled(CheckTreePicker, {
     componentProps: {
       data: controlledData,

src/DatePicker/DatePicker.tsx

@@ -14,7 +14,6 @@ import Stack from '../Stack';
 import PredefinedRanges from './PredefinedRanges';
 import { DatePickerLocale } from '../locales';
 import {
-  createChainedFunction,
   mergeRefs,
   useClassNames,
   useControlled,
@@ -274,12 +273,10 @@ const DatePicker: RsRefForwardingComponent<'div', DatePickerProps> = React.forwa
       onChange,
       onChangeCalendarDate,
       onClean,
-      onClose,
       onEntered,
       onExited,
       onNextMonth,
       onOk,
-      onOpen,
       onPrevMonth,
       onSelect,
       onToggleMonthDropdown,
@@ -721,6 +718,8 @@ const DatePicker: RsRefForwardingComponent<'div', DatePickerProps> = React.forwa
       }
 
       setShowMonthDropdown(false);
+
+      props.onClose?.();
     });
 
     const showCleanButton = cleanable && hasValue && !readOnly;
@@ -734,8 +733,8 @@ const DatePicker: RsRefForwardingComponent<'div', DatePickerProps> = React.forwa
         ref={trigger}
         placement={placement}
         onClose={handleTriggerClose}
-        onEntered={createChainedFunction(onOpen, onEntered)}
-        onExited={createChainedFunction(onClose, onExited)}
+        onEntered={onEntered}
+        onExited={onExited}
         speaker={renderCalendarOverlay}
       >
         <Component

src/DatePicker/test/DatePickerSpec.tsx

@@ -27,7 +27,7 @@ describe('DatePicker', () => {
     }
   });
 
-  testPickers(DatePicker);
+  testPickers(DatePicker, { role: 'textbox', ariaHaspopup: 'dialog' });
 
   testControlledUnControlled(DatePicker, {
     defaultValue: new Date('2023-10-01'),

src/DateRangePicker/DateRangePicker.tsx

@@ -236,12 +236,10 @@ const DateRangePicker = React.forwardRef((props: DateRangePickerProps, ref) => {
     value: valueProp,
     onChange,
     onClean,
-    onClose,
     onEnter,
     onEntered,
     onExited,
     onOk,
-    onOpen,
     onSelect,
     onShortcutClick,
     renderTitle,
@@ -900,8 +898,8 @@ const DateRangePicker = React.forwardRef((props: DateRangePickerProps, ref) => {
       pickerProps={pick(props, pickTriggerPropKeys)}
       placement={placement}
       onEnter={createChainedFunction(handleEnter, onEnter)}
-      onEntered={createChainedFunction(onOpen, onEntered)}
-      onExited={createChainedFunction(onClose, onExited)}
+      onEntered={onEntered}
+      onExited={onExited}
       speaker={renderCalendarOverlay}
     >
       <Component

src/DateRangePicker/test/DateRangePickerSpec.tsx

@@ -51,7 +51,7 @@ describe('DateRangePicker', () => {
     }
   });
 
-  testPickers(DateRangePicker);
+  testPickers(DateRangePicker, { role: 'textbox', ariaHaspopup: 'dialog' });
   testControlledUnControlled(DateRangePicker, {
     defaultValue: [new Date('2023-11-01'), new Date('2023-11-02')],
     value: [new Date('2023-11-03'), new Date('2023-11-04')],

src/InputPicker/InputPicker.tsx

@@ -164,8 +164,6 @@ const InputPicker: PickerComponent<InputPickerProps> = React.forwardRef(
       onCreate,
       onSearch,
       onSelect,
-      onOpen,
-      onClose,
       onBlur,
       onFocus,
       searchBy,
@@ -524,7 +522,6 @@ const InputPicker: PickerComponent<InputPickerProps> = React.forwardRef(
     const handleExited = useEventCallback(() => {
       setFocusItemValue(multi ? value?.[0] : value);
       resetSearch();
-      onClose?.();
     });
 
     const handleFocus = useEventCallback(() => {
@@ -747,7 +744,7 @@ const InputPicker: PickerComponent<InputPickerProps> = React.forwardRef(
         ref={triggerRef}
         trigger="active"
         onEnter={createChainedFunction(handleEnter, onEnter)}
-        onEntered={createChainedFunction(onEntered, onOpen)}
+        onEntered={onEntered}
         onExit={createChainedFunction(handleExit, onExit)}
         onExited={createChainedFunction(handleExited, onExited)}
         speaker={renderPopup}

src/MultiCascader/MultiCascader.tsx

@@ -170,8 +170,6 @@ const MultiCascader: PickerComponent<MultiCascaderProps<DataItemValue>> = React.
       onSearch,
       onSelect,
       onChange,
-      onOpen,
-      onClose,
       onCheck,
       menuClassName: DEPRECATED_menuClassName,
       menuStyle: DEPRECATED_menuStyle,
@@ -274,8 +272,6 @@ const MultiCascader: PickerComponent<MultiCascaderProps<DataItemValue>> = React.
     });
 
     const { active, handleEntered, handleExited } = useActive({
-      onOpen,
-      onClose,
       onEntered,
       onExited,
       target,

src/MultiCascader/test/MultiCascaderSpec.tsx

@@ -22,7 +22,7 @@ describe('MultiCascader', () => {
       return screen.getByRole('combobox');
     }
   });
-  testPickers(MultiCascader);
+  testPickers(MultiCascader, { ariaHaspopup: 'tree' });
   testControlledUnControlled(MultiCascader, {
     componentProps: {
       data: items,

src/SelectPicker/SelectPicker.tsx

@@ -167,8 +167,6 @@ const SelectPicker = React.forwardRef(
       onChange,
       onSelect,
       onSearch,
-      onClose,
-      onOpen,
       sort,
       renderValue,
       renderMenu,
@@ -276,23 +274,19 @@ const SelectPicker = React.forwardRef(
       onExit: handleClean,
       onMenuKeyDown: onFocusItem,
       onMenuPressEnter: handleMenuPressEnter,
-      onClose: () => {
-        setFocusItemValue(null);
-      },
       ...rest
     });
 
     const handleExited = useEventCallback(() => {
       resetSearch();
       setActive(false);
       onSearch?.('');
-      onClose?.();
+      setFocusItemValue(null);
     });
 
     const handleEntered = useEventCallback(() => {
       setActive(true);
       setFocusItemValue(value);
-      onOpen?.();
     });
 
     // Find active `MenuItem` by `value`

src/TreePicker/TreePicker.tsx

@@ -160,13 +160,11 @@ const TreePicker: PickerComponent<TreePickerProps> = React.forwardRef((props, re
     onExit,
     onExited,
     onClean,
-    onOpen,
     onSearch,
     onSelect,
     onSelectItem,
     onChange,
     onEntered,
-    onClose,
     onDragEnd,
     onDragStart,
     onDragEnter,
@@ -476,14 +474,11 @@ const TreePicker: PickerComponent<TreePickerProps> = React.forwardRef((props, re
   });
 
   const handleOpen = useEventCallback(() => {
-    trigger.current?.open?.();
     focusActiveNode();
-    onOpen?.();
     setActive(true);
   });
 
   const handleClose = useEventCallback(() => {
-    trigger.current?.close?.();
     setSearchKeyword('');
     setActive(false);
     setFocusItemValue(activeNode?.[valueKey]);
@@ -580,7 +575,6 @@ const TreePicker: PickerComponent<TreePickerProps> = React.forwardRef((props, re
     searchInput,
     active,
     onExit: handleClean,
-    onClose: handleClose,
     onMenuKeyDown: event => {
       onMenuKeyDown(event, {
         down: () => handleFocusItem(KEY_VALUES.DOWN),
@@ -795,7 +789,7 @@ const TreePicker: PickerComponent<TreePickerProps> = React.forwardRef((props, re
       placement={placement}
       onEnter={handleOpen}
       onEntered={onEntered}
-      onExit={createChainedFunction(onClose, onExit)}
+      onExit={onExit}
       onExited={createChainedFunction(handleClose, onExited)}
       speaker={renderTreeView}
     >

src/TreePicker/test/TreePickerSpec.tsx

@@ -25,7 +25,7 @@ describe('TreePicker', () => {
     }
   });
 
-  testPickers(TreePicker);
+  testPickers(TreePicker, { ariaHaspopup: 'tree' });
   testControlledUnControlled(TreePicker, {
     componentProps: { data, defaultOpen: true },
     value: '1',