From 673f7f73bcca9bc8b952aa7526ee0c7a0a0baf11 Mon Sep 17 00:00:00 2001 From: Adam Butterworth Date: Fri, 5 Feb 2016 14:04:09 -0800 Subject: [PATCH] Update the way ReactTransitionGroup manages children and triggers animation events in order to allow for interruption of transitions. The ReactTransitionGroup test now tests for this behavior. CSSTransition group was updated to expect children to be appended rather than prepended. --- .../transitions/ReactTransitionGroup.js | 196 +++++++++--------- .../__tests__/ReactCSSTransitionGroup-test.js | 14 +- .../__tests__/ReactTransitionGroup-test.js | 147 +++++++++++-- 3 files changed, 233 insertions(+), 124 deletions(-) diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js index 816edfacc57..882581f67b5 100644 --- a/src/addons/transitions/ReactTransitionGroup.js +++ b/src/addons/transitions/ReactTransitionGroup.js @@ -14,7 +14,6 @@ var React = require('React'); var ReactTransitionChildMapping = require('ReactTransitionChildMapping'); -var assign = require('Object.assign'); var emptyFunction = require('emptyFunction'); var ReactTransitionGroup = React.createClass({ @@ -33,138 +32,151 @@ var ReactTransitionGroup = React.createClass({ }, getInitialState: function() { + // children - the set of children that we are trying to acheive + // childrenToRender - expresses our current state and is what we actually render return { children: ReactTransitionChildMapping.getChildMapping(this.props.children), + childrenToRender: {}, }; }, componentWillMount: function() { - this.currentlyTransitioningKeys = {}; - this.keysToEnter = []; - this.keysToLeave = []; + this.actionsToPerform = {}; + this.setState({ + childrenToRender: this.updatechildrenToRender(this.state.children), + }); }, componentDidMount: function() { - var initialChildMapping = this.state.children; - for (var key in initialChildMapping) { - if (initialChildMapping[key]) { - this.performAppear(key); - } - } + this.performchildrenToRenderActions(true); }, componentWillReceiveProps: function(nextProps) { - var nextChildMapping = ReactTransitionChildMapping.getChildMapping( - nextProps.children - ); - var prevChildMapping = this.state.children; + var nextChildMapping = ReactTransitionChildMapping.getChildMapping(nextProps.children); + + var nextchildrenToRender = this.updatechildrenToRender(nextChildMapping); this.setState({ - children: ReactTransitionChildMapping.mergeChildMappings( - prevChildMapping, - nextChildMapping - ), + children: nextChildMapping, + childrenToRender: nextchildrenToRender, }); + }, - var key; + componentDidUpdate: function() { + this.performchildrenToRenderActions(); + }, - for (key in nextChildMapping) { - var hasPrev = prevChildMapping && prevChildMapping.hasOwnProperty(key); - if (nextChildMapping[key] && !hasPrev && - !this.currentlyTransitioningKeys[key]) { - this.keysToEnter.push(key); + updatechildrenToRender: function(newchildren) { + newchildren = newchildren || {}; + var childrenToRender = this.state.childrenToRender; + var nextActionsToPerform = {}; + + // Find new children and add + for (var key in newchildren) { + + if (childrenToRender[key]) { + // Already exists + + // Exists but was on it's way out. Let's interrupt + if (!childrenToRender[key].shouldBeInDOM) { + childrenToRender[key].shouldBeInDOM = true; + // Queue action to be performed during componentDidUpdate + nextActionsToPerform[key] = childrenToRender[key]; + } + } else { + // Is new + childrenToRender[key] = { + child: newchildren[key], + shouldBeInDOM: true, + }; + // Queue action to be performed during componentDidUpdate + nextActionsToPerform[key] = childrenToRender[key]; } } - for (key in prevChildMapping) { - var hasNext = nextChildMapping && nextChildMapping.hasOwnProperty(key); - if (prevChildMapping[key] && !hasNext && - !this.currentlyTransitioningKeys[key]) { - this.keysToLeave.push(key); - } - } + // Find nodes that should longer exist, mark for removal + var childrenKeys = Object.keys(newchildren); + var keysForRemoval = Object.keys(childrenToRender).filter(function(k) { + return childrenKeys.indexOf(k) < 0; + }); + keysForRemoval.forEach(function(keyToRemove) { + childrenToRender[keyToRemove].shouldBeInDOM = false; + // Queue action to be performed during componentDidUpdate + nextActionsToPerform[keyToRemove] = childrenToRender[keyToRemove]; + }); + + this.actionsToPerform = nextActionsToPerform; // If we want to someday check for reordering, we could do it here. + + return childrenToRender; }, - - componentDidUpdate: function() { - var keysToEnter = this.keysToEnter; - this.keysToEnter = []; - keysToEnter.forEach(this.performEnter); - - var keysToLeave = this.keysToLeave; - this.keysToLeave = []; - keysToLeave.forEach(this.performLeave); + + performchildrenToRenderActions: function(isInitialMount) { + for (var key in this.actionsToPerform) { + if (this.actionsToPerform[key].shouldBeInDOM) { + if (isInitialMount) { + this.performAppear(key); + } else { + this.performEnter(key); + } + } else { + this.performLeave(key); + } + } + // Reset actions since we've performed all of them. + this.actionsToPerform = {}; }, performAppear: function(key) { - this.currentlyTransitioningKeys[key] = true; - var component = this.refs[key]; if (component.componentWillAppear) { - component.componentWillAppear( - this._handleDoneAppearing.bind(this, key) - ); + component.componentWillAppear(this._handleDoneAppearing.bind(this, key)); } else { this._handleDoneAppearing(key); } }, _handleDoneAppearing: function(key) { + if (!this.state.childrenToRender[key].shouldBeInDOM) { + // Ignore this callback if the component should now be in the DOM + return; + } + var component = this.refs[key]; + if (component.componentDidAppear) { component.componentDidAppear(); } - - delete this.currentlyTransitioningKeys[key]; - - var currentChildMapping = ReactTransitionChildMapping.getChildMapping( - this.props.children - ); - - if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) { - // This was removed before it had fully appeared. Remove it. - this.performLeave(key); - } }, performEnter: function(key) { - this.currentlyTransitioningKeys[key] = true; - var component = this.refs[key]; if (component.componentWillEnter) { - component.componentWillEnter( - this._handleDoneEntering.bind(this, key) - ); + component.componentWillEnter(this._handleDoneEntering.bind(this, key)); } else { this._handleDoneEntering(key); } }, _handleDoneEntering: function(key) { - var component = this.refs[key]; - if (component.componentDidEnter) { - component.componentDidEnter(); + if (!this.state.childrenToRender[key].shouldBeInDOM) { + // Ignore this callback if the component should no longer be in the DOM + return; } - delete this.currentlyTransitioningKeys[key]; - - var currentChildMapping = ReactTransitionChildMapping.getChildMapping( - this.props.children - ); + var component = this.refs[key]; - if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) { - // This was removed before it had fully entered. Remove it. - this.performLeave(key); + if (component.componentDidEnter) { + component.componentDidEnter(); } }, performLeave: function(key) { - this.currentlyTransitioningKeys[key] = true; - var component = this.refs[key]; + if (component.componentWillLeave) { component.componentWillLeave(this._handleDoneLeaving.bind(this, key)); } else { @@ -176,36 +188,29 @@ var ReactTransitionGroup = React.createClass({ }, _handleDoneLeaving: function(key) { + if (this.state.childrenToRender[key].shouldBeInDOM) { + return; + } + var component = this.refs[key]; if (component.componentDidLeave) { component.componentDidLeave(); } - delete this.currentlyTransitioningKeys[key]; - - var currentChildMapping = ReactTransitionChildMapping.getChildMapping( - this.props.children - ); - - if (currentChildMapping && currentChildMapping.hasOwnProperty(key)) { - // This entered again before it fully left. Add it again. - this.performEnter(key); - } else { - this.setState(function(state) { - var newChildren = assign({}, state.children); - delete newChildren[key]; - return {children: newChildren}; - }); - } + var newChildrenToRender = this.state.childrenToRender; + delete newChildrenToRender[key]; + this.setState({ + childrenToRender: newChildrenToRender, + }); }, render: function() { // TODO: we could get rid of the need for the wrapper node // by cloning a single child var childrenToRender = []; - for (var key in this.state.children) { - var child = this.state.children[key]; + for (var key in this.state.childrenToRender) { + var child = this.state.childrenToRender[key].child; if (child) { // You may need to apply reactive updates to a child as it is leaving. // The normal React way to do it won't work since the child will have @@ -213,15 +218,16 @@ var ReactTransitionGroup = React.createClass({ // a childFactory function to wrap every child, even the ones that are // leaving. childrenToRender.push(React.cloneElement( - this.props.childFactory(child), - {ref: key, key: key} + this.props.childFactory(child), + { ref: key, key: key } )); } } + return React.createElement( this.props.component, this.props, - childrenToRender + childrenToRender, ); }, }); diff --git a/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js index 0bf8c6bb2ff..bea7cb2409b 100644 --- a/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js @@ -23,7 +23,7 @@ describe('ReactCSSTransitionGroup', function() { var container; beforeEach(function() { - jest.resetModuleRegistry(); + require('mock-modules').dumpCache(); React = require('React'); ReactDOM = require('ReactDOM'); ReactCSSTransitionGroup = require('ReactCSSTransitionGroup'); @@ -90,8 +90,8 @@ describe('ReactCSSTransitionGroup', function() { container ); expect(ReactDOM.findDOMNode(a).childNodes.length).toBe(2); - expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('two'); - expect(ReactDOM.findDOMNode(a).childNodes[1].id).toBe('one'); + expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('one'); + expect(ReactDOM.findDOMNode(a).childNodes[1].id).toBe('two'); // For some reason jst is adding extra setTimeout()s and grunt test isn't, // so we need to do this disgusting hack. @@ -125,8 +125,8 @@ describe('ReactCSSTransitionGroup', function() { container ); expect(ReactDOM.findDOMNode(a).childNodes.length).toBe(2); - expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('two'); - expect(ReactDOM.findDOMNode(a).childNodes[1].id).toBe('one'); + expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('one'); + expect(ReactDOM.findDOMNode(a).childNodes[1].id).toBe('two'); }); it('should switch transitionLeave from false to true', function() { @@ -160,8 +160,8 @@ describe('ReactCSSTransitionGroup', function() { container ); expect(ReactDOM.findDOMNode(a).childNodes.length).toBe(2); - expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('three'); - expect(ReactDOM.findDOMNode(a).childNodes[1].id).toBe('two'); + expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('two'); + expect(ReactDOM.findDOMNode(a).childNodes[1].id).toBe('three'); }); it('should work with no children', function() { diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js index f98302be10c..e8911d06436 100644 --- a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js @@ -92,9 +92,8 @@ describe('ReactTransitionGroup', function() { }); }); - it('should handle enter/leave/enter/leave correctly', function() { + it('should handle enter/leave/enter/leave correctly when able to complete transitions', function() { var log = []; - var willEnterCb; var Child = React.createClass({ componentDidMount: function() { @@ -102,7 +101,7 @@ describe('ReactTransitionGroup', function() { }, componentWillEnter: function(cb) { log.push('willEnter'); - willEnterCb = cb; + cb(); }, componentDidEnter: function() { log.push('didEnter'); @@ -137,24 +136,34 @@ describe('ReactTransitionGroup', function() { var instance = ReactDOM.render(, container); expect(log).toEqual(['didMount']); + instance.setState({count: 2}); - expect(log).toEqual(['didMount', 'didMount', 'willEnter']); - for (var k = 0; k < 5; k++) { - instance.setState({count: 2}); - expect(log).toEqual(['didMount', 'didMount', 'willEnter']); - instance.setState({count: 1}); - } - // other animations are blocked until willEnterCb is called - willEnterCb(); + expect(log).toEqual(['didMount', 'didMount', 'willEnter', 'didEnter']); + + instance.setState({count: 1}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', 'didLeave', 'willUnmount', + ]); + + instance.setState({count: 2}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', 'didLeave', 'willUnmount', + 'didMount', 'willEnter', 'didEnter', + ]); + + instance.setState({count: 1}); expect(log).toEqual([ - 'didMount', 'didMount', 'willEnter', - 'didEnter', 'willLeave', 'didLeave', 'willUnmount', + 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', 'didLeave', 'willUnmount', + 'didMount', 'willEnter', 'didEnter', + 'willLeave', 'didLeave', 'willUnmount', ]); }); - it('should handle enter/leave/enter correctly', function() { + it('should handle enter/leave/enter/leave correctly when unable to complete enter transition', function() { var log = []; - var willEnterCb; var Child = React.createClass({ componentDidMount: function() { @@ -162,7 +171,7 @@ describe('ReactTransitionGroup', function() { }, componentWillEnter: function(cb) { log.push('willEnter'); - willEnterCb = cb; + // no callback (this animation didn't have time to complete) }, componentDidEnter: function() { log.push('didEnter'); @@ -197,16 +206,110 @@ describe('ReactTransitionGroup', function() { var instance = ReactDOM.render(, container); expect(log).toEqual(['didMount']); + instance.setState({count: 2}); expect(log).toEqual(['didMount', 'didMount', 'willEnter']); - for (var k = 0; k < 5; k++) { - instance.setState({count: 1}); - expect(log).toEqual(['didMount', 'didMount', 'willEnter']); - instance.setState({count: 2}); - } - willEnterCb(); + + instance.setState({count: 1}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', + 'willLeave', 'didLeave', 'willUnmount', + ]); + + instance.setState({count: 2}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', + 'willLeave', 'didLeave', 'willUnmount', + 'didMount', 'willEnter', + ]); + + instance.setState({count: 1}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', + 'willLeave', 'didLeave', 'willUnmount', + 'didMount', 'willEnter', + 'willLeave', 'didLeave', 'willUnmount', + ]); + }); + + it('should handle enter/leave/enter/leave correctly when unable to complete leave transition', function() { + var log = []; + var leaveCB; + + var Child = React.createClass({ + componentDidMount: function() { + log.push('didMount'); + }, + componentWillEnter: function(cb) { + log.push('willEnter'); + cb(); + }, + componentDidEnter: function() { + log.push('didEnter'); + }, + componentWillLeave: function(cb) { + log.push('willLeave'); + // no callback (this animation didn't have time to complete) + leaveCB = cb; + }, + componentDidLeave: function() { + log.push('didLeave'); + }, + componentWillUnmount: function() { + log.push('willUnmount'); + }, + render: function() { + return ; + }, + }); + + var Component = React.createClass({ + getInitialState: function() { + return {count: 1}; + }, + render: function() { + var children = []; + for (var i = 0; i < this.state.count; i++) { + children.push(); + } + return {children}; + }, + }); + + var instance = ReactDOM.render(, container); + expect(log).toEqual(['didMount']); + instance.setState({count: 2}); + expect(log).toEqual(['didMount', 'didMount', 'willEnter', 'didEnter']); + + instance.setState({count: 1}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', + ]); + + instance.setState({count: 2}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', + 'willEnter', 'didEnter', + ]); + + instance.setState({count: 1}); + expect(log).toEqual([ + 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', + 'willEnter', 'didEnter', + 'willLeave', + ]); + + leaveCB(); // Leave given enough time to complete + expect(log).toEqual([ 'didMount', 'didMount', 'willEnter', 'didEnter', + 'willLeave', + 'willEnter', 'didEnter', + 'willLeave', + 'didLeave', 'willUnmount', ]); });