Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/BaseSelect/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,9 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref)
}

if (!disabled) {
triggerOpen(false);
triggerOpen(false, {
lazy: true,
});
Comment on lines 572 to +575

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The triggerOpen function is now called with a lazy option set to true. This introduces a delay in closing the dropdown on blur. Ensure this delay does not negatively impact the user experience, especially in scenarios where immediate feedback is expected.

onBlur?.(event);
}
};
Expand All @@ -582,7 +584,9 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref)
// We should give focus back to selector if clicked item is not focusable
if (popupElement?.contains(target as HTMLElement) && triggerOpen) {
// Tell `open` not to close since it's safe in the popup
triggerOpen(true, true);
triggerOpen(true, {
ignoreNext: true,
});
Comment on lines 586 to +589

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The triggerOpen function is called with ignoreNext: true when clicking inside the popup. This prevents the dropdown from closing immediately. Verify that this behavior is consistent with the intended user experience and doesn't introduce unexpected side effects.

}

onMouseDown?.(event, ...restArgs);
Expand Down
36 changes: 23 additions & 13 deletions src/hooks/useOpen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ const macroTask = (fn: VoidFunction, times = 1) => {
* Trigger by latest open call, if nextOpen is undefined, means toggle.
* ignoreNext will skip next call in the macro task queue.
*/
export type TriggerOpenType = (nextOpen?: boolean, ignoreNext?: boolean) => void;
export type TriggerOpenType = (
nextOpen?: boolean,
config?: {
ignoreNext?: boolean;
lazy?: boolean;
},
) => void;
Comment on lines +25 to +31

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The type definition for TriggerOpenType has been updated to include a config object with ignoreNext and lazy properties. Ensure that all usages of TriggerOpenType are updated to reflect this change.


/**
* When `open` is controlled, follow the controlled value;
Expand Down Expand Up @@ -61,30 +67,34 @@ export default function useOpen(
internalSetOpen(nextOpen);
});

const toggleOpen = useEvent<TriggerOpenType>((nextOpen?: boolean, ignoreNext = false) => {
const toggleOpen = useEvent<TriggerOpenType>((nextOpen, config = {}) => {
const { ignoreNext = false, lazy = false } = config;

Comment on lines +70 to +72

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The toggleOpen function now accepts a config object. The default values for ignoreNext and lazy are set to false. This is good for backward compatibility, but ensure that all call sites are reviewed to determine if these options should be explicitly set.

taskIdRef.current += 1;
const id = taskIdRef.current;

const nextOpenVal = typeof nextOpen === 'boolean' ? nextOpen : !mergedOpen;

// Since `mergedOpen` is post-processed, we need to check if the value really changed
if (nextOpenVal) {
triggerEvent(true);

// Lock if needed
if (ignoreNext) {
taskLockRef.current = ignoreNext;

macroTask(() => {
taskLockRef.current = false;
}, 2);
if (nextOpenVal || !lazy) {
if (!taskLockRef.current) {
triggerEvent(nextOpenVal);

// Lock if needed
if (ignoreNext) {
taskLockRef.current = ignoreNext;

macroTask(() => {
taskLockRef.current = false;
}, 2);
}
}
Comment on lines +79 to 91

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for opening the dropdown has been modified to include a check for the lazy option. If nextOpenVal is true or lazy is false, the dropdown is opened immediately (if not locked). This change affects the timing of dropdown opening. Consider adding a comment explaining the rationale behind this change.

return;
}

macroTask(() => {
if (id === taskIdRef.current && !taskLockRef.current) {
triggerEvent(false);
triggerEvent(nextOpenVal);
Comment on lines 95 to +97

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for closing the dropdown is now executed within a macroTask. This introduces a delay in closing the dropdown. Ensure that this delay does not negatively impact the user experience, especially in scenarios where immediate feedback is expected.

}
});
});
Expand Down
Loading