refactor(Popup): detect container changes and replace global detection

shervinchen · May 23, 2024 · e45fec3 · e45fec3
1 parent 609e6b1
commit e45fec3
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 33 deletions.
diff --git a/packages/Popup/Popup.tsx b/packages/Popup/Popup.tsx
@@ -8,12 +8,7 @@ import React, {
 } from 'react';
 import { createPortal } from 'react-dom';
 import { PopupProps, PopupPosition } from './Popup.types';
-import {
-  useClickAnyWhere,
-  useMutationObserver,
-  usePortal,
-  useResize,
-} from '../utils/hooks';
+import { useMutationObserver, usePortal, useResize } from '../utils/hooks';
 
 const Popup: FC<PropsWithChildren<PopupProps>> = ({
   name,
@@ -29,6 +24,7 @@ const Popup: FC<PropsWithChildren<PopupProps>> = ({
     top: 0,
     transform: 'translate(0, 0)',
   });
+  const container = getPopupContainer?.() ?? document.body;
 
   const handleClick = (event: MouseEvent<HTMLDivElement>) => {
     event.stopPropagation();
@@ -44,11 +40,13 @@ const Popup: FC<PropsWithChildren<PopupProps>> = ({
 
   useResize(updatePopupPosition);
 
-  useClickAnyWhere(() => {
-    updatePopupPosition();
-  });
+  useMutationObserver(targetRef?.current, updatePopupPosition);
 
-  useMutationObserver(targetRef, updatePopupPosition);
+  useMutationObserver(container, updatePopupPosition, {
+    attributes: true,
+    childList: false,
+    subtree: false,
+  });
 
   useEffect(() => {
     const targetNode = targetRef?.current ?? null;

diff --git a/packages/Popup/__tests__/Popup.test.tsx b/packages/Popup/__tests__/Popup.test.tsx
@@ -1,4 +1,4 @@
-import { fireEvent, render, screen } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import Popup from '..';
 import { MutableRefObject, useRef } from 'react';
@@ -109,7 +109,7 @@ describe('Popup', () => {
     expect(mockGetPopupPosition).toHaveBeenCalledTimes(2);
   });
 
-  test('should update popup position when document click', async () => {
+  test('should update popup position when mouse over', async () => {
     render(
       <Component
         visible
@@ -118,21 +118,47 @@ describe('Popup', () => {
       />
     );
     expect(mockGetPopupPosition).toHaveBeenCalledTimes(1);
-    await user.click(document.body);
+    await user.hover(targetRefMock.current);
     expect(mockGetPopupPosition).toHaveBeenCalledTimes(2);
   });
 
-  test('should update popup position when mouse over', async () => {
+  test('should update popup position when document size change', async () => {
     render(
-      <Component
-        visible
-        targetRef={targetRefMock}
-        getPopupContainer={getPopupContainerMock}
-      />
+      <Component visible targetRef={targetRefMock} getPopupContainer={null} />
     );
     expect(mockGetPopupPosition).toHaveBeenCalledTimes(1);
-    await user.hover(targetRefMock.current);
-    expect(mockGetPopupPosition).toHaveBeenCalledTimes(2);
+    document.body.style.height = '10000px';
+    await waitFor(() => {
+      expect(mockGetPopupPosition).toHaveBeenCalledTimes(2);
+    });
+  });
+
+  test('should update popup position when container size change', async () => {
+    render(
+      <div
+        id="parentElement"
+        style={{
+          position: 'relative',
+          overflowY: 'auto',
+          width: '400px',
+          height: '200px',
+        }}
+      >
+        <Component
+          visible
+          targetRef={targetRefMock}
+          getPopupContainer={() => document.querySelector('#parentElement')}
+        />
+      </div>
+    );
+    expect(mockGetPopupPosition).toHaveBeenCalledTimes(1);
+    const parentElement = document.querySelector(
+      '#parentElement'
+    ) as HTMLElement;
+    parentElement.style.height = '10000px';
+    await waitFor(() => {
+      expect(mockGetPopupPosition).toHaveBeenCalledTimes(2);
+    });
   });
 
   test('should render to specified container', () => {

diff --git a/packages/Select/__tests__/Select.test.tsx b/packages/Select/__tests__/Select.test.tsx
@@ -351,9 +351,8 @@ describe('Select', () => {
     );
     const select = screen.getByTestId('selectContainer');
     expect(select).toHaveTextContent('React');
-    await act(async () => {
-      await user.click(screen.getAllByTestId('selectTagIcon')[0]);
-    });
+    fireEvent.mouseEnter(select);
+    await user.click(screen.getAllByTestId('selectTagIcon')[0]);
     expect(select).not.toHaveTextContent('React');
   });
 
@@ -369,9 +368,8 @@ describe('Select', () => {
     );
     const select = screen.getByTestId('selectContainer');
     expect(select).toHaveTextContent('React');
-    await act(async () => {
-      await user.click(screen.getAllByTestId('selectTagIcon')[0]);
-    });
+    fireEvent.mouseEnter(select);
+    await user.click(screen.getAllByTestId('selectTagIcon')[0]);
     expect(select).toHaveTextContent('React');
   });
 

diff --git a/packages/utils/hooks/useMutationObserver.ts b/packages/utils/hooks/useMutationObserver.ts
@@ -1,4 +1,4 @@
-import { MutableRefObject, useEffect } from 'react';
+import { useEffect } from 'react';
 
 const defaultOptions: MutationObserverInit = {
   attributes: true,
@@ -7,17 +7,18 @@ const defaultOptions: MutationObserverInit = {
 };
 
 export const useMutationObserver = (
-  ref: MutableRefObject<HTMLElement | null> | undefined,
+  element: HTMLElement | null | undefined,
   callback: MutationCallback,
   options: MutationObserverInit = defaultOptions
 ) => {
   useEffect(() => {
-    if (!ref?.current) return;
+    if (!element) return;
+
     const observer = new MutationObserver(callback);
-    observer.observe(ref.current, options);
+    observer.observe(element, options);
+
     return () => {
       observer.disconnect();
     };
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [ref]);
+  }, [element, callback, options]);
 };