fix(Transition): fix component wrapping inside Transition.Group (#2130)

Semantic-Org · Sep 30, 2017 · 943bf13 · 943bf13
1 parent 8ecb71d
commit 943bf13
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 11 deletions.
diff --git a/src/modules/Transition/Transition.js b/src/modules/Transition/Transition.js
@@ -125,6 +125,7 @@ export default class Transition extends Component {
   componentDidMount() {
     debug('componentDidMount()')
 
+    this.mounted = true
     this.updateStatus()
   }
 
@@ -134,7 +135,7 @@ export default class Transition extends Component {
     const { current: status, next } = this.computeStatuses(nextProps)
 
     this.nextStatus = next
-    if (status) this.setState({ status })
+    if (status) this.setSafeState({ status })
   }
 
   componentDidUpdate() {
@@ -145,6 +146,8 @@ export default class Transition extends Component {
 
   componentWillUnmount() {
     debug('componentWillUnmount()')
+
+    this.mounted = false
   }
 
   // ----------------------------------------
@@ -156,7 +159,7 @@ export default class Transition extends Component {
     const status = this.nextStatus
 
     this.nextStatus = null
-    this.setState({ status, animating: true }, () => {
+    this.setSafeState({ status, animating: true }, () => {
       _.invoke(this.props, 'onStart', null, { ...this.props, status })
       setTimeout(this.handleComplete, normalizeTransitionDuration(duration, 'show'))
     })
@@ -175,7 +178,7 @@ export default class Transition extends Component {
     const status = this.computeCompletedStatus()
     const callback = current === Transition.ENTERING ? 'onShow' : 'onHide'
 
-    this.setState({ status, animating: false }, () => {
+    this.setSafeState({ status, animating: false }, () => {
       _.invoke(this.props, callback, null, { ...this.props, status })
     })
   }
@@ -284,6 +287,8 @@ export default class Transition extends Component {
     return { ...childStyle, animationDuration }
   }
 
+  setSafeState = (...args) => this.mounted && this.setState(...args)
+
   // ----------------------------------------
   // Render
   // ----------------------------------------

diff --git a/src/modules/Transition/TransitionGroup.js b/src/modules/Transition/TransitionGroup.js
@@ -72,22 +72,27 @@ export default class TransitionGroup extends React.Component {
       const { [key]: prevChild } = prevMapping
       const isLeaving = !_.get(prevChild, 'props.visible')
 
-      // item is new (entering), should be wrapped
+      // Heads up!
+      // An item is new (entering), it will be picked from `nextChildren`, so it should be wrapped
       if (hasNext && (!hasPrev || isLeaving)) {
-        children[key] = this.wrapChild(child, true)
+        children[key] = this.wrapChild(child, { transitionOnMount: true })
         return
       }
 
-      // item is old (exiting), should be updated
+      // Heads up!
+      // An item is old (exiting), it will be picked from `prevChildren`, so it has been already
+      // wrapped, so should be only updated
       if (!hasNext && hasPrev && !isLeaving) {
         children[key] = cloneElement(prevChild, { visible: false })
         return
       }
 
-      // item hasn't changed transition states, copy over the last transition props;
+      // Heads up!
+      // An item item hasn't changed transition states, but it will be picked from `nextChildren`,
+      // so we should wrap it again
       const { props: { visible, transitionOnMount } } = prevChild
 
-      children[key] = cloneElement(child, { visible, transitionOnMount })
+      children[key] = this.wrapChild(child, { transitionOnMount, visible })
     })
 
     this.setState({ children })
@@ -105,9 +110,13 @@ export default class TransitionGroup extends React.Component {
     })
   }
 
-  wrapChild = (child, transitionOnMount = false) => {
+  wrapChild = (child, options = {}) => {
     const { animation, duration } = this.props
     const { key } = child
+    const {
+      visible = true,
+      transitionOnMount = false,
+    } = options
 
     return (
       <Transition
@@ -117,6 +126,7 @@ export default class TransitionGroup extends React.Component {
         onHide={this.handleOnHide}
         reactKey={key}
         transitionOnMount={transitionOnMount}
+        visible={visible}
       >
         {child}
       </Transition>

diff --git a/test/specs/modules/Transition/Transition-test.js b/test/specs/modules/Transition/Transition-test.js
@@ -230,8 +230,9 @@ describe('Transition', () => {
           <p />
         </Transition>,
       )
-      wrapper.setProps({ visible: false })
+      wrapper.should.have.state('status', Transition.ENTERED)
 
+      wrapper.setProps({ visible: false })
       wrapper.instance().should.include({ nextStatus: Transition.EXITING })
     })
 
@@ -241,8 +242,10 @@ describe('Transition', () => {
           <p />
         </Transition>,
       )
-      wrapper.setProps({ visible: true })
+      wrapper.should.have.state('status', Transition.UNMOUNTED)
 
+      wrapper.instance().mounted = true
+      wrapper.setProps({ visible: true })
       wrapper.should.have.state('status', Transition.EXITED)
       wrapper.instance().should.include({ nextStatus: Transition.ENTERING })
     })

diff --git a/test/specs/modules/Transition/TransitionGroup-test.js b/test/specs/modules/Transition/TransitionGroup-test.js
@@ -45,6 +45,7 @@ describe('TransitionGroup', () => {
         .everyWhere((item) => {
           item.should.have.prop('animation', 'scale')
           item.should.have.prop('duration', 1500)
+          item.type().should.equal(Transition)
         })
     })
 
@@ -85,6 +86,9 @@ describe('TransitionGroup', () => {
       wrapper.setProps({ children: [<div key='first' />] })
 
       wrapper.children().should.have.length(2)
+      wrapper.childAt(0).type().should.equal(Transition)
+      wrapper.childAt(0).should.have.prop('visible', true)
+      wrapper.childAt(1).type().should.equal(Transition)
       wrapper.childAt(1).should.have.prop('visible', false)
     })