Skip to content

Commit

Permalink
[fixed] respect closeTimeoutMS during unmount
Browse files Browse the repository at this point in the history
Fixes issue reactjs#17: "When the modal is unmounted, it will abruptly close,
not waiting for any animations to finish."

The bug was caused by the Modal component unmounting the portal
immediately in `componentWillUnmount` regardless of whether the portal
is currently closing or would animate to close.

Now when a Modal has a non-zero closeTimeoutMS, the Modal inspects the
portal state before closing.  If the portal is open or closing, but not
closed, it waits to unmount the portal.  If the portal is open and not
already closing, the Modal calls closeWithTimeout() to trigger the
close.
  • Loading branch information
Martin Hunt authored and mgh committed Feb 6, 2017
1 parent 6ba1c8d commit c9f793c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
18 changes: 18 additions & 0 deletions lib/components/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,24 @@ export default class Modal extends Component {
ariaAppHider.show(this.props.appElement);
}

var state = this.portal.state;
var now = Date.now();
var closesAt = state.isOpen && this.props.closeTimeoutMS
&& (state.closesAt
|| now + this.props.closeTimeoutMS);

if (closesAt) {
if (!state.beforeClose) {
this.portal.closeWithTimeout();
}

setTimeout(this.removePortal, closesAt - now);
} else {
this.removePortal();
}
}

removePortal () {
ReactDOM.unmountComponentAtNode(this.node);
const parent = getParentElement(this.props.parentSelector);
parent.removeChild(this.node);
Expand Down
10 changes: 6 additions & 4 deletions lib/components/ModalPortal.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,18 @@ export default class ModalPortal extends Component {
}

closeWithTimeout () {
this.setState({ beforeClose: true }, () => {
this.closeTimer = setTimeout(this.closeWithoutTimeout, this.props.closeTimeoutMS);
});
var closesAt = Date.now() + this.props.closeTimeoutMS;
this.setState({beforeClose: true, closesAt: closesAt}, function() {
this.closeTimer = setTimeout(this.closeWithoutTimeout, this.state.closesAt - Date.now());
}.bind(this));
}

closeWithoutTimeout = () => {
this.setState({
beforeClose: false,
isOpen: false,
afterOpen: false,
closesAt: null
}, this.afterClose);
}

Expand Down Expand Up @@ -182,7 +184,7 @@ export default class ModalPortal extends Component {
}

shouldBeClosed () {
return !this.props.isOpen && !this.state.beforeClose;
return !this.state.isOpen && !this.state.beforeClose;
}

contentHasFocus () {
Expand Down

0 comments on commit c9f793c

Please sign in to comment.