Material-ui: [Dialog] Add a hook to run a function when dialog is shown to the user

[adrianmcli]

This is a feature request discussion. The dialog component is supposed to be "focused on a specific task" as mentioned in the official material design spec here.

As a result, it makes sense to be able to set focus to certain elements or trigger some kind of Javascript the moment the dialog is shown to the user.

This is the material-ui component under discussion: https://www.google.com/design/spec/components/dialogs.html

The Problem

If your dialog contains a text field where the user is expected to enter in some text, it only makes logical sense to be able to set focus to that text field when the dialog is shown. However, as of now, there is no easy way to do this. There is no hook for when the modal is shown because the modal is shown whenever the open prop receives a true value.

Potential Solutions

As an initial foray into this problem, I have evaluated two possible solutions: namely being able to call a function when the component is mounted and detecting state change to see when the modal dialog is shown. Both methods assume that we are going to have an onDialogShown prop that will receive a function to be run whenever the dialog is shown to the user. The first method does not work, but the second one seems promising.

Component Mounting

Initially, my naive approach was to call the this.props.onDialogShown inside the componentDidMount() lifecycle function, but I quickly realized that this wouldn't work because the Dialog component is mounted long before the actual dialog is shown to the user. As a result, by the time the dialog is actually shown to the user, componentDidMount() would have been run a long long time ago.

This method would not work unless the actual rendering of the dialog was a component in and of itself that we could control.

Prop Change Detection

Since the dialog is displayed whenever the open prop being passed in evaluates to true, my second idea was that we can try to detect when the open prop goes from false to true. This will involve looking at the componentDidUpdate() life cycle function and essentially making it look something like this:

componentDidUpdate(prevProps, prevState) {
  if (!prevProps.open && this.props.open) {
    this.props.onDialogShown();
  }
}

Before forking and making a full pull-request though, I would like to ask the community whether or not this is the best way forward. Thoughts?

[adrianmcli]

@mbrookes excuse me for not knowing the conventions, why close this if it's still an open question?

[mbrookes]

Because it's a question. Please read the issue template you deleted. :+1:

[adrianmcli]

@mbrookes is this not a feature request?

[nathanmarks]

@adrianmc There's an issue template he's referring to that we have to make issues easier to read as we get so many of them. Can you edit your original post to use the template or resubmit? It auto fills the box when you write an issue.

[mbrookes]

@nathanmarks To be fair, the template does say to remove the headings for feature requests, but on first read, @adrianmc's issue appeared to be a question better suited to alternative forums, and the template says questions may be closed without comment.

If there's a clear feature request here, let's reclassify.

[adrianmcli]

@mbrookes @nathanmarks
Thanks for taking the time to reply. Perhaps I was not clear in my original writing, but I have since edited the original post. Please take a look and let me know if this is okay to be re-opened.

[mbrookes]

Thanks for the detailed explanation.

So my initial question is, given that the open prop is controlled by the parent component and so the state of that prop is already known externally, should the desired behavior not be driven from there, rather than reflecting the state of the open prop back through a callback function prop ('Prop Change Detection` proposal)?

[adrianmcli]

@mbrookes You have a good point there, and no doubt what you say is entirely possible. However, my initial knee-jerk reaction is that it would create a lot more boilerplate for the parent component.

I'll be honest, I am coming from the React-Bootstrap community and am quite used to the wealth of callbacks available there. See link here.

The React-Bootstrap modal dialogs have an almost ridiculous number of callbacks. In fact, there are three callbacks just for the showing of a modal dialog (onEnter, onEntering, and onEntered).

Sure, I absolutely agree with you that it is possible to do it without these callbacks. But I think the primary benefit of what I am suggesting is developer convenience. It would be really nice to be able to pass in a function and not worry about managing changing state in the parent (after all, isn't that one of the main benefits of componentization?).

[mbrookes]

t would be really nice to be able to pass in a function and not worry about managing changing state in the parent

The thing is, you have to manage that state in (or pass a prop through to or however you're communicating that application sate down to Dialog) in the parent component anyway in order to control the open prop on Dialog. Acting on that state or prop in the appropriate component in the hierarchy is practically the same code you would pass to a callback.

[adrianmcli]

Yes, I understand that. But the less state the parent has to manage the better. Ideally (in terms of managing state) the developer ONLY has to worry about the opening and closing of the dialog.

So perhaps what we disagree on is the marginal convenience gained from implementing a hook?

[adrianmcli]

@mbrookes

Actually, I have just tried setting a text input focus in the parent on state change (using componentDidUpdate()) and it simply does not work. It is not possible to set focus to a text input on the prop change of the parent because there seems to be a race condition issue. The text input is not rendered by the time the parent tries to set the focus to it.

What's happening is that the prop in the parent changes long before the modal dialog is displayed (since there's an animation and thus a time delay between the parent prop change and the actual dialog content being displayed).

The whole point of this feature request is to be able to do something AFTER the dialog has been displayed/shown. And since the transition animation is not controlled by the parent, this isn't something that can be easily done.

Again, I urge you to reconsider the need for something like this. It appears to be more complex than it looked at first glance.

[adrianmcli]

I'll leave some code to illustrate what I have done.

In the parent component:

componentDidUpdate(prevProps, prevState) {
    if (!prevState.modalOpen && this.state.modalOpen) {
      $('#myInput').focus();
    }
  }

Note that there is a text input element in the dialog with an id of myInput.

This, however, works if I try to delay it a bit:

componentDidUpdate(prevProps, prevState) {
    if (!prevState.modalOpen && this.state.modalOpen) {
      setTimeout(function(){$('#myInput').focus();},1000);
    }
  }

I'm sure you'll agree with me that this is not a good solution.

[mbrookes]

Ideally (in terms of managing state) the developer ONLY has to worry about the opening and closing of the dialog.

Based on your proposal to that point, this was all the developer had to worry about anyway. Reflecting the value of open prop via a callback adds little utility since that state is known to the parent component.

It is not possible to set focus to a text input on the prop change of the parent because there seems to be a race condition issue. The text input is not rendered by the time the parent tries to set the focus to it.

Okay, now we're getting somewhere. :smile:

In this case I can see the value in a hasOpened style prop, although interestingly, for a recent issue for another component with a similar concern, setting a timeout on the parent component as you have done was the accepted solution.

I'd be interested to hear what the consensus is here. @callemall/material-ui ?

[adrianmcli]

I'm using the timeout for now, but I'm sure you'll agree that it would make a lot more sense to have an actual hook. If, for some reason, the animation lags or is longer than the expected time, the expected behaviour would fall apart.

I'm not well-versed in how the transition animation is handled in the dialog component, but perhaps I'll take a deeper look this weekend.

[nathanmarks]

@adrianmc

Can you make a demo repo with the issue? (using material-ui), or paste some code in here that I can try out

Side note: I looked at the react-bootstrap modal and it has 6 callbacks dedicated to various stages of transition. This isn't something that is in the API design of this library right now. However, I'm really interested in seeing an example of the issue as I do understand how that could be frustrating.

[adrianmcli]

@nathanmarks I've put together a component that demonstrates the issue in its entirety:

import React from 'react';

import Dialog from 'material-ui/lib/dialog';
import TextField from 'material-ui/lib/text-field';
import FlatButton from 'material-ui/lib/flat-button';

export default class Test extends React.Component {

  constructor(props) {
    super(props);
    this.state = {
      open: false
    };
  }

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

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

  componentDidUpdate(prevProps, prevState) {
    if (!prevState.open && this.state.open) {

      this._textField.focus();      // Doesn't work

      setTimeout(() => {
        this._textField.focus();    // Works
      },250);
    }
  }

  render() {

    const actions = [
      <FlatButton
        label="Cancel"
        secondary={true}
      />,
      <FlatButton
        label="Submit"
        primary={true}
      />,
    ];

    return (
      <div>
        <button onClick={this.handleOpen.bind(this)}>Click</button>
        <Dialog
          open={ this.state.open }
          onRequestClose={ this.handleClose.bind(this) }
          actions={ actions }
        >
          <TextField
            floatingLabelText="Title"
            ref={(x) => this._textField = x}
            fullWidth
          />
        </Dialog>
      </div>
    );
  }
}

Go ahead and render the component. The only dependencies are React and Material-UI. You'll see a button you can click on to display a modal dialog.

You'll notice that, if you open the console, you'll see the following error:

Uncaught TypeError: Cannot read property 'focus' of undefined

This error halts execution and it won't ever focus onto the text field for you. The command fails because the text field isn't even rendered yet, that's why you can't focus onto it. The modal is still being animated into view.

Comment out the line marked "Doesn't work" and you'll see that the command being executed inside the setTimeout allows it to work as expected.

[AndrewLindsay221]

+1 for this feature, we are attempting exactly the same, a dialog with text field that we'd like to focus on show.

[adrianmcli]

I still think this is an important feature, but the community has been kind of silent around this use case. My best guess is that most everyone (including myself) has given up and just went back to using setTimeout() instead, which is kind of sad.

[benderunit]

@AndrewLindsay221 The simplest answer I found is setting the autoFocus property on the text field you want to focus on. <TextField autoFocus />. The property is camelCased, so watch out!

[nathanmarks]

@adrianmc We have a couple of issues with dialog that need addressing for 0.16.0. I will make sure we look at this in the process.

I'm assuming that what you're looking for is the ability to call a function after the transitionend event, correct?

[adrianmcli]

@nathanmarks yup, that's exactly right. Glad to hear it's in the books!

[AndrewLindsay221]

@benderunit - we did try this solution as nobody likes to see magic timeout numbers, but we found that something else would grab the focus away from the text field almost immediately, so much like @adrianmc has said, we eventually gave up and accepted the setTimeout solution.

Great to hear that a better solution is on the cards though.

[dwilt]

@adrianmc I'm not using an input but I would like this feature as well. I'm trying to have some images and text fade in when the modal is done _animating in_ not just when the state is now set to true. If I had a callback like you're saying.. say onModalEntered={myCallback}, that would be quite handy. That way I could apply a class to my content and thus trigger off the animations

[nathanmarks]

@dwilt @AndrewLindsay221

The callback will be available in 0.16.0. I already have the new dialog in development.

[nevenduranec]

+1, or dynamically update the input once the dialog opens

[ankitduseja]

Another use case is, I was trying to display the Facebook Page Plugin inside a dialog and I need something similar. If we are lazy loading the fb plugin, we need to call FB.XFBML.parse(); when the dialog has been opened.

I would also like to suggest to pass a scope of dialog to the calling function as I had to pass that to the parse function above. There would be several other similar use cases as well.

[ankitduseja]

@nathanmarks There could be four type of callback events:
boolean willOpen : Triggered before dialog opens. If returned false dialog should not open.
void onOpened : Triggered after dialog opens.
boolean willClose : Triggered before dialog closes. If returned false dialog should not close.
void onClosed : Triggered after dialog closes.

[nathanmarks]

@ankitduseja the callbacks are (in 0.16.0, which is under development):

onEnter
onEntering
onEntered
onExit
onExiting
onExited

[ankitduseja]

@nathanmarks: Sounds good. I hope you have passed the scope of the Dialog div as a parameter in the callbacks.

[TomMahle]

@nathanmarks Thanks so much for tackling this! I am taking a look at the 0.16.0 version and I'm not seeing the new proptypes. Have these been pushed off to the 'next' branch?

[oliviertassinari]

@TomMahle Yes, they should be there.

I can see the following ones:

    /**
     * Callback fires when the backdrop is clicked on.
     */
    onBackdropClick: PropTypes.func,
    /**
     * Callback fired before the dialog is entering.
     */
    onEnter: PropTypes.func,
    /**
     * Callback fired when the dialog is entering.
     */
    onEntering: PropTypes.func,
    /**
     * Callback fired when the dialog has entered.
     */
    onEntered: PropTypes.func, // eslint-disable-line react/sort-prop-types
    /**
     * Callback fires when the escape key is pressed and the modal is in focus.
     */
    onEscapeKeyUp: PropTypes.func, // eslint-disable-line react/sort-prop-types
    /**
     * Callback fired before the dialog is exiting.
     */
    onExit: PropTypes.func,
    /**
     * Callback fired when the dialog is exiting.
     */
    onExiting: PropTypes.func,
    /**
     * Callback fired when the dialog has exited.
     */
    onExited: PropTypes.func, // eslint-disable-line react/sort-prop-types
    /**
     * Callback fired when the dialog requests to be closed.
     */
    onRequestClose: PropTypes.func,

[adrianmcli]

Really glad to see this come full circle! Good job everyone!

[TomMahle]

Fantastic! Can't wait for this :D

[marcfalk]

+1 waiting for this to be released :)

[usergit]

when is this going to be released? any timeframe

[oliviertassinari]

@usergit Those new hooks will be released with the next branch. We don't have any ETA.
We are working on it, component per component.

[jr69]

Sorry, im pretty new here. I'm glad that I am not the only one with this problem. I checked the material-ui version on the front page and its V.0.16.5 on npm. Is this already here, and if so, whats the function to use?

[shift-keshav-pudaruth]

Whilst waiting on the new version to be out, here's a dirty solution to focus on an input element on dialog open:

    componentWillReceiveProps(nextProps)
    {
        //Check if dialog will be opened
        if(this.props.open_dialog === false && nextProps.open_dialog === true)
        {
            //Wait for dialog to finish rendering, then focus on input
            setTimeout(() => {
                this.myInputReference.focus();
            },300)
        }
    }

[jpdillingham]

To clarify @shift-keshav-pudaruth 's solution a little more, if your dialog component is set up as so:

<Dialog open={this.props.open_dialog}>...</Dialog>

You can implement componentWillReceiveProps as follows:

componentWillReceiveProps(nextProps) {
    if (this.props.open_dialog && !nextProps.open_dialog) {
        // TODO: implement dialog close logic
    }

    if (!this.props.open_dialog && nextProps.open_dialog) {
        // TODO: implement dialog open logic
    }
}

[vladimirivanoviliev]

@shift-keshav-pudaruth / @jpdillingham - better approach for the old versions (if you want to avoid setTimeout) would be wrapping the Dialog content in custom component just to track it's ComponentDidMount:

<Dialog
    {...others}
>
    <MountTracker
        onMount={this._onModalContentMount}
    >
        { children }
    </MountTracker>
</Dialog>

```JSX
@autobind
_onModalContentMount() {
if (this.props.onModalContentMount) {
this.props.onModalContentMount();
}
}

```JSX
class MountTracker extends Component {
    componentDidMount() {
       if (this.props.onMount) {
           this.props.onMount();
       }
    }

    render() {
        return this.props.children;
    }
}