Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Commit

Permalink
prevent ConfirmDialog from firing multiple alerts at once
Browse files Browse the repository at this point in the history
ConfirmDialog wasn’t idempotent - if it was rendered multiple times in
a row with `display: true`, multiple confirm modals would be fired.

This didn’t happen before in `dash-renderer` but it seems like it is
now with the `devtools` updates. Regardless of `dash-renderer`’s
behaviour, this refactoring makes the component less brittle to its
parent’s logic.

This commit also simplifies the logic by making it stateless and
removing the `setProps` check now that `setProps` is always defined.
  • Loading branch information
chriddyp committed Apr 17, 2019
1 parent 370c5ea commit 8b64f2d
Showing 1 changed file with 9 additions and 26 deletions.
35 changes: 9 additions & 26 deletions src/components/ConfirmDialog.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,39 +10,30 @@ import {Component} from 'react';
export default class ConfirmDialog extends Component {
constructor(props) {
super(props);
this.state = {
displayed: props.displayed,
};
}

_setStateAndProps(value) {
const {setProps} = this.props;
this.setState({displayed: value.displayed});
if (setProps) {
setProps(value);
}
componentDidUpdate(prevProps) {
this._update(!prevProps.displayed && this.props.displayed);
}

componentWillReceiveProps(props) {
this.setState({displayed: props.displayed});
componentDidMount() {
this._update(this.props.displayed);
}

_update() {
const {message, cancel_n_clicks, submit_n_clicks} = this.props;
_update(shouldTriggerDisplay) {
const {message, setProps, cancel_n_clicks, submit_n_clicks} = this.props;

const displayed = this.state.displayed;

if (displayed) {
if (shouldTriggerDisplay) {
new Promise(resolve => resolve(window.confirm(message))).then(
result => {
if (result) {
this._setStateAndProps({
setProps({
submit_n_clicks: submit_n_clicks + 1,
submit_n_clicks_timestamp: Date.now(),
displayed: false,
});
} else {
this._setStateAndProps({
setProps({
cancel_n_clicks: cancel_n_clicks + 1,
cancel_n_clicks_timestamp: Date.now(),
displayed: false,
Expand All @@ -53,14 +44,6 @@ export default class ConfirmDialog extends Component {
}
}

componentDidUpdate() {
this._update();
}

componentDidMount() {
this._update();
}

render() {
return null;
}
Expand Down

0 comments on commit 8b64f2d

Please sign in to comment.