Skip to content

Commit b3b520b

Browse files
committed
fix(FR-1404): ensure BAIUnmountAfterClose always unmounts after close animations (#4183)
Resolves #4183 ([FR-1404](https://lablup.atlassian.net/browse/FR-1404)) ## Summary Fixes the BAIUnmountAfterClose component to ensure it always properly unmounts Modal and Drawer components after close animations complete, regardless of whether parent components provide afterClose or afterOpenChange callback props. ## Problem The component was using type guards to conditionally add unmounting handlers only when original props existed. This caused the component to fail to unmount when parents didn't provide these callbacks, leading to memory leaks. ## Solution - Removed conditional type guards (`hasAfterClose`, `hasAfterOpenChange`) - Always add our own unmounting handlers (`afterClose`, `afterOpenChange`) - Preserve any existing callbacks while ensuring proper unmounting behavior - Ensures component unmounts after close animations in all scenarios ## Changes - Simplified prop handling by always applying unmounting handlers - Reduced code complexity by removing unnecessary conditional logic - Maintains backward compatibility with existing callback preservation [FR-1404]: https://lablup.atlassian.net/browse/FR-1404?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent 96450ef commit b3b520b

File tree

1 file changed

+10
-18
lines changed

1 file changed

+10
-18
lines changed

packages/backend.ai-ui/src/components/BAIUnmountAfterClose.tsx

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,32 +28,26 @@ const BAIUnmountAfterClose: React.FC<BAIUnmountModalAfterCloseProps> = ({
2828
children,
2929
}) => {
3030
// Ensure there is only one child element
31-
const modalElement = React.Children.only(children);
32-
const isModalOpen = modalElement.props.open;
31+
const childElement = React.Children.only(children);
32+
const isOpen = childElement.props.open;
3333

3434
// Manage internal rendering state
35-
const [isMount, setIsMount] = useState(isModalOpen);
35+
const [isMount, setIsMount] = useState(isOpen);
3636

3737
// Update internal state when the child's open prop becomes true
3838
useLayoutEffect(() => {
39-
if (isModalOpen) {
39+
if (isOpen) {
4040
setIsMount(true);
4141
}
42-
}, [isModalOpen]);
42+
}, [isOpen]);
4343

4444
// Return null if the modal should not be rendered
4545
if (!isMount) {
4646
return null;
4747
}
4848

49-
// Type guards to check if the element is a Modal or Drawer
50-
const hasAfterClose = 'afterClose' in modalElement.props;
51-
const hasAfterOpenChange = 'afterOpenChange' in modalElement.props;
52-
5349
// Preserve the original afterClose callback if it exists
54-
const originalAfterClose = hasAfterClose
55-
? (modalElement.props as ModalProps).afterClose
56-
: undefined;
50+
const originalAfterClose = (childElement.props as ModalProps).afterClose;
5751

5852
// New handler to intercept afterClose
5953
const handleModalAfterClose: ModalProps['afterClose'] = (...args) => {
@@ -65,9 +59,7 @@ const BAIUnmountAfterClose: React.FC<BAIUnmountModalAfterCloseProps> = ({
6559
};
6660

6761
// Preserve the original afterOpenChange callback if it exists
68-
const originalAfterOpenChange = hasAfterOpenChange
69-
? modalElement.props.afterOpenChange
70-
: undefined;
62+
const originalAfterOpenChange = childElement.props.afterOpenChange;
7163

7264
// New handler to intercept afterOpenChange
7365
const handleModalAfterOpenChange: DrawerProps['afterOpenChange'] = (open) => {
@@ -81,11 +73,11 @@ const BAIUnmountAfterClose: React.FC<BAIUnmountModalAfterCloseProps> = ({
8173
};
8274

8375
// Clone the child element with proper typing
84-
const clonedChild = React.cloneElement(modalElement, {
76+
const clonedChild = React.cloneElement(childElement, {
8577
// for Modal
86-
...(hasAfterClose && { afterClose: handleModalAfterClose }),
78+
afterClose: handleModalAfterClose,
8779
// for Drawer
88-
...(hasAfterOpenChange && { afterOpenChange: handleModalAfterOpenChange }),
80+
afterOpenChange: handleModalAfterOpenChange,
8981
});
9082

9183
return clonedChild;

0 commit comments

Comments
 (0)