Skip to content

Commit

Permalink
fix: ensure consistency between controlled and uncontrolled logic
Browse files Browse the repository at this point in the history
  • Loading branch information
dujiaqi committed Oct 9, 2023
1 parent f6c6803 commit 9561c4c
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
26 changes: 23 additions & 3 deletions src/hooks/useRangeOpen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export default function useRangeOpen(
firstTimeOpen: boolean,
triggerOpen: (open: boolean, activeIndex: 0 | 1 | false, source: SourceType) => void,
] {
const rafRef = React.useRef<number>(null);

const [firstTimeOpen, setFirstTimeOpen] = React.useState(false);

const [afferentOpen, setAfferentOpen] = useMergedState(defaultOpen || false, {
Expand All @@ -61,13 +63,26 @@ export default function useRangeOpen(

const [nextActiveIndex, setNextActiveIndex] = React.useState<0 | 1>(null);

const queryNextIndex = (index: number) => (index === 0 ? 1 : 0);

React.useEffect(() => {
if (mergedOpen) {
setFirstTimeOpen(true);
}
}, [mergedOpen]);

const queryNextIndex = (index: number) => (index === 0 ? 1 : 0);
React.useEffect(() => {
if (!afferentOpen && rafRef.current !== null) {
// Unfocus
raf.cancel(rafRef.current);
rafRef.current = null;

// Since the index will eventually point to the next one, it needs to be reset.
if (mergedActivePickerIndex !== null) {
setMergedActivePickerIndex(queryNextIndex(mergedActivePickerIndex));
}
}
}, [afferentOpen]);

const triggerOpen = useEvent((nextOpen: boolean, index: 0 | 1 | false, source: SourceType) => {
if (index === false) {
Expand Down Expand Up @@ -102,12 +117,17 @@ export default function useRangeOpen(
setFirstTimeOpen(false);
setMergedActivePickerIndex(customNextActiveIndex);
}

setNextActiveIndex(null);

// Focus back
if (customNextActiveIndex !== null && !disabled[customNextActiveIndex]) {
raf(() => {
// Trigger closure to ensure consistency between controlled and uncontrolled logic.
if (afferentOpen && !firstTimeOpen && nextActiveIndex === null) {
setMergedOpen(false);
}

rafRef.current = raf(() => {
const ref = [startInputRef, endInputRef][customNextActiveIndex];
ref.current?.focus();
});
Expand Down
24 changes: 22 additions & 2 deletions tests/range.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import moment from 'moment';
import KeyCode from 'rc-util/lib/KeyCode';
import { spyElementPrototypes } from 'rc-util/lib/test/domHook';
import { resetWarned } from 'rc-util/lib/warning';
import React from 'react';
import React, { useState } from 'react';
import type { PickerMode } from '../src/interface';
import zhCN from '../src/locale/zh_CN';
import type { RangePickerProps } from '../src/RangePicker';
Expand Down Expand Up @@ -1975,7 +1975,7 @@ describe('Picker.Range', () => {
const onOpenChange = jest.fn();

render(
<MomentRangePicker open showTime onOpenChange={onOpenChange} />,
<MomentRangePicker open showTime onOpenChange={onOpenChange} />
);

for (let i = 0; i < 2; i++) {
Expand All @@ -1985,4 +1985,24 @@ describe('Picker.Range', () => {

expect(onOpenChange).toHaveBeenCalledWith(false);
});

it('controlled open logic should be consistent with the uncontrolled logic', () => {
const Demo: React.FC = () => {
const [open, setOpen] = useState(false);

return <MomentRangePicker open={open} onOpenChange={(bool) => setOpen(bool)} />;
};

const { container, baseElement } = render(<Demo />);

openPicker(container);

expect(baseElement.querySelector('.rc-picker-dropdown-hidden')).toBeFalsy();

selectCell(2);
selectCell(4);

expect(baseElement.querySelector('.rc-picker-dropdown-hidden')).toBeTruthy();
expect(baseElement.querySelectorAll('.rc-picker-input')[1]).toHaveClass('rc-picker-input-active');
});
});

0 comments on commit 9561c4c

Please sign in to comment.