Fix motion popups sometimes getting stuck (#157) (#158)

* Fix motion popups sometimes getting stuck (#157)

If the popup transitioned from visible to invisible before the
status reached 'motion', the popup would get stuck in a visible
state. This changes the code to immediately set status to 'stable'
during a transition from visible to invisible if the status is
something before 'motion'.

Also fixes an issue where an animation frame could change the status
to something invalid after a transition from visible to invisible.

* Add tests

* Remove if statement around raf.cancel

Author: mogzol

src/Popup.tsx

@@ -118,7 +118,14 @@ class Popup extends Component<PopupProps, PopupState> {
       // Init render should always be stable
       newState.status = 'stable';
     } else if (visible !== prevVisible) {
-      newState.status = visible || supportMotion(mergedMotion) ? null : 'stable';
+      if (
+        visible ||
+        (supportMotion(mergedMotion) && ['motion', 'AfterMotion', 'stable'].includes(status))
+      ) {
+        newState.status = null;
+      } else {
+        newState.status = 'stable';
+      }
 
       if (visible) {
         newState.alignClassName = null;
@@ -136,6 +143,9 @@ class Popup extends Component<PopupProps, PopupState> {
     const { status } = this.state;
     const { getRootDomNode, visible, stretch } = this.props;
 
+    // If there is a pending state update, cancel it, a new one will be set if necessary
+    this.cancelFrameState();
+
     if (visible && status !== 'stable') {
       switch (status) {
         case null: {

tests/popup.test.jsx

@@ -0,0 +1,85 @@
+import React from 'react';
+import { mount } from 'enzyme';
+import raf from 'raf';
+import Popup from '../src/Popup';
+
+jest.mock('raf', () => {
+  const rafMock = jest.fn(() => 1);
+  rafMock.cancel = jest.fn();
+  return rafMock;
+});
+
+describe('Popup', () => {
+  afterEach(() => {
+    raf.mockClear();
+    raf.cancel.mockClear();
+  });
+
+  describe('Popup getDerivedStateFromProps status behavior', () => {
+    it('returns stable on init', () => {
+      const props = { visible: false };
+      const state = { prevVisible: null, status: 'something' };
+
+      expect(Popup.getDerivedStateFromProps(props, state).status).toBe('stable');
+    });
+
+    it('does not change when visible is unchanged', () => {
+      const props = { visible: true };
+      const state = { prevVisible: true, status: 'something' };
+
+      expect(Popup.getDerivedStateFromProps(props, state).status).toBe('something');
+    });
+
+    it('returns null when visible is changed to true', () => {
+      const props = { visible: true };
+      const state = { prevVisible: false, status: 'something' };
+
+      expect(Popup.getDerivedStateFromProps(props, state).status).toBe(null);
+    });
+
+    it('returns stable when visible is changed to false and motion is not supported', () => {
+      const props = { visible: false };
+      const state = { prevVisible: true, status: 'something' };
+
+      expect(Popup.getDerivedStateFromProps(props, state).status).toBe('stable');
+    });
+
+    it('returns null when visible is changed to false and motion is started', () => {
+      const props = {
+        visible: false,
+        motion: {
+          motionName: 'enter',
+        },
+      };
+      const state = { prevVisible: true, status: 'motion' };
+
+      expect(Popup.getDerivedStateFromProps(props, state).status).toBe(null);
+    });
+
+    it('returns stable when visible is changed to false and motion is not started', () => {
+      const props = {
+        visible: false,
+        motion: {
+          motionName: 'enter',
+        },
+      };
+      const state = { prevVisible: true, status: 'beforeMotion' };
+
+      expect(Popup.getDerivedStateFromProps(props, state).status).toBe('stable');
+    });
+  });
+
+  it('Popup cancels pending animation frames on update', () => {
+    const wrapper = mount(
+      <Popup visible motion={{}}>
+        <div>popup content</div>
+      </Popup>,
+    );
+
+    expect(raf).toHaveBeenCalledTimes(1);
+
+    raf.cancel.mockClear();
+    wrapper.setProps({ visible: false });
+    expect(raf.cancel).toHaveBeenCalledTimes(1);
+  });
+});