fix: StrictMode modal scroll bar (#174)

* refactor: Support strict mode * fix: Logic * update logic * more test case
react-component · Dec 8, 2020 · 6e8bb72 · 6e8bb72
1 parent cd5701e
commit 6e8bb72
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 70 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -4,6 +4,7 @@ const config = {
     'react/no-did-update-set-state': 0,
     'react/no-find-dom-node': 0,
     'import/no-extraneous-dependencies': 0,
+    'react/sort-comp': 0,
   },
 };
 

diff --git a/src/PortalWrapper.tsx b/src/PortalWrapper.tsx
@@ -54,14 +54,7 @@ export interface PortalWrapperProps {
   }) => React.ReactNode;
 }
 
-export interface PortalWrapperState {
-  _self: PortalWrapper;
-}
-
-class PortalWrapper extends React.Component<
-  PortalWrapperProps,
-  PortalWrapperState
-> {
+class PortalWrapper extends React.Component<PortalWrapperProps> {
   container?: HTMLElement;
 
   componentRef: React.RefObject<PortalRef> = React.createRef();
@@ -74,30 +67,54 @@ class PortalWrapper extends React.Component<
     visible: boolean;
   }) => void;
 
-  constructor(props: PortalWrapperProps) {
-    super(props);
-    const { visible, getContainer } = props;
-    if (supportDom && getParent(getContainer) === document.body) {
-      openCount = visible ? openCount + 1 : openCount;
-    }
-    this.state = {
-      _self: this,
-    };
-  }
-
   componentDidMount() {
+    this.updateOpenCount();
+
     if (!this.attachToParent()) {
       this.rafId = raf(() => {
         this.forceUpdate();
       });
     }
   }
 
-  componentDidUpdate() {
+  componentDidUpdate(prevProps: PortalWrapperProps) {
+    this.updateOpenCount(prevProps);
+
     this.setWrapperClassName();
     this.attachToParent();
   }
 
+  updateOpenCount = (prevProps?: Partial<PortalWrapperProps>) => {
+    const { visible: prevVisible, getContainer: prevGetContainer } =
+      prevProps || {};
+    const { visible, getContainer } = this.props;
+
+    // Update count
+    if (
+      visible !== prevVisible &&
+      supportDom &&
+      getParent(getContainer) === document.body
+    ) {
+      if (visible && !prevVisible) {
+        openCount += 1;
+      } else if (prevProps) {
+        openCount -= 1;
+      }
+    }
+
+    // Clean up container if needed
+    const getContainerIsFunc =
+      typeof getContainer === 'function' &&
+      typeof prevGetContainer === 'function';
+    if (
+      getContainerIsFunc
+        ? getContainer.toString() !== prevGetContainer.toString()
+        : getContainer !== prevGetContainer
+    ) {
+      this.removeCurrentContainer();
+    }
+  };
+
   componentWillUnmount() {
     const { visible, getContainer } = this.props;
     if (supportDom && getParent(getContainer) === document.body) {
@@ -108,36 +125,6 @@ class PortalWrapper extends React.Component<
     raf.cancel(this.rafId);
   }
 
-  static getDerivedStateFromProps(props, { prevProps, _self }) {
-    const { visible, getContainer } = props;
-    if (prevProps) {
-      const {
-        visible: prevVisible,
-        getContainer: prevGetContainer,
-      } = prevProps;
-      if (
-        visible !== prevVisible &&
-        supportDom &&
-        getParent(getContainer) === document.body
-      ) {
-        openCount = visible && !prevVisible ? openCount + 1 : openCount - 1;
-      }
-      const getContainerIsFunc =
-        typeof getContainer === 'function' &&
-        typeof prevGetContainer === 'function';
-      if (
-        getContainerIsFunc
-          ? getContainer.toString() !== prevGetContainer.toString()
-          : getContainer !== prevGetContainer
-      ) {
-        _self.removeCurrentContainer();
-      }
-    }
-    return {
-      prevProps: props,
-    };
-  }
-
   attachToParent = (force = false) => {
     if (force || (this.container && !this.container.parentNode)) {
       const parent = getParent(this.props.getContainer);

diff --git a/tests/Portal.test.tsx b/tests/Portal.test.tsx
@@ -117,30 +117,54 @@ describe('Portal', () => {
     });
   });
 
-  it('openCount', () => {
-    const Demo = ({ count, visible }: { count: number; visible: boolean }) => {
-      return (
-        <>
-          {new Array(count).fill(null).map((_, index) => (
-            <PortalWrapper key={index} visible={visible}>
-              {() => <div>2333</div>}
-            </PortalWrapper>
-          ))}
-        </>
-      );
-    };
+  describe('openCount', () => {
+    it('start as 0', () => {
+      expect(getOpenCount()).toEqual(0);
 
-    const wrapper = mount(<Demo count={1} visible />);
-    expect(getOpenCount()).toEqual(1);
+      const wrapper = mount(
+        <PortalWrapper visible={false}>{() => <div>2333</div>}</PortalWrapper>,
+      );
+      expect(getOpenCount()).toEqual(0);
 
-    wrapper.setProps({ count: 2 });
-    expect(getOpenCount()).toEqual(2);
+      wrapper.setProps({ visible: true });
+      expect(getOpenCount()).toEqual(1);
 
-    wrapper.setProps({ count: 1 });
-    expect(getOpenCount()).toEqual(1);
+      wrapper.unmount();
+    });
 
-    wrapper.setProps({ visible: false });
-    expect(getOpenCount()).toEqual(0);
+    it('correct count', () => {
+      const Demo = ({
+        count,
+        visible,
+      }: {
+        count: number;
+        visible: boolean;
+      }) => {
+        return (
+          <>
+            {new Array(count).fill(null).map((_, index) => (
+              <PortalWrapper key={index} visible={visible}>
+                {() => <div>2333</div>}
+              </PortalWrapper>
+            ))}
+          </>
+        );
+      };
+
+      expect(getOpenCount()).toEqual(0);
+
+      const wrapper = mount(<Demo count={1} visible />);
+      expect(getOpenCount()).toEqual(1);
+
+      wrapper.setProps({ count: 2 });
+      expect(getOpenCount()).toEqual(2);
+
+      wrapper.setProps({ count: 1 });
+      expect(getOpenCount()).toEqual(1);
+
+      wrapper.setProps({ visible: false });
+      expect(getOpenCount()).toEqual(0);
+    });
   });
 
   it('wrapperClassName', () => {