[Dialog] open property vs render

[lukescott]

Looking at the simple dialog example here: http://www.material-ui.com/#/components/dialog

Is there any reason this couldn't be done like this?

export default class DialogExampleSimple extends React.Component {
  state = {
    open: false,
  };

  handleOpen = () => {
    this.setState({open: true});
  };

  handleClose = () => {
    this.setState({open: false});
  };

  renderDialog() {
    const actions = [
      <FlatButton
        label="Cancel"
        primary={true}
        onTouchTap={this.handleClose}
      />,
      <FlatButton
        label="Submit"
        primary={true}
        keyboardFocused={true}
        onTouchTap={this.handleClose}
      />,
    ];

    return (
      <Dialog
        title="Dialog With Actions"
        actions={actions}
        modal={false}
        onRequestClose={this.handleClose}
      >
        The actions in this window were passed in as an array of React objects.
      </Dialog>
    );
  }

  render() {
    return (
      <div>
        <RaisedButton label="Dialog" onTouchTap={this.handleOpen} />
        {this.state.open && this.renderDialog()}
      </div>
    );
  }
}

Note the removal of open={this.state.open}. The dialog shows when Dialog is mounted, and doesn't show when Dialog is unmounted.

Starting out with material-ui, the original example gives me pause because I have a table with many rows with different dialogs.

Looking at #1682 it says:

I think it would be idiomatic react (and flux) to have props on the dialog that determine if the dialog is visible or not.

I thought idiomatic React was you render something when you want to show it and don't render it when you don't want to show it.

Would it be possible to make open optional and have the above work as well?

[oliviertassinari]

Is there any reason this couldn't be done like this?

Yes, there is. This approach is throwing away the animation.
A better pattern could be the following. Allowing a faster rendering. But I don't think that we support it yet.

    return (
      <Dialog
        title="Dialog With Actions"
        actions={actions}
        modal={false}
        onRequestClose={this.handleClose}
      >
        {open && The actions in this window were passed in as an array of React objects.}
      </Dialog>
    );

[oliviertassinari]

I have been wondering about this issue lately. Given we don't mount the modal DOM on the page, we save more than half of the performance cost, we only pay the React abstraction. I haven't found the need for this enhancement so far. It's something we can implement without too much effort, but the API is less "sexy". So, I'm closing it. If someone is having a performance issue with our Modal, we will reconsider it :).